PDO error when using POST data value as parameter

Question

I have a SELECT statement that I am building via PHP and PDO to provide a list of users who have logged in the last XX minutes. When I hard code the interval of time the SQL statement executes fine yet when I try to substitute an interval selected from a web form I get a SQL error. I am not sure what is wrong. I am using PDO and the PREPARE statement

try
{
    $sql = 'SELECT DISTINCT PlayerName 
        FROM Player_Data pd LEFT JOIN character_data cd 
        ON pd.PlayerUID = cd.PlayerUID 
        WHERE cd.LastLogin > DATE_SUB(NOW(), :login_interval_value)';
    $statement = $pdo->prepare($sql);
    $statement->bindValue(':login_interval_value',$_POST['login_interval']);
    $statement->execute();
    $results = $statement->fetchAll();
}
catch (PDOException $e)
{
    $error = 'Error getting player names: ' . $e->getMessage();
    include 'error.html.php';
    exit();
}

This is the error I get ...

Error getting player names: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''INTERVAL 60 MINUTES')' at line 4

why not try to store `$_POST['login_interval']` to another variable and assign it on the parameter? — Netorica, May 29 '13 at 01:16
Per your suggestion I tried that but I ended up with the same error. — webworm, May 29 '13 at 01:24

score 4 · Accepted Answer · answered May 29 '13 at 01:27

4

Two corrections. The time unit is singular(1). The other is that you need to have your post data for login_interval just be the number of minutes. This is perfectly legal:

DATE_SUB(NOW(), INTERVAL '60' MINUTE)

This is not correct and is what happens when your post data is the entire interval expression:

DATE_SUB(NOW(), 'INTERVAL 60 MINUTE')

So either change your form so $_POST['login_interval'] is just the number of minutes or extract the number from it. Assuming you change your form, this is what your code changes to:

try
{
    $sql = 'SELECT DISTINCT PlayerName 
        FROM Player_Data pd LEFT JOIN character_data cd 
        ON pd.PlayerUID = cd.PlayerUID 
        WHERE cd.LastLogin > DATE_SUB(NOW(), INTERVAL :login_interval_value MINUTE)';
    $statement = $pdo->prepare($sql);
    $statement->bindValue(':login_interval_value',$_POST['login_interval']);
    $statement->execute();
    $results = $statement->fetchAll();
}

1 - https://dev.mysql.com/doc/refman/5.5/en/date-and-time-functions.html#function_date-add

answered May 29 '13 at 01:27

Erik Nedwidek

6,134
1
25
25

1

I'd also add that I would add a check to make sure that the $_POST['login_interval'] is actually a number. Read those docs to see what MySQL does with quoted strings in that context. The other point to embed the casted int value like xdazz points out. – Erik Nedwidek May 29 '13 at 01:31
This did it! I didn't realize that the integer value for the INTERVAL could be quoted nor that the bindValue parameter would have single quotes around it. I wanted too use the bindValue to gaurd against SQL injection. Thank you so much! – webworm May 29 '13 at 01:34
@webworm please note that the comment, unlike the answer itself, is totally misleading. Mysql will cast a quoted string to a number all right, with no fatal consequences. And to embed the casted int value like xdazz points out **is not the way to go.** – Your Common Sense May 29 '13 at 01:48
@Your_Common_Sense I am afraid I don't quite understand. The other answers (though appreciated) seem to build the SQL statement dynamically which I had always learned was a bad idea, hence why I wanted to "prepare" the query. – webworm May 29 '13 at 01:53
Upon reread, I see my comment is poorly phrased. I definitely agree that the way in the answer is the way to do it. Using xdazz's [perfectly legal] solution hurts my eyes when PDO is being used. MySQL is fine using bare [no quotes] numbers, but quoting them is much better form. When you use bare numbers, MySQL will still treat them as a string and can do funny things (look at that doc link for an example). – Erik Nedwidek May 29 '13 at 02:02
@webworm you are right, but the first comment told you not to do so. That's why I disagree with *comment*. The answer itself is all right and actually the only right answer here. – Your Common Sense May 29 '13 at 02:26
@ErikNedwidek you are probably confusing data literal with an expression. Mysql will treat a passed value either as a string or a number, but not as expression by any means. So, there is no difference which value you bind to this placeholder - it will be treat as a number only – Your Common Sense May 29 '13 at 03:05

score 1 · Answer 2 · answered May 29 '13 at 01:18

You can't use placeholder for INTERVAL 60 MINUTES, it will quote it.

You just need to cast the value into integer.(Post number instead.)

$sql = 'SELECT DISTINCT PlayerName 
    FROM Player_Data pd LEFT JOIN character_data cd 
    ON pd.PlayerUID = cd.PlayerUID 
    WHERE cd.LastLogin > DATE_SUB(NOW(), INTERVAL '.(int)$_POST['login_interval'].' MINUTES)';
$statement = $pdo->prepare($sql);
$statement->execute();
$results = $statement->fetchAll();

PDO error when using POST data value as parameter

2 Answers2