CODEWITHSUNDEEP
phparrayssessionescaping
zeckdude
zeckdude

Reputation: 16163

Why does the addslashes() function not work on my array in php?

I have a series of session variables in an array. When I use quotes in one of my string variables, I try to addslashes so I can eventually insert it into the DB, but the addslashes() function is not working. Here is an example.

In the comments field, I write this:

This is the "comment"

I realize this is a problem so I added a function before I enter it into the database that runs through a series of Session variables, including the comments variable.

$strip_fields = array($_SESSION['comments'],$_SESSION['employee_id'],$_SESSION['approved_by'],$_SESSION['delivery_email'],$_SESSION['full_name'],$_SESSION['first_name'],$_SESSION['last_name']);

        foreach($strip_fields as $key => $value) {
            $key = addslashes($key);
        }

After I run this function I try to echo out the comments variable $_SESSION['comments']

This is the "comment"

So I can see that the addslashes function somehow does not work the way I am using it. Why does the addslashes function not work the way I'm using it?

THIS IS MY SOLUTION (I used a bit from both suggestions)

$strip_fields = array(
            'employee_id', 'approved_by', 'delivery_email', 'full_name', 
            'first_name', 'last_name', 'title', 'title_2', 'dept_div', 
            'dept_div_2', 'email', 'comments', 'special_instructions'
        );

        foreach($strip_fields as $key) {
            $_SESSION[$key] = $conn->real_escape_string($_SESSION[$key]);
        }

Upvotes: 1

Views: 2873

Answers (5)

DaedalusFall
DaedalusFall

Reputation: 8565

Your question implies that you actually want to modify the contents of your $_SESSION variable, but this doesn't seem like a good idea, as you will just end up adding slashes again and again whenever the script is called (just an observation without having seen all your code).

Also, you shouldn't use addslashes to escape for a database, as each database (or database abstraction layer) has it's own way of escaping data (for example mysql_real_escape_string or PDO prepared statements).

Also, $_SESSION and a database are different ways of persisting data, and it may well be that mixing them is a bad design choice.

EDIT AFTER COMMENT...

If you want to put all those variables into the database, and prior to that they are in $_SESSION (which as previously stated may not be the best idea), and you are using the mysql php module, then you could do something like:

$db_names=array(
    "comments",
    "employee_id",
    "approved_by",
    "full_name",
    "first_name",
    "last_name"
);
$clean=array();
foreach($db_names as $name)
    $clean[$name]=mysql_real_escape_string($_SESSION[$db_name]);
mysql_query("
        INSERT INTO comments_table 
        (
            comments,
            employee_id,
            approved_by,
            full_name,
            first_name,
            last_name
        )
        VALUES
        (
            '{$clean["comments"]}',
            '{$clean["employee_id"]}',
            '{$clean["approved_by"]}',
            '{$clean["full_name"]}',
            '{$clean["first_name"]}',
            '{$clean["last_name"]}'
        )
");

However, its best not to use the mysql module, but instead use mysqli or PDO. Each have different (and better, for a number of reasons) ways of escaping strings.

Upvotes: 5

kapa
kapa

Reputation: 78671

DON'T USE addslashes() TO ESCAPE VALUES FOR AN SQL QUERY! (sorry for shouting)

In the case of MySQL, your choice is mysql_real_escape_string() but other engines have their own escape functions. addslashes() is amazingly easy to "fool" if you are trying to do an SQL Injection attack.

The best thing you could do is not escaping $_SESSION itself, but creating a copy, and using array_map() along with mysql_real_escape_string(). This way you can keep you original unescaped version also.

$escaped_SESSION=array_map('mysql_real_escape_string', $_SESSION);

Others already explained why foreach is not the best choice for manipulating your array. It works with a copy of the array you are running it on. array_map() will apply a callback to all the elements of your array, and return the resulting array.

Upvotes: 1

Gaurav
Gaurav

Reputation: 28755

$array = array('comment' => $_SESSION['comment'], 'employee_id' => $_SESSION['employee_id']);  // other keys of session

foreach(array_keys($array) as $value){
    $array[$value] = addslashes($array[$value]);
}

use $array instead of using $_SESSION.

Upvotes: 0

David Harkness
David Harkness

Reputation: 36522

There are a few things wrong here:

  1. You are copying the values in $_SESSION to new variables.
  2. You are passing the keys to addslashes(), but you placed the values into array values.
  3. foreach() copies the values from the array into $key and $value, so you're operating on a copy of a copy.

You should be able to use references for this, but I think skipping them will be clearer.

$strip_fields = array(
        'comments', 'employee_id', 'approved_by', 'delivery_email', 
        'full_name', 'first_name', 'last_name']
    );

foreach($strip_fields as $key) {
    $_SESSION[$key] = addslashes($_SESSION[$key]);
}

Upvotes: 2

Brombomb
Brombomb

Reputation: 7076

You need to store the resulting value back into the value you are echoing

$strip_fields = array($_SESSION['comments'],$_SESSION['employee_id'],$_SESSION['approved_by'],$_SESSION['delivery_email'],$_SESSION['full_name'],$_SESSION['first_name'],$_SESSION['last_name']);

    foreach($strip_fields as $key => $value) {
        $strip_fields[$key] = addslashes($value); // Store it back into the strip_fields var
    }

Also as @Gaurav noted you want to clean the array data, not the index/key.

Upvotes: -1

Related Questions