0

I am trying to test a simple user account registration page for a project for class, and I am trying to create a check that will notify the user if their email is already in the database, and therefore will not add them to it again. Here's my PHP code.

    <?php
$collegeid = mysql_real_escape_string('1');
$email = mysql_real_escape_string('abc@test.com');
$password = mysql_real_escape_string(md5('test1'));
$name = mysql_real_escape_string('Test Test');
$bday = mysql_real_escape_string('1900-01-01');
$class = mysql_real_escape_string('Freshman');
//echo "<p>Test</p>";
$servername = "localhost";
$username = redacted;
$serverpassword = redacted;
$dbname = redacted;

$conn = new mysqli($servername, $username, $serverpassword, $dbname);

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

$checkquery = "SELECT * FROM Student WHERE Email = '$email'";
$insertquery = "INSERT INTO Student (CollegeID, Name, Birthday, Email, Classification, Password) VALUES ('$collegeid', '$name', '$bday', '$email', '$class', '$password')";

if (mysql_num_rows($conn->query($checkquery)) > 0)
{
    echo "Error: Email already in use";
}

else
{
    $conn->query($insertquery);
    echo "Account Created.";
}

?>

However, it always tells me the account is created, regardless of whether or not that user is in the database.

TZHX
  • 5,291
  • 15
  • 47
  • 56
user3246167
  • 129
  • 1
  • 15
  • Are you sure the query succeeds, you would generally check that as well. – adeneo Apr 20 '15 at 16:47
  • Please, [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared statements](http://en.wikipedia.org/wiki/Prepared_statement) instead, and use [PDO](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Apr 20 '15 at 16:47
  • What is going on in your code? Your escaping strings that you set with mysql_, using a mysql_num_rows function but using mysqli? It is all over the place. Use mysqli prepared statements and the proper mysqli functions. – Devon Bessemer Apr 20 '15 at 16:48
  • plus one on the prepared statements. Otherwise, try mysqli_num_rows instead. – g_uint Apr 20 '15 at 16:52
  • Upvotes? Why in the world would this question get upvotes? – Jay Blanchard Apr 20 '15 at 16:56

1 Answers1

4

You are mixing mysql and mysqli functions. You should not use mysql functions as they are deprecated and you seem to be using mysqli for almost everything except escaping your values and checking the number of found rows.

The problem is caused by your use of mysql_real_escape_string. When no mysql_* database connection is found, that returns false which is the equivalent of an empty string so you are checking for empty values in your database and everytime you don't find that, you add a new row.

To secure yourself against sql injection on mysqli, you should switch to prepared statements instead of using mysqli_real_escape_string.

Edit: It is also mysql_num_rows that is returning false in case of an error...

jeroen
  • 91,079
  • 21
  • 114
  • 132
  • 1
    Good answer - might I add : Consider implementing a unique key for email if you'd only like that value to exist once in a table - http://www.w3schools.com/sql/sql_unique.asp (whether this is a primary key or not setting it to unique would be useful in this instance) – Mattamorphic Apr 20 '15 at 16:51
  • @MattBarber That would be a useful addition. If the errors are handled properly of course. – jeroen Apr 20 '15 at 16:53