CODEWITHSUNDEEP
phpmysqlpdo
matski
matski

Reputation: 541

php PDO UPDATE statements: which is safer?

I'm learning PDO, and finding it tricky to make sure my statements work correctly. I have a PHP function which is updating my database by simply adding the number 1 to the total.

    function add_rating($place_id,$rating_id) {

    //make $db accessible inside the function
    global $db;

    // query v1
    $sql = "UPDATE places_ratings SET ? +1 WHERE place_id=?";
    $q = $db->prepare($sql);
    $q->execute(array($rating_id,$place_id));   

}

I tried variations of this, none of which I could get to work. I don't know if I was using question marks wrong. I was following this guide and also a previous SO question. In the end I tried a different method which worked first time, so I am tempted to re-use it as it also seems a lot simpler.

    function add_rating($place_id,$rating_id) {

    //make $db accessible inside the function
    global $db;

    // query v2
    $query = "UPDATE places_ratings SET $rating_id = ($rating_id +1) WHERE place_id = $place_id";
    $update = $db->query($query);

}

My question is: which statement is better/safer? And secondly, what am I doing wrong with the first version with question marks? Thanks...

Upvotes: 1

Views: 963

Answers (3)

anon
anon

Reputation:

In general prepared statements as in your first example are safer because they are immune to SQL injection.

Your example doesn't work because you can't specify field names using a ? parameter in a prepared statement. Even if you could your SQL still would be wrong, this would expand to 

 UPDATE places_ratings SET whatever +1 WHERE place_id=?

which is not valid.

If your $rating_id is generated in code and not taken from user input you could combine both approaches.

Upvotes: 2

tereško
tereško

Reputation: 58444

Please, go an learn what prepared statements are. And you could also use a tutorial, that does not promote bad practices and vulnerable code.

A correctly created and used prepared statement will always be more secure then concatenated query string, because prepared statements send query logic and data separately.

Also , if you are using PDO, then quite often the use of bindParam() method should be preferred over passing the values directly in the execute() method as an array. This is because, when passing values in execute(), the values are bound as PDO::PARAM_STR, even if DB column expects and integer.

P.S. Stop using global in your code !!

Upvotes: 1

deceze
deceze

Reputation: 522382

Prepared statements are not simply like copy'n'pasting variables into a piece of text. Prepared statements separate between the query logic and the values the query should work on. They're there so you're able to tell your database "You're supposed to do this", let the database understand it, then give it the values it's supposed to do that something with. The logic itself cannot be variable, it needs to be complete the first time.

Therefore, you can only use placeholders for values. Your query needs to read UPDATE ... SET FIELD = VALUE WHERE FIELD = VALUE. The FIELD parts need to be in the statement, the VALUE parts you can use placeholders for. It looks like your $rating_id variable is a variable field name. First of all, that's a bad idea. You should not make field names variable if possible. But if you have to, you cannot use prepared statement placeholders for them. Instead, you'll have to do it like this:

$rating_id = 'field_name';
$query = "UPDATE places_ratings SET `$rating_id` = `$rating_id` + 1 WHERE `place_id` = ?";
$stmt = $db->prepare($query);
$stmt->execute(array($place_id));

It's up to you to make sure $rating_id is safe and contains known, whitelisted values. Don't let the user supply the value for it in any way.

Upvotes: 2

Related Questions