Create Account Problem

[Joshua F]

Hello, I am having a problem with this code. I know it's messy, but it works.

 

The Problem is that you can register a username more then once, It detects there is already a username of that in the database, but it still inserts it, and then there would be two of the same username.

 

My Code:

<?php
    if($_SERVER['REQUEST_METHOD'] == 'POST') {
  if($_POST['username'] == "" || $_POST['password1'] == "" || $_POST['password2'] == "" || $_POST['email'] == "" || $_POST['rights'] == "" ||  $_POST['ipaddress'] == "")
  {
    echo '<p class="info" id="warning"><span class="info_inner">You left one or more fields blank.</span></p>';
  }
  else
  {
    $rr = mysql_query('SELECT * FROM users WHERE username=\'' . realEscape($_POST['username']) . '\'') ;
    if(mysql_num_rows($rr) > 0)
    {
    echo '<p class="info" id="error"><span class="info_inner">ERROR: The username is already in use!</span></p>';
    }
    $rrr = mysql_query('SELECT * FROM users WHERE email=\'' . realEscape($_POST['email1']) . '\'') ;
    if(mysql_num_rows($rrr) > 0)
    {
    echo '<p class="info" id="error"><span class="info_inner">ERROR: The email is already in use!</span></p>';
    } else {
      if($_POST['password1'] == $_POST['password2'])
      {
        if(preg_match('/[A-Za-z0-9-\s]{3,13}/i', $_POST['username'], $matches) && strlen($matches[0]) === strlen($_POST['username']))
        {
          if(preg_match('/[a-z0-9]{3,13}/i', $_POST['password1'], $matches) && strlen($matches[0]) === strlen($_POST['password1']))
          {
            if(is_numeric($_POST['rights'])) {
              mysql_query("INSERT INTO users (username, password, rights, ipaddress, email, date) VALUES ('". realEscape($_POST['username']) ."', '". encrypt($_POST['password1']) ."', '". realEscape($_POST['rights']) ."', '". realEscape($_POST['ipaddress']) ."', '". realEscape($_POST['email']) ."', NOW())");
              echo '
              <p class="info" id="info"><span class="info_inner">The account has been created.</p>';
            } else {
              echo '<p class="info" id="error"><span class="info_inner">ERROR: Undefined</span>';
            }
          }
          else
          {
            echo '<p class="info" id="error"><span class="info_inner">ERROR: Invalid password. Your password can only contain Numbers and Letters, and be 3-12 characters in length.</span></p>';
          }
        }
        else
        {
          echo '<p class="info" id="error"><span class="info_inner">ERROR: Invalid username. Your password can only contain Numbers and Letters, and be 3-12 characters in length.</span></p>';
        }
      }
      else
      {
        echo '<p class="info" id="error"><span class="info_inner">ERROR: Passwords do not match.</span></p>';
      }
    }
  }
  }
?>

[objnoob]

You can start by researching the UNIQUE property for MySQL columns. 

[Psycho]

You're right, it is messy - but it doesn't work or else you would not be here. If you write your code in a logical format it will be less error prone and will be easier to read/review. All those nested if/else statements are a bear to understand. But, that is the current problem. You simply have an IF statement for when there is a username match. After that the code continues on just the same if there wasn't a match.

 

Putting your validations in a more linear model makes more sense. Also, your code quits validating after the first error condition. While this makes sense if they have not entered all the information, why would you not alert the user to an invalid email address if the email is already in use (do you even see the illogical problem there?)

 

Do all the easy validations first - leave database validations for the last part. And, keep it all in a logical sequence

<?php
    
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
    //Parse the input values
    $username  = trim($_POST['username']);
    $password1 = trim($_POST['password1']);
    $password2 = trim($_POST['password2']);
    $email     = trim($_POST['email']);
    $rights    = trim($_POST['rights']);
    $ipaddress = trim($_POST['ipaddress']);
    $errors    = array();
    
    if(empty($username) || empty($password1) || empty($password2) || empty($email) || empty($rights) || empty($ipaddress))
    {
        $errors[] = 'You left one or more fields blank.';
    }
    else
    {
        if (preg_match('/[a-z0-9-\s]{3,13}/i', $username)==0))
        {   //Validate username format
            $errors[] = 'ERROR: Invalid username. Your username can only contain Numbers and Letters, and be 3-13 characters in length.';
        }
        if($password1 != $password2)
        {   //Validate passwords match
            $errors[] = 'ERROR: Passwords do not match.';
        }
        else if(preg_match('/[a-z0-9]{3,13}/i', $password1)==0)
        {   //Validate password format
            $errors[] = 'ERROR: Invalid password. Your password can only contain Numbers and Letters, and be 3-13 characters in length.';
        }
        if(!is_numeric($_POST['rights']))
        {   //Validate rights
            $errors[] = '<p class="info" id="error"><span class="info_inner">ERROR: Undefined rights.</span>';
        }
        if(count($errors)==0)
        {
            //Format validations passed validate unique username and email adress
            $query = "SELECT username, email
                      FROM users
                      WHERE username='".realEscape($username)."'
                         OR email='".realEscape($email)."'";
            $result = mysql_query($query);
            if(mysql_num_rows($result) > 0)
            {
                $found = mysql_fetch_assoc($result);
                if($found['username']==$username)
                {
                    $errors[] = 'ERROR: The username is already in use!';
                }
                if($found['email']==$email)
                {
                    $errors[] = 'ERROR: The email is already in use!';
                }
            }
        }
    }
    
    //Check if there were any validation errors
    if(count($errors)>0)
    {
        echo "<p class=\"info\" id=\"warning\"><span class=\"info_inner\">The following errors occured:<ul>\n"
        foreach ($errors as $error)
        {
            echo "<li>{$error}</li>";
        }
        echo "</ul></span></p>\n";
    }
    else
    {
        //No errors occured, insert record
        $query = "INSERT INTO users (username, password, rights, ipaddress, email, date)
                  VALUES ('".realEscape($username)."', '".encrypt($password1)."', '".realEscape($rights)."',
                          '".realEscape($ipaddress)."', '". realEscape($email)."', NOW())";
        mysql_query($query);
        echo '<p class="info" id="info"><span class="info_inner">The account has been created.</p>';
    }
}
?>

