1

I have this line in my registration page.

if (device_id_exists($_POST['device_id']) == true) {
           $errors[] = 'Sorry the Serial Number \'' . htmlentities($_POST['device_id']) . '\' does not exist.';
    }

I have this in my function page.

function device_id_exists($device_id) {
   $device_id = sanitize($device_id);
   $query = mysql_query("SELECT COUNT(`numbers`) FROM `devicenumbers` WHERE `numbers` = '$numbers'");
   return (mysql_result($query, 0) == 0) ? true : false;

If I run this query SELECT COUNT(numbers) FROMdevicenumbersWHEREnumbers= '1234567890' (a valid number) it will return 1 = match found right? If I put a bogus number it returns a '0'. What is happening is when there is a valid number it still returns the error the number doesn't exist. If I change it to the result to == 1 it will submit any number? Im a newbie to DB calls any help appreciated. I hope I provided enough info.

John Conde
  • 217,595
  • 99
  • 455
  • 496
sectech
  • 43
  • 1
  • 8
  • in device_id_exists() where does $numbers come from? should it be $device_id? – Waygood Mar 25 '13 at 17:29
  • unrelated: `return (mysql_result($query, 0) == 0) ? true : false;` can be reduced to `return (mysql_result($query, 0) == 0);` – scones Mar 25 '13 at 17:30
  • IRRELEVANT: read [this](http://stackoverflow.com/questions/129677/whats-the-best-method-for-sanitizing-user-input-with-php) about sanitation. – cygorx Mar 25 '13 at 17:34
  • 1
    [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://j.mp/PoWehJ). – h2ooooooo Mar 25 '13 at 17:41
  • Thanks scones, I did have that earier but obviously having the wrong value wasn't helping. I did shorten and it works great! thanks1 – sectech Mar 25 '13 at 17:46

2 Answers2

2

Looks like you're calling the incorrect variable. Within the device_id_exists() function, you're accepting a variable named $device_id. However when you're performing the query, you're calling what appears to be an undefined variable: $numbers. I suspect $numbers should be renamed to $device_id.

I see your $device_id comes from a form post. I'd HIGHLY recommend you escape the variable, using mysql_real_escape_string() to ensure you are protected against SQL injection. Please note that sanitize() does NOT protect against SQL injection!

On one additional note, I'd recommend utilizng mysql_num_rows() rather than mysql_result() because mysql_result() actually asks the database server to return an actual result when all you really care about is whether the entry exists or not, not it's actual value.

function device_id_exists($device_id) {
  $device_id = sanitize($device_id);
  $device_id = mysql_real_escape_string($device_id);
  $query = mysql_query("SELECT COUNT(`numbers`) FROM `devicenumbers` WHERE `numbers` = '$device_id'");
  return mysql_num_rows($query) ? True : False;
}
Joshua Burns
  • 8,268
  • 4
  • 48
  • 61
  • This is what happens when your up until 1:30 working on registration/login pages :) Thank you! I for some reason kept over looking that and thinking its something with the true/false or 0/1. – sectech Mar 25 '13 at 17:42
  • i know that feeling well! while developing i'd recommend turning on php's display errors and error reporting to easily catch these types of scenarios. – Joshua Burns Mar 25 '13 at 17:44
  • Is there a way I can force it to be numbers the user posts? so I know there will not be any letters and I know it will always be under 40. Its not important right now but would like that control. – sectech Mar 25 '13 at 17:59
  • sure, instead of `return mysql_num_rows($query) ? True : False;` just `return mysql_num_rows($query);` – Joshua Burns Mar 25 '13 at 20:05
0

I had a similar problem with mysql result set , It returns nums_rows == 1 even when there are no records (while using max() inside select query - In your case you have used count())... Instead of checking mysqlquery to 0, check it whether the result set empty (That's how i solved my problem).. eg. if(!empty(mysql_result($query))) ? true : false;

user170851
  • 385
  • 5
  • 6