0

I am converting to PDO and I'm having a problem converting at the section where it checks to see if the username and email is taken or not.

below is the code:

<?php
session_start();
$host     = "localhost";
$username = "root";
$password = "123";
$dbname   = "test";
$conn = new PDO("mysql:host=$host;dbname=$dbname",$username,$password);
?>

<?php
if(isset($_POST['register'])){
$username = $_POST['username'];
$password = $_POST['password'];
$email = $_POST['email'];

    $usernamecheck = $conn->query("SELECT `id` FROM `user` WHERE                   username='$username'");
    $emailcheck  = $conn->query("SELECT `id` FROM `user` WHERE email='$email'");
    if(mysql_num_rows($usernamecheck) > 0){
        echo "That username is already taken";
    }elseif(mysql_num_rows($emailcheck) > 0){
        echo "That e-mail address is already in use";
}    
?>

The errors I get are at the two following lines:

if(mysql_num_rows($usernamecheck) > 0){

}elseif(mysql_num_rows($emailcheck) > 0){

Thanks in Advance.

Community
  • 1
  • 1
Sam Bo
  • 23
  • 3
  • 1
    See http://stackoverflow.com/questions/2304315/get-number-of-rows-from-a-select-statement, but note that you are currently getting _none_ of the benefit of using PDO since you're not using parameterized queries. Your code is still vulnerable to SQL injection. – Michael Berkowski Jul 18 '13 at 00:09
  • I strongly recommend [reading through this tutorial](http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers) – Michael Berkowski Jul 18 '13 at 00:09
  • 1
    You can't mix the `mysql_` functions and PDO together like that. The database handles are not interchangable. Pick one or the other. Not both. – Charles Jul 18 '13 at 00:11
  • thanks guys...i just started php for like 2-3 weeks...and then stumbled upon PDO so i googled some examples of how to select, insert, and update. I was just following an example I found on how to format a pdo select... didn't realize it wasn't even the proper way...i will be more diligent in the future – Sam Bo Jul 18 '13 at 00:46

2 Answers2

1

You're using mysql_num_rows() for a PDO query. You can't mix these APIs.

You're also interpolating $_POST variables directly into your SQL, which is a no-no for security. The benefit of using PDO is that you can easily use SQL query parameters instead, which is much easier and more secure.

Here's how I'd code this task:

$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$stmt = $conn->prepare("SELECT COUNT(*) AS count FROM `user` WHERE username=?");
$stmt->execute(array($username));
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
  $username_count = $row["count"];
}
if ($username_count > 0) {
  echo "That username is already taken";
}

$stmt = $conn->prepare("SELECT COUNT(*) AS count FROM `user` WHERE email=?");
$stmt->execute(array($email));
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
  $email_count = $row["count"];
}
if ($email_count > 0) {
  echo "That email address is already in use";
}

Also keep in mind that even if you check first, you should assume that someday two people may be trying to create the same username simultaneously, and if the code for their respective requests executes in just the wrong sequence, they could both be told the username does not exist, go ahead and INSERT it. So you should define a UNIQUE KEY on the columns that must be unique. Only the first one to INSERT will succeed, the other will get an error. So you must check for errors.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • hi thanks for the answer... i have a question about your answer... what do yo u mean by interpolating_$post variables? is it when I tried to do the follow: $emailcheck = $conn->query("SELECT `id` FROM `user` WHERE email='$email'"); Thanks in advance – Sam Bo Jul 18 '13 at 00:38
  • Yes, interpolation is when you put a variable inside another string like this SQL statement, and PHP substitutes the variable name with its value. There is no interpolation in my example above when I use the `?` parameter placeholders. – Bill Karwin Jul 18 '13 at 04:21
1

First of all, the entire task is rather pointless. Making a username unique makes no sense. Given email is used to identify a user, the username - or, rather - display name could be anything and allow duplicates, just like it is done right here, on Stack Overflow.

But if you want the username to be unique, obviously it can be done in one query, without any num rows functionality which being essentially useless

$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$sql = "SELECT username, email AS count FROM `user` WHERE username=? OR email=?";
$stmt = $conn->prepare($sql);
$stmt->execute([$username, $email]);
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
    if ($row['username'] === $username) {
        $errors[] = "Username is taken";
    }
    if ($row['email'] === $email) {
        $errors[] = "Email is taken";
    }
}
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345