2

I'm trying to execute a MySQL query in PHP but I am having some problems. It appears that when running this query in PHP it always returns 0 rows, but running this in my console works as expected.

<?php
include '../mysql.php';
$name = $_POST["name"];
$email = $_POST["email"];
$sql = "SELECT COUNT(*) FROM accounts WHERE name = '" . $name . "' AND email = '" . $email . "'";
echo $sql . "</br>";
$res = mysqli_query($connection, $sql);
if(!$res) {
    die("Query Failed!");
}
$row_cnt = $result->num_rows;
printf("Result set has %d rows.</br>", $row_cnt);
if($row_cnt > 0) {
    echo "Account exists!";
} else {
    echo "Account does not exist! Creating...</br>";
    if(mysqli_query($connection, "INSERT INTO accounts (`name`, `email`, `password`, `ip`) VALUES ('" . $name . "', '" . $email . "', 'abc123', 'localhost')")) {
        echo "Created account!";
    } else {
        echo "Failed to create account";
    }
}
mysqli_free_result($res);
mysql_close($connection);

The output:

SELECT COUNT(*) FROM accounts WHERE name = 'testUser' AND email = 'abc123@gmail.com'
Result set has 0 rows.
Account does not exist! Creating...
Created account!

Then in the table we see the expected:

mysql> select * from accounts;
+----+----------+------------------+----------+-----------+
| id | name     | email            | password | ip        |
+----+----------+------------------+----------+-----------+
| 10 | testUser | abc123@gmail.com | abc123   | localhost |
+----+----------+------------------+----------+-----------+
1 row in set (0.00 sec)

But when the query is ran again it has the same exact output and another row is created.

mysql> select * from accounts;
+----+----------+------------------+----------+-----------+
| id | name     | email            | password | ip        |
+----+----------+------------------+----------+-----------+
| 10 | testUser | abc123@gmail.com | abc123   | localhost |
| 11 | testUser | abc123@gmail.com | abc123   | localhost |
+----+----------+------------------+----------+-----------+
2 rows in set (0.00 sec)

Any ideas on why it cannot detect that the data exists in a row already?

gen_Eric
  • 223,194
  • 41
  • 299
  • 337
  • You're using `SELECT count(*)` then checking the row count, this will never work. You need to get the value of the first (count) column which will be the number of rows matching your conditions. – Devon Bessemer Jun 14 '16 at 13:44
  • imo, since you are certain to get one row back with a count it. Then just fetch it anyway. The `num_rows` count tells you nothing useful. It rarely tells you anything useful since you want the data and then process it. Assume you will get some data. So always fetch it. You test for the end of it anyway. i.e. `num_rows` is rarely useful when used with `select` queries. imo. – Ryan Vincent Jun 14 '16 at 14:06
  • Hash your passwords if you care about your data. Protect from sql injection. Some apps are throw-away apps, sure, and they don't count. – Drew Jun 14 '16 at 14:25
  • **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use string interpolation or concatenation to accomplish this because you have created a severe [SQL injection bug](http://bobby-tables.com/). **NEVER** put `$_POST` or `$_GET` data directly into a query, it can be very harmful if someone seeks to exploit your mistake. – tadman Jun 14 '16 at 14:57
  • **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.2/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and never store passwords as plain-text. – tadman Jun 14 '16 at 14:57

1 Answers1

4

You have two different variables, $result and $res. Change this:

$row_cnt = $result->num_rows;

To:

$row_cnt = $res->num_rows;

UPDATE:

Also change this:

$sql = "SELECT COUNT(*) FROM accounts WHERE name = '" . $name . "' AND email = '" . $email . "'";

To:

$sql = "SELECT * FROM accounts WHERE name = '" . $name . "' AND email = '" . $email . "'";

Otherwise your $res will always be equal to one and so application will not behave as expected. – Devon

UPDATE #2:

Close connection with

mysqli_close($connection); 

Instead

mysql_close($connection);

Don't mix these two!! – Saty

UPDATE #3

Read about cleaning PHP variables to protect your application. One good post can be found here and plenty more available on google.

Community
  • 1
  • 1
K.I
  • 568
  • 1
  • 6
  • 19
  • Except, because of using the aggregate function count in `SELECT count(*)` this will always return 1 whether there are 0 rows matching the conditions or 1000 rows matching the conditions. – Devon Bessemer Jun 14 '16 at 13:47
  • There are 2 problems, but this is definitely an answer to one of them. – Jonnix Jun 14 '16 at 13:49
  • No, it will not... The row count will always be 1 when doing select count without grouping, regardless of the query. – Devon Bessemer Jun 14 '16 at 13:50
  • @K.I I think you're missing what Devon is pointing out. SELECT COUNT will _always_ return at least 1 row. – Jonnix Jun 14 '16 at 13:51
  • Oh yes, sorry! Stupid me! It will always be = 1. Updated my answer. – K.I Jun 14 '16 at 13:53
  • Add one more thing close connection with `mysqli_close($connection);` instead `mysql_close($connection);` don't mix these two!! – Saty Jun 14 '16 at 13:58
  • Plus also add some information about sql injection!! – Saty Jun 14 '16 at 14:03