Comparison code failing

[Landslyde]

Hello everyone:

 

I'm having trouble with code that compares a Form value ($uname) to Field values (username) from my database. In testing, I'm using the same Form data over and over, and I now have several identical records instead of just one unique record for this user.

  if (empty($_POST["uname"])) {
    $unameErr = "* Username is required";

  } else {
    $uname = test_input($_POST["uname"]);
    if (!preg_match("/^[a-zA-Z0-9]*$/",$uname)) {
      $unameErr = "* Only letters and numerals are allowed";

  } else {
      // Now sure the username is legit, we check to see if it's a
      // unique username by comparing it to all usernames already in member table.

      require_once 'login.php'; // This file contains database access credentials

      $db_conn = new mysqli($db_hostname, $db_username, $db_password, 'login');
      if($db_conn->connect_error) die ('Connect Error: ('.$db_conn->connect_errno.')'.$db_conn->connect_error);

      $query = "select username from member"; // Only selecting the field to compare to
      $result = $db_conn->query($query);
      if(!$result) die ('Database access failed: ('.$db_conn->connect_errno.')'.$db_conn->connect_error);	    

      $rows = $result->num_rows;
      for($i = 0; $i <= $rows; $i++) {
        if(mysqli_fetch_assoc($result) == $uname) { 
	$unameErr = "* Username already in use. Please choose another.";
	mysql_close($db_conn);
	exit;			
        }
     }
     $query = "insert into member values(NULL, '$uname', '$pwd1', '$fname', '$lname')";
     $result = $db_conn->query($query);
     if(!$result) die ("Database insertion failed: ".mysql_error());
    }
  }

This is my first attempt at this using PHP and am pleased that I can at least access my db. But I just can't figure out how to make this comparison check.

 

Thanks in advance for any help you offer.

 

~Landslyde

[Monkuar]

It's because

$query = "select username from member";

Isn't checking against a username, you need to specify WHERE username = ?

 

I'm sure that's why you havn't had $unameErr = "* Username already in use. Please choose another."; firing and you're receiving duplicates?

 

New code: (I updated the new query = WHERE username and added mysql_real_escape_string to it. I also tabbed the code a bit to make it look nice. Let me know how this goes:

<?php  
	
	if (empty($_POST["uname"])) {
		$unameErr = "* Username is required";
		
		} else {
		$uname = test_input($_POST["uname"]);
		if (!preg_match("/^[a-zA-Z0-9]*$/",$uname)) {
			$unameErr = "* Only letters and numerals are allowed";
			
			} else {
			// Now sure the username is legit, we check to see if it's a
			// unique username by comparing it to all usernames already in member table.
			
			require_once 'login.php'; // This file contains database access credentials
			
			$db_conn = new mysqli($db_hostname, $db_username, $db_password, 'login');
			if($db_conn->connect_error) die ('Connect Error: ('.$db_conn->connect_errno.')'.$db_conn->connect_error);
			
			$query = 'select username from member WHERE username = \''.mysql_real_escape_string($uname).'\' LIMIT 1'; // Only selecting the field to compare to
			$result = $db_conn->query($query);
			if(!$result) die ('Database access failed: ('.$db_conn->connect_errno.')'.$db_conn->connect_error);	    
			
			$rows = $result->num_rows;
			for($i = 0; $i <= $rows; $i++) {
				if(mysqli_fetch_assoc($result) == $uname) { 
					$unameErr = "* Username already in use. Please choose another.";
					mysql_close($db_conn);
					exit;			
				}
			}
				if (empty($unameErr)){ // Make sure unameErr is empty (no errors) before submitting the data.
					$query = "insert into member values(NULL, '$uname', '$pwd1', '$fname', '$lname')";
					$result = $db_conn->query($query);
					if(!$result) die ("Database insertion failed: ".mysql_error());
				}
		}
	}
	
?>

.. and Shitty IPB forum software doesn't save tabbed code, what a joke. Oh well. Sorry >_>

 

Edit: Oh it does save it when you turn off the editing feature.. lol

Edited January 27, 2015 by Monkuar

[Landslyde]

Thanks, Monkuar. I was about to edit my post because I finally woke up and realized that I needed to use WHERE in my Query only to come here and see you had already made the changes. I'm bad about that, getting tunnel vision, not seeing the trees for the forest. Thank you for your time and patience with an old man who obviously didn't drink enough Geritol today! And thanks for the tabbing...looks like a million bucks

 

~Landslyde

Edited January 27, 2015 by Landslyde

[kicken]

Checking with a select query first is the wrong way to handle this in the first place. What you should be doing is setting a UNIQUE constraint on your database, then check if the insert failed due to a violation of that constraint.

 

A quick example would be:

<?php

$db = new PDO('mysql:host=localhost;dbname=test', 'username', 'password');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

/* //Uncomment to create the table 
$sql = '
create table unique_example (
 id int auto_increment primary key not null
 , username varchar(100) unique not null
)
';
$db->exec($sql);
*/

$username = 'example';
try {
    $stmt = $db->prepare('INSERT INTO unique_example (username) VALUES (?)');
    $stmt->execute([$username]);
    echo 'Data inserted successfully';
}
catch (PDOException $ex){
    if ($ex->getCode() == 23000){ //Check if it's a duplciate key violation.
        echo 'Username must be unique';
    }
    else {
        throw $ex;  //Rethrow other errors
    }
}

Notice when the table is created (in the commented code) that I specify UNIQUE in the column attributes. This tells Mysql to create a unique index on that column. Whenever new data is inserted or existing data changed mysql will verify that the new value does not already exist. If the value does already exist it will reject the query with an error.

 

 

The problem with your select-then-insert approach is that it is possible that someone else inserts the duplicate name after your select but before your insert. Eg:

Client A: SELECT 1 FROM table WHERE username='blah';

Client B: SELECT 1 FROM table WHERE username='blah';

Client A: INSERT INTO table (username) VALUES ('blah'); => Ok, data added

Client B: INSERT INTO table (username) VALUES ('blah'); => Ok, data added

 

Now you have two rows with the same data despite your check.

[Landslyde]

Very insightful, kicken. I'm just a few days into PHP, but not to programming. I'll study your code and try to learn what I can on the use of PDO in PHP. Although I refined my code:

$query = "select username from member WHERE username = '".mysqli_real_escape_string($uname)."'"; // Only selecting the field to compare to
$result = $db_conn->query($query);
if(!$result) die ('Database access failed: ('.$db_conn->connect_errno.')'.$db_conn->connect_error);	    
$rows = $result->num_rows;
$i = 0; 
if($rows > $i) {
   $unameErr = "* Username already in use. Please choose another.";
   mysql_close($db_conn);
   exit;			
}

to simply check if a row had been returned, the Query is still wrong somehow. I'm still able to create the same username over and over. But what you show me is interesting, cleaner looking. Funny how we want what we want yesterday and today's already too late. Time to slow down and get to know PHP a little better. Thanks for your explanation and code example. It won't go to waste.

 

~Landslyde

Edited January 27, 2015 by Landslyde

[QuickOldCar]

I always trim and lower user names as well as doing a unique on them in the database.

If are saving then as a *_ci collation in mysql is case insensitive

If retrieve them from a database or doing any sort of comparisons should lower them anyway.

Maybe is a force of habit or just my way of coding, trying to make it always work than the chance of failing somewhere.