php and mysql_num_rows not detecting if something isnt in the database

Question

I have a form on a website that needs validating before entering the form data into the database.

Its checking whether a username already exists by user the mysql_num_rows function. But i cannot seem to get it working. When testing, it doesn't let a new username be added.

Here is the full code being used:

<?php
session_start();

include("databaseConnect.php");
// Insert a row of information into the table "example"


// check if username is already in database
if(mysql_num_rows(mysql_query("SELECT userName FROM registeredUsers WHERE userName =     '$_POST[userName]'"))){
 echo "Username: ". $_POST[userName]." already exists in the Database<br><br>";
    echo "You will be redirect back to the form in 5 seconds";
$ref = $_SERVER['HTTP_REFERER'];
header( 'refresh: 5; url='.$ref);

//check if hemis is already in database
}elseif(mysql_num_rows(mysql_query("SELECT hemis FROM registeredUsers WHERE hemis = '$_POST[hemis]'"))){
echo "Student [Hemis] Number: ". $_POST[hemis]." already exists in the Database<br><br>";
echo "You will be redirect back to the form in 5 seconds";
$ref = $_SERVER['HTTP_REFERER'];
header( 'refresh: 5; url='.$ref);


// if all the conditions above are fine, it will insert the data to MySQL
}else{
  mysql_query("INSERT INTO registeredUsers
(firstName, lastName, hemis, userName, MAC) VALUES('$_POST[firstName]', '$_POST[lastName]', '$_POST[hemis]', '$_POST[userName]', '$_POST[mac]' ) ")
or die(mysql_error());

echo "Data Inserted! <br><br>";
}

Thanks a lot :)

haha thats good - but doesnt help too much lol - This is a system in development, so there are not supposed to be any student records in the table until they register with this online form - but isnt recognising, with this code, if a user name doesnt exist... — Ryan Jones, May 29 '11 at 11:03
That's not a self-contained code snippet; you have misplaced the `}` somewhere. Check if that's not the problem. — Piskvor left the building, May 29 '11 at 11:13
Note also that `HTTP_REFERER` is rather unreliable, you *will* be getting bug reports of an infinite redirect loop, as some browsers don't send this at all - better redirect to your form explicitly. — Piskvor left the building, May 29 '11 at 11:15
Thankyou - i will change the HTTP_REFERER - but at this stage, that isnt causing the problem as far as i know. Ive added a larger code snippet so you can get a better idea of what is happening — Ryan Jones, May 29 '11 at 11:20

Matty · Accepted Answer · 2011-05-29T11:48:07.740

I'd rewrite this completely. It's subject to SQL injection, inefficient and a little too terse. Plus, you're generally better off using the PHP mysqli extension

Also, make sure you enclose the $_POST variable names in quotes. You've written them as constants, as opposed to strings. (Unless you've defined constants elsewhere in code that represent the string values, this is an error. Turn on PHP warnings while you're developing.)

$safe_username = mysqli_real_escape_string($_POST['userName']);
$sql = "SELECT userName FROM registeredUsers WHERE userName='$safe_username' LIMIT 1";
$result = mysqli_query($database_connection, $sql);
if (mysqli_num_rows($result))
{
    // username already found code
    mysqli_free_result($result);
}
else
{
    $safe_hemis = mysqli_real_escape_string($_POST['hemis']);
    $sql = "SELECT hemis FROM registeredUsers WHERE hemis='$safe_hemis' LIMIT 1";
    // Side note, LIMIT 1 tells the database engine to stop looking after it's found one hit. More efficient as you're only looking for a Boolean value anyway.
    $result = mysqli_query($database_connection, $sql);
    if (mysqli_num_rows($result))
    {
        // hemis found code
        mysqli_free_result($result);
    }
}

The rest you can probably figure out from this.

Please validate and escape all input. Validation includes checking for sanity - is the data within bounds (string length, numerical bounds, etc), etc. All input is evil!

You really don't want to depend on HTTP_REFERER either. User agents don't always pass referrers.

Also, I know it's not a big deal, but use CSS rather than <br>. If you're using the XHTML doctype, you have to properly close all tags, so <br> would become <br />. This is a good idea anyway.

+1 I'd perform some thorough refactoring too, if not starting from scratch. :) — GolezTrol, May 29 '11 at 11:55

score 1 · Answer 2 · answered May 29 '11 at 11:54

It's best to check the result of mysql_query too. It may return a result set for which you can get the number of rows, but it could return false when the query fails. In that case you will not have a result set, and mysql_count_rows will fail to. That fail you interpret as 0 rows.

Apart from all the great suggestions Matty gave you, I'd do some extra checking and strict type checking too.

if ($result = mysql_query('....') === false)
{
  die('Your query failed in the first place. Error: ' . mysql_error());
}

There are many improvements you can make (like using count in the query and such), but I think you should have at least these kinds of checks. It will help you understand what is actually going wrong instead of having to guess. It'll save you tons of debugging time wether your a beginner or an experienced programmer.

This is one thing I forgot to write in my answer. `$result` may be NULL, not "strict" false, and this should be tested for. Best way is to test for values that evaluate to true, and put the `die()` in the `else` clause. — Matty, May 29 '11 at 12:00
If you found mysql_query to return NULL, you probably found a bug. It should return a resource of false for statements that return data (like select) and it should return true or false for stamenets that don't (like update). It says nowhere in the documentation that NULL is a valid result. — GolezTrol, May 30 '11 at 06:40

php and mysql_num_rows not detecting if something isnt in the database

2 Answers2