0

I just recently asked a question and got my code fixed to the one below:

{ 
//Check if user already exists 
$un_check = mysql_query("SELECT Username FROM users WHERE Username = '$un'");  
if(mysql_num_rows($un_check) >0) {
echo "Username already exists";
}
else{
 // Username Free
}

And when signing up it states that "username already exists" however, it still allows me to create the account anyway and it adds the same information to the database.

Is this a problem with my code or the database?

user2517092
  • 35
  • 1
  • 6
  • 3
    Your code is vulnerable to SQL Injection Attacks! – Adam Bliss Jun 25 '13 at 13:58
  • 2
    *Obligatory:* The `mysql_*` functions will be [deprecated in PHP 5.5](http://php.net/manual/en/faq.databases.php#faq.databases.mysql.deprecated). It is not recommended for writing new code as it will be removed in the future. Instead, either the [MySQLi](http://php.net/manual/en/book.mysqli.php) or [PDO](http://php.net/manual/en/book.pdo.php) and [be a better PHP Developer](http://jason.pureconcepts.net/2012/08/better-php-developer/). – Jason McCreary Jun 25 '13 at 14:01
  • This is a check, and you need to code what should happen based on the outcome of this check. We cannot write code upto your requirements here – Stoleg Jun 25 '13 at 14:03
  • I am aware of this and I will fix it. – user2517092 Jun 25 '13 at 14:04
  • @user2517092 — *Fix existing code. Throw existing code away. Rewrite code using the correct API.* is not an efficient approach to programming! – Quentin Jun 25 '13 at 14:08
  • When you get [an answer](http://stackoverflow.com/questions/17298295/database-not-checking-if-username-is-free), please do not post exactly the same code over again. – tadman Jun 25 '13 at 14:25

6 Answers6

3

If you want to forbid entering the same values twice in a table create a unique index.

Checking for an existent entry is one thing - prohibiting that another row with same values can be inserted is another thing.

Adding such an index works like this:

ALTER TABLE `users` ADD UNIQUE `MY_UNIQUE_INDEX` ( `username` ) 
conceptdeluxe
  • 3,753
  • 3
  • 25
  • 29
1

You can simply put the mysql query code that adds a user to the database inside the else block. This way, you will never insert into the database if the user already exists.

jh314
  • 27,144
  • 16
  • 62
  • 82
0

Possible cases

  • Username is not unique in your database.

  • (If you don't want to change your table structure) Put the insert part of the code inside else statement.

    if(mysql_num_rows($un_check) >0) { echo "Username already exists"; } else{ // insert new username }

BTW don't use mysql_ functions. DEPRECATED

Bere
  • 1,627
  • 2
  • 16
  • 22
0

Another way is to murder the script. I mean, use the die(); function. This stops the script wherever you place it. You would want to insert it like this:

//Check if user already exists 
$un_check = mysql_query("SELECT Username FROM users WHERE Username = '$un'");  
if(mysql_num_rows($un_check) >0) {
    echo "Username already exists";
    die(); // Don't continue, as we don't want to insert a username
}
else{
    // Username Free
}

Although this would work, if there is any other code that you still want to execute regardless of whether the username exists or not, simply place the code that inserts your users inside the else{} block as others have suggested.

SixteenStudio
  • 1,016
  • 10
  • 24
  • Am I missing something or is he always checking the same user with the name $un instead of checking the content of the variable $un ? – Bun Jun 25 '13 at 15:44
  • PHP converts variables placed inside double quotes to their variable content, if that's what you're confused about. $un = 'john'; $un_check = mysql_query("SELECT username FROM users WHERE Username = '$un'"); will translate to **SELECT username FROM users WHERE Username = 'john'**, whereas with only single quotes, it would run the query as **SELECT username FROM users WHERE Username = '$un'**. – SixteenStudio Jun 25 '13 at 17:05
0

The problem is two fold.

First, strictly from the data storage - aka database - point of view, a problem is from poorly executed database design. If the username is a field that needs to be unique, then that should be declared in the database by adding a unique index to the username column. This creates the proper database design so no a new record cannot be added if the username value already exists in the table - hence the unique index.

Sencond, your code that is checking if the username exists. Is it still creating the account after you check the database for duplicates or are you just saying that you can duplicate usernames manually in the database? If the codes is still duplicating the user, then it could be because the result is an empty set - ie no results cuz no username exists and so it won't return number of rows so change > to >=.

Robert DeBoer
  • 1,715
  • 2
  • 16
  • 26
-1

I was Passed through the same problem and my solution was this

<?php
//Connection Script Start
$mysql_host = "localhost";
$mysql_user = "root";
$mysql_password = "*******";
$mysql_database = "db_name";
$connect = mysqli_connect($mysql_host, $mysql_user, $mysql_password, $mysql_database);
//Connection Script Ends

$un = "userabc";
$search = "SELECT * FROM table_name WHERE username='$un'";
$query = mysqli_query($connect, $search);
$i = mysqli_num_rows($query);
if($i==0){
//username free
}else{
echo "This username is already taken";
}
?>
  • 2
    Reckless disregard for [proper escaping practices](http://bobby-tables.com/php) will get you into trouble with this approach. Also, please **do not** use `mysql_query` in new code. It's deprecated and will be removed in future versions of PHP. – tadman Jun 25 '13 at 14:23
  • Niraj: Even with mysqli_query you need to escape as your code is vulnerable to a trivial SQL injection attack. If you want to try it, set up a database, and pass in `x' ; DROP TABLE table_name -- `, causing unescaped SQL to be run and deleting your table. – nanofarad Jun 26 '13 at 15:02
  • The way you're using `mysqli_query` is completely wrong. You **must** use [bind_param](http://php.net/manual/en/mysqli-stmt.bind-param.php) instead of string concatenation. Your original question had `mysql_query` in it, so you've made an improvement here, but `$un` should not be in your query string. – tadman Jun 26 '13 at 17:30