PDO. Am I doing it correctly?

[cloudll]

I have only recently started using PDO to connect to my database and was wondering if the code i'm using is okay.

 

I am using the following to query my database.

    $sql = "SELECT * FROM movement WHERE id='1'";
    foreach ($connect->query($sql) as $row)
    {
    $dbpos_x = $row['pos_x'];
    $dbpos_y = $row['pos_y'];
    }

and I am using the following to update my database.

    $pos_y = 25;
    $id = 1;
    $sql = "UPDATE movement SET pos_y = pos_y-? WHERE id=?";   
    $statement = $connect->prepare($sql);
    $statement->execute(array($pos_y,$id));

Does that all look okay, I wasn't sure if i was using the prepared statements correctly. Thanks for any tips.

[Psycho]

I have only recently started using PDO to connect to my database and was wondering if the code i'm using is okay.

 

I am using the following to query my database.

    $sql = "SELECT * FROM movement WHERE id='1'";
    foreach ($connect->query($sql) as $row)
    {
    $dbpos_x = $row['pos_x'];
    $dbpos_y = $row['pos_y'];
    }

 

1. Don't use 'SELECT *' - state the fields you want in the SELECT clause. Using SELECT * can open you up to vulnerabilities and defects that you would not otherwise have.

2. If you only expect one record - don't use a loop to get the record.

3. There is no logic to handle a situation if the query fails

4. I would assume the 'id' is really dynamic and your example just doesn't show that. If so, you should be using prepared statements to prevent SQL injection

5. No need to create variables for the returned data - just use '$row['pos_x']', for example. It is a waste of code to create variables for ones that already exist. There are situations where you may need to do that, but this doesn't appear to be one.

 

 

    $pos_y = 25;
    $id = 1;
    $sql = "UPDATE movement SET pos_y = pos_y-? WHERE id=?";   
    $statement = $connect->prepare($sql);
    $statement->execute(array($pos_y,$id));

Does that all look okay, I wasn't sure if i was using the prepared statements correctly. Thanks for any tips.

 

1, Again, no code to check for errors.

2. I prefer to use named placeholders with bound parameters. That way you just assign a value to a variable and execute the query.

3. Also, by default, PDO only simulates prepared statements. You should turn emulation off to increase security

$connect->setAttribute( PDO::ATTR_EMULATE_PREPARES, false );

Edited May 11, 2015 by Psycho

[cloudll]

Thanks for all the tips

[cloudll]

Sorry i posted this in the wrong thread. This one is solved

 

 

 

So I looked into using the new hashing function. To test it i created a password with the following code and inserted it into my database

$password = "password";
$hash = password_hash($password, PASSWORD_DEFAULT);

I then check if the login password matches that password in the database using the following

if (($username == $db_username) && (password_verify($db_password, $hash))) {                    
$_SESSION['username'] = $_POST['username'];

However my login fails.

If i echo the database password directly and print the password entered in my login form. The hashes do not match. Infact they change on every refresh so how would they ever match the password in the database?

Edited May 14, 2015 by cloudll

[ginerjm]

Can you show a more complete example? So many things that we don't know about in your current ex.

 

The html? The capture of the input? The whole sequence of that and the query and THEN the check, altho you really don't need a check if you do this correctly.

[Psycho]

This line of code makes no sense:

 

if (($username == $db_username) && (password_verify($db_password, $hash))) {

 

The query should include the username in the WHERE clause. So only the record that matches the username would be returned (or no results would be returned). So, why would you need to verify the username from the DB results?

[cloudll]

It is being used for a private login with only one username and password. I can add the WHERE close if you think it will help