PDO user verification

Question

The count($existuser) function always returns 1. Even if there is not a user with that name or email.

Here is the code:

function registerUser($username, $password, $passwordagain, $email, $mcname) {
    include $_SERVER['DOCUMENT_ROOT'] . "/config/config.php";
    $conn          = new PDO('mysql:host=' . $ip . ';dbname=' . $database, $username, $password);
    $validusername = "/^[a-z0-9]+$/";
    $validpassword = "/^[A-Za-z0-9]+$/";
    $validemail    = "/^([a-zA-Z0-9_\.\-])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/";
    $validmcname   = "/^[A-Za-z0-9]+$/";
    $error         = 0;

    if (strlen($username) < 4 || strlen($username) > 24) {
        $error = 1;
    }
    if (strlen($password) < 6 || strlen($password) > 24) {
        $error = 1;
    }
    if (strlen($mcname) < 4 || strlen($mcname) > 24) {
        $error = 1;
    }
    if (!preg_match($validusername, $username)) {
        $error = 1;
    }
    if (!preg_match($validpassword, $password)) {
        $error = 1;
    }
    if (!preg_match($validemail, $email)) {
        $error = 1;
    }
    if (!preg_match($validmcname, $mcname)) {
        $error = 1;
    }
    if ($password != $passwordagain) {
        $error = 1;
    }
//test
    $userquery = $conn->query('SELECT * FROM users WHERE username="' . $username . '"');
    $existuser = $userquery->fetch();
    echo count($existuser);
//test
    if (count($existuser)) {
        $error = 1;
        echo "<div class='erroralert'>Username already exists!</div>";
    }
//test
    $emailquery = $conn->query('SELECT * FROM users WHERE email="' . $email . '"');
    $existemail = $emailquery->fetch();
//test
    if (count($existemail)) {
        $error = 1;
        echo "<div class='erroralert'>E-mail already exists!</div>";
    }
    if ($error != 1) {
        $encryptedpassword = hash('sha512', $password);
        $registeruser      = $conn->query("INSERT INTO users(username, password, email, mcname) VALUES ('$username', '$encryptedpassword', '$email', '$mcname')");
        echo "<div class='successalert'>Succesfully registred</div>";
    }
}

I do not see your query posted, and it'll also be good for your debugging process to change your error status so you can identify what is wrong with what you/the user is inputting into the function. — Daryl Gill, Dec 23 '12 at 15:58
Please see: http://php.net/faq.passwords.php - also you can put the length constrains for validation into the regular expressions (you already do this with the email regex) and you should use prepared statements here because you don't handle the validation results. — hakre, Dec 23 '12 at 16:57

score 1 · Answer 1 · answered Dec 23 '12 at 16:00

1

Why are you using PDO if you do this:

$userquery = $conn->query('SELECT * FROM users WHERE username="' . $username . '"');
$existuser = $userquery->fetch();

You should have the following logic:

$userquery = $conn->prepare('SELECT * FROM users WHERE username = ?');
$userquery->execute(array($username));
if ($userquery->rowCount()) {
    // found user
} else {
    // user not found
}

answered Dec 23 '12 at 16:00

Ranty

3,333
3
22
24

Mind that `SELECT` and `PDOStatement::rowCount()` do not go well together, see http://stackoverflow.com/a/14012410/367456 – hakre Dec 23 '12 at 16:02
@hakre I agree that on portable applications it's valid, but if you are not making one, you do two queries instead of one. – Ranty Dec 23 '12 at 16:05
Oh, well for Mysql, you can do it this way, right: [Work-around for PHP5's PDO rowCount MySQL issue](http://stackoverflow.com/q/460010/367456) - However that is *not* what you suggest in your answer. – hakre Dec 23 '12 at 16:07
@hakre I guess I'm not making it clear enough. I'm not saying he should use `rowCount()` to get *exact amount of rows affected by select*. I'm saying if it doesn't return 0, there are rows. Try it. The amount is wrong, but it's not 0. And if there are no rows, it's 0. Correct me if I'm wrong. – Ranty Dec 23 '12 at 16:11
I'm just saying that it does not work that way on every system with Mysql. That's all. I also updated my answer to highlight the concrete cause of the issue and the fix is rather trivial (you have a good point here with the prepared statement). – hakre Dec 23 '12 at 16:21

score 0 · Answer 2 · answered Dec 23 '12 at 15:20

The query is probably failing to execute, and that is why you are always getting "1", that is probably the error report.

Reading through your code, I suggest you to try this:

In the following query, you are enclosing the username by ", however, this isn't always supported, but instead you should use single quote ' and enclose the string itself with ". So the following line

$userquery = $conn->query('SELECT * FROM users WHERE username="' . $username . '"');

should be

$userquery = $conn->query("SELECT * FROM users WHERE username='" . $username . "'");

do the same to other queries that have the same problem.

hakre · Answer 3 · 2012-12-23T16:20:19.583

You are using count() wrong here. It will return 1 for any normal variable and for an array, it will return the number of elements.

The important part is the first one. If there is no row, PDOStatement::fetch() will return FALSE, which counts 1 which is truthy:

count(FALSE);      # 1
count($existuser); # 1 when there is no user, when there is a user at least 2
                   #   for default fetchmode PDO::FETCH_BOTH

So you have just checked nothing. Instead test that it is not FALSE:

if ($existuser === FALSE) {
   // error.
}

PDO user verification

3 Answers3