[Joshua F]

You're right, it is messy - but it doesn't work or else you would not be here. If you write your code in a logical format it will be less error prone and will be easier to read/review. All those nested if/else statements are a bear to understand. But, that is the current problem. You simply have an IF statement for when there is a username match. After that the code continues on just the same if there wasn't a match.

 

Putting your validations in a more linear model makes more sense. Also, your code quits validating after the first error condition. While this makes sense if they have not entered all the information, why would you not alert the user to an invalid email address if the email is already in use (do you even see the illogical problem there?)

 

Do all the easy validations first - leave database validations for the last part. And, keep it all in a logical sequence

<?php
    
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
    //Parse the input values
    $username  = trim($_POST['username']);
    $password1 = trim($_POST['password1']);
    $password2 = trim($_POST['password2']);
    $email     = trim($_POST['email']);
    $rights    = trim($_POST['rights']);
    $ipaddress = trim($_POST['ipaddress']);
    $errors    = array();
    
    if(empty($username) || empty($password1) || empty($password2) || empty($email) || empty($rights) || empty($ipaddress))
    {
        $errors[] = 'You left one or more fields blank.';
    }
    else
    {
        if (preg_match('/[a-z0-9-\s]{3,13}/i', $username)==0))
        {   //Validate username format
            $errors[] = 'ERROR: Invalid username. Your username can only contain Numbers and Letters, and be 3-13 characters in length.';
        }
        if($password1 != $password2)
        {   //Validate passwords match
            $errors[] = 'ERROR: Passwords do not match.';
        }
        else if(preg_match('/[a-z0-9]{3,13}/i', $password1)==0)
        {   //Validate password format
            $errors[] = 'ERROR: Invalid password. Your password can only contain Numbers and Letters, and be 3-13 characters in length.';
        }
        if(!is_numeric($_POST['rights']))
        {   //Validate rights
            $errors[] = '<p class="info" id="error"><span class="info_inner">ERROR: Undefined rights.</span>';
        }
        if(count($errors)==0)
        {
            //Format validations passed validate unique username and email adress
            $query = "SELECT username, email
                      FROM users
                      WHERE username='".realEscape($username)."'
                         OR email='".realEscape($email)."'";
            $result = mysql_query($query);
            if(mysql_num_rows($result) > 0)
            {
                $found = mysql_fetch_assoc($result);
                if($found['username']==$username)
                {
                    $errors[] = 'ERROR: The username is already in use!';
                }
                if($found['email']==$email)
                {
                    $errors[] = 'ERROR: The email is already in use!';
                }
            }
        }
    }
    
    //Check if there were any validation errors
    if(count($errors)>0)
    {
        echo "<p class=\"info\" id=\"warning\"><span class=\"info_inner\">The following errors occured:<ul>\n"
        foreach ($errors as $error)
        {
            echo "<li>{$error}</li>";
        }
        echo "</ul></span></p>\n";
    }
    else
    {
        //No errors occured, insert record
        $query = "INSERT INTO users (username, password, rights, ipaddress, email, date)
                  VALUES ('".realEscape($username)."', '".encrypt($password1)."', '".realEscape($rights)."',
                          '".realEscape($ipaddress)."', '". realEscape($email)."', NOW())";
        mysql_query($query);
        echo '<p class="info" id="info"><span class="info_inner">The account has been created.</p>';
    }
}
?>

 

Sorry, i've been sick. I added the code, and got this error.

 

Parse error: syntax error, unexpected ')' in C:\xampp\htdocs\Mpz Scripts\admin\users1.php on line 140

 

Line 140:

        if (preg_match('/[a-z0-9-\s]{3,13}/i', $username)==0))

[Rifts]

specifying what line 140 is would help

[Joshua F]

I did.

[wizr]

 

Sorry, i've been sick. I added the code, and got this error.

 

Parse error: syntax error, unexpected ')' in C:\xampp\htdocs\Mpz Scripts\admin\users1.php on line 140

 

Line 140:

        if (preg_match('/[a-z0-9-\s]{3,13}/i', $username)==0))

 

I don't know much but I'm trying to help, isn't there one to many ) on the very end? Try:

 

        if (preg_match('/[a-z0-9-\s]{3,13}/i', $username)==0)

[Psycho]

As my signature states, I do not always test the code I provide. Code I provide is to be considered a "guide" in completing your code, not somethign you can simply drop in. I also expect people to be able to debug simple errors such as this. In this case, just remove the ")" at the end of that line.

[Joshua F]

As my signature states, I do not always test the code I provide. Code I provide is to be considered a "guide" in completing your code, not somethign you can simply drop in. I also expect people to be able to debug simple errors such as this. In this case, just remove the ")" at the end of that line.

 

I was tired when I posted that error. I figured it out.