UPDATE SQL with prepared stmt using only 1 variable

Question

HTML form field names must be equal to SQL table field names.

Changing only table name and allowed fields can be used in many other update pages.

How can I improve this?

$allowed = array("name","surname","email","rank"); 
$items = '';
foreach($_POST as $key => $value) {
    if (in_array($key , $allowed)) {
        $items.="`".str_replace("`","``",$key)."`". "='$value', ";
    }
}
$items = substr($items, 0, -2); 

$table = "users" ;
$userId = $_POST['userId'];

$sql = "UPDATE $table SET $items WHERE ID = ?";
if ($stmt = $mysqli->prepare($sql)) {
    $stmt->bind_param('i', $userId);
    $stmt->execute();
}

CODE REVIEW with PDO prepare statement

In accord with the answer, my first code was open to SQL injection and i change it with PDO prepared statement like a suggestion.

$dbh = new PDO('mysql:host=localhost;port=0000;dbname=xxx','yyy','zzz',
array(PDO::ATTR_PERSISTENT => false,PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION, PDO::ATTR_EMULATE_PREPARES => false));

$allowed = array("name","surname","email","rank");
$params = array();
$items = null;

foreach($_POST as $key => $value) {
    if (in_array($key , $allowed)) {
        $items .= "$key=:$key ,"; // create parametrized string
        $params[$key] = $value; // populate the array with allowed $_POST values 
    }
}

$items = substr($items, 0, -2); // escaping the last coma
$table = "users" ;

$sql = "UPDATE $table SET $items WHERE ID = :userId ";
$params['userId'] = $_POST['userId']; // add the WHERE param value to array
$stmt = $dbh->prepare($sql);
$stmt->execute($params);

In this way, the variable $items doing all job :)

Cerad · Accepted Answer · 2014-03-07 20:08:39Z

3

As it stands right now, you are wide open to sql injection injection attacks because you are copying posted data directly into your sql update statement. All kinds of nasty things could be injected.

I started to rewrite it but it became a pain since mysqli requires you to bind each parameter individually. Way too much work. I'd suggest shifting to PDO which supports binding by arrays as well as named parameters. Not to mention exceptions.

Something like:

// Some test data
$_POST   = array('name' => 'New Name', 'email' => 'New Email', 'something' => 'nothing', 'userId' => 1);

$allowed = array("name","surname","email","rank"); 

$params = array();
$items = null;

foreach($_POST as $key => $value) {
    if (in_array($key , $allowed)) {
        if ($items) $items .= ', ';
        $items .= "$key=:$key";
        $params[$key] = $value;
    }
}
if (!$items) die("Nothing to update");
echo $items . "\n"; // name=:name, email=:email

$sql = sprintf('UPDATE %s SET %s WHERE ID = :userId','users',$items);

echo $sql . "\n"; // UPDATE users SET name=:name, email=:email WHERE ID = :userId

$params['userId'] = $_POST['userId'];

$stmt = $pdo->prepare($sql);
$stmt->execute($params); // This is reason enough to shift to PDO

answered Mar 7, 2014 at 20:08

Cerad

2911 silver badge3 bronze badges

\$\begingroup\$ With bind_param you can bind as much params as you want. (php.net/manual/de/mysqli-stmt.bind-param.php) \$\endgroup\$

Marco Acierno
– Marco Acierno

2014-03-07 20:19:16 +00:00
Commented Mar 7, 2014 at 20:19
\$\begingroup\$ In theory but go ahead and show your code for binding a variable number of parameters. Unless something has changed, you can't just feed bind an array. \$\endgroup\$

Cerad
– Cerad

2014-03-07 20:29:08 +00:00
Commented Mar 7, 2014 at 20:29
\$\begingroup\$ @Cerad i understand and i change the code with PDO. Only 1 question : why you use sprintf in your code instead of : "UPDATE $table set $items WHERE ID = :userId" ? \$\endgroup\$

geomo
– geomo

2014-03-08 14:58:13 +00:00
Commented Mar 8, 2014 at 14:58
\$\begingroup\$ Personal preference. Long time C programmer. I find sprintf's easier to read. Either approach is fine. \$\endgroup\$

Cerad
– Cerad

2014-03-08 15:01:10 +00:00
Commented Mar 8, 2014 at 15:01

Add a comment |

Stack Exchange Network

UPDATE SQL with prepared stmt using only 1 variable

1 Answer 1

You must log in to answer this question.

Hot Network Questions

UPDATE SQL with prepared stmt using only 1 variable

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions