1

I went through the process of converting mysql_* code into PDO code. I've run it and checked that it works and everything. I just want Stack Overflow's review of it, to make sure that I'm killing the connection properly, whether I should use some other method instead (e.g. transactions), making sure there are not massive security flaws. Here's the code:

<?php
    try {
        $link = new PDO('mysql:****;dbname=****;charset=UTF-8','****','****');
        $link->exec("INSERT INTO Registration (`First Name`, `Last Name`) VALUES ('$_POST[fname]', '$_POST[lname]')");
    } catch(PDOException $e) {
        print "Error!: " . $e->getMessage() . "<br/>";
        die();
    }
?>

Like I said, it works, but I want it to be safe and effective when 100 people register at the same time. Does everything look okay?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Richard
  • 5,840
  • 36
  • 123
  • 208
  • You should be using bound parameters to generate your SQL - if you're just adding in user-supplied input, there's no security gain at all from using PDO. – andrewsi Sep 17 '12 at 20:28
  • You can use my simple, single-file [PDO library](https://github.com/Xeoncross/DByte) so you don't have to worry about SQL injection or passing a global `$link` variable around your pages. As it is now, you are not safe since you are directly inserting variables into your query strings. – Xeoncross Sep 17 '12 at 20:28
  • Don't ever use non-sanitized data from the user directly in your query. Also, put single/double quotes around the key when you use an array variable. – AdamGold Sep 17 '12 at 20:31

1 Answers1

5

No .. you are converting mysql_ to PDO 1:1. This way, issues in mysql_ will also be a issue in PDO.

You should look at prepared queries and parameter binding.

Here is a example of what I mean:

$dbh = new PDO('mysql:****;dbname=****;charset=UTF-8','****','****');

$first = 'John';
$last = 'Doe';

$stmt = $dbh->prepare(
   "INSERT INTO Registration (firstname, lastname) VALUES (:first, :last)");
$stmt->bindParam(':first', $first);
$stmt->bindParam(':last', $last);

$stmt->execute();

// insert another row with different values
$first = 'John';
$last = 'Smith';
$stmt->execute();
JvdBerg
  • 21,777
  • 8
  • 38
  • 55
  • 1
    Can I put this into a for-loop? Because let's say I have 21 people i want to insert, then can I iterate through $_POST to add every single person? – Richard Sep 17 '12 at 20:35
  • 1
    Yes you can. You create the dbh and prepare the query once. Then you loop, and change the parameters in every loop and execute. No need to do mysql_escape_stuff! – JvdBerg Sep 17 '12 at 20:37
  • Okay will do. So I'm not sure what the bindParam function does. I tried reading the documentation on it but I'm not fully understanding its purpose. I understand the variable $first is being binded to an sql statement...but what does that do for me O.o – Richard Sep 17 '12 at 20:50