0

I'm having a problem with the following PHP script. Specifically, the part that creates the user_id. This is part of a larger registration.php file that works fine without the section that creates the user_id.

As you can see, it's a while loop that uses a variable, $useridexits, to control the loop. It's set to true by default, so the loop runs. A random number is generated and then checked against the database. If a result is returned, the $useridexists variable is set to true and the loop continues. If no results are returned, $useridexists is set to false, so the loops stops. The number generated is then set to $userid and is then added to the database in the following section.

Here's the code:

//This section creates a new userid for each user.  

//This varible is used by the while loop and is set to true by default. 
$useridexists = true; 

//While loop to create userid and check the database to see if the userid
//already exists.  If it does, the while loop keeps going until a unique 
//user id is created.  
while($useridexists){

    // Function to create random user id number.
    function randomNumber($length) {
        $result = '';

        for($i = 0; $i < $length; $i++) {
            $result .= mt_rand(0, 9);
        }
        return $result;
    }

    // user id value created from randomNumber function. 
    $number = randomNumber(1);

    //query the database to see if the user id already exists 
    $query = "SELECT * FROM users WHERE user_id = :number";

    $query_params = array(':number' => '$number');

    try {
        // These two statements run the query against the database table. 
        $stmt   = $db->prepare($query);
        $result = $stmt->execute($query_params);
    }
    catch (PDOException $ex) {

        $response["success"] = 0;
        $response["message"] = "Failed to run query: " . $ex->getMessage();
        die(json_encode($response));
    }

    $row = $stmt->fetch();

    if ($row){
        $useridexists = true;
    }else{
        $useridexists = false; 
    }
}

$userid = $number; 

// This section adds the values to the database:
$query = "INSERT INTO users (username, password, email, firstname, lastname, user_id) VALUES ( :user, :pass, :email, :firstname, :lastname, :uid)";

//update tokens with the actual data:
$query_params = array(
    ':user' => $_POST['username'],
    ':pass' => $_POST['password'], 
    ':email' => $_POST['email'],
    ':firstname' => $_POST['firstName'], 
    ':lastname' => $_POST['lastName'],
    ':uid' => $userid
);

//run the query, and create the user
try {
    $stmt   = $db->prepare($query);
    $result = $stmt->execute($query_params);
}
catch (PDOException $ex) {
    $response["success"] = 0;
    $response["message"] = "Failed to run query: " . $ex->getMessage();
    die(json_encode($response));
}


$response["success"] = 1;
$response["message"] = "Username Successfully Added! Please log in.";
echo json_encode($response);

$email= mysql_escape_string($_POST['email']);
$username = mysql_escape_string($_POST['username']); 

If I comment out this section, everything works:

// user id value created from randomNumber function. 
$number = randomNumber(1);

//query the database to see if the user id already exists 
$query = "SELECT * FROM users WHERE user_id = :number";

$query_params = array(
    ':number' => '$number'
    );

try {
    // These two statements run the query against the database table. 
    $stmt   = $db->prepare($query);
    $result = $stmt->execute($query_params);
}
catch (PDOException $ex) {
    $response["success"] = 0;
    $response["message"] = "Failed to run query: " . $ex->getMessage();
    die(json_encode($response));
}

$row = $stmt->fetch();

if ($row){
    $useridexists = true;
}else{
    $useridexists = false; 
}

If I don't comment that section out, I don't get any errors, but nothing gets added to the database.

Everything works except the part that checks the database to see if the user_id already exists and changes the $useridexists variable to false, which should escape the while loop. When I add that, nothing gets added to the database.

BTW: I'm using a 1 digit value for testing purposes, but I'll change it to $number = randomNumber(7); once the code actually works.

Kevin Bright
  • 791
  • 1
  • 6
  • 17
  • 1
    You cannot use `mysql_escape_string()` with PDO – Jay Blanchard Jul 14 '16 at 13:53
  • 4
    Is there a specific reason why you don't use auto increment in your database? – Fairy Jul 14 '16 at 13:54
  • 1
    Why don't you just auto-increment the userid? – WillardSolutions Jul 14 '16 at 13:54
  • Seems a lot of work when an id field could be set as auto-increment and MYSQL would do it all for you – RiggsFolly Jul 14 '16 at 13:55
  • 1
    **Never store plain text passwords!** Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure you ***[don't escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. – Jay Blanchard Jul 14 '16 at 13:55
  • did you specified primary key for the table, as you mentioned you are using one single number (7) for testing. it could be generating "mysql's duplicate entry" error. – Basit Munir Jul 14 '16 at 13:57
  • 1
    `*/function randomNumber($length) {` What's with the `*/` there? You say that there's no error, but have you actually checked your error logs? – Patrick Q Jul 14 '16 at 13:59
  • Is this an exercise in logic (or in this case lack of it) OR are you actually thinking of implementing this as a real solution? To a non existant problem I might add – RiggsFolly Jul 14 '16 at 14:05
  • **This is just a silly idea!** With each user that you create you increase the likelyhood of your random number generator creating a duplicate and therefore the need to query that database more than once for each new user that registers. That means you will be using CPU cycles and database accesses for no good reason that could be used to do something useful to your application – RiggsFolly Jul 14 '16 at 14:13
  • For that matter we are all just Waiting for Godot anyway, no? – Drew Jul 14 '16 at 14:23
  • The `*/` by the `function randomNumber($lenght) was a typo. I've edited it out of the post. It was left over from when I was commenting out different parts of the code. I know not to store passwords in plain text. I will update once the code is finished and working. The reason I'm using this rather than auto-incrementing is because the user_id will be visible to users and I don't want people to know how many users I have. Also, I don't want user_id's like '00000012'. – Kevin Bright Jul 14 '16 at 14:24
  • Kevin, there are better ways of obfiscating a userid than over complicating a process. It will eventually bite you in the ???? – RiggsFolly Jul 14 '16 at 14:55
  • Any suggestions on alternative ways to obfuscate the userid? – Kevin Bright Jul 14 '16 at 15:06
  • 1
    Just start your auto increment at some random number. – Patrick Q Jul 14 '16 at 15:10
  • That's a good idea... I'll look into that. I wanted to avoid dropping the leading zeros, but I suppose I could just start at 1100011 or something so it looks somewhat random and I don't have to worry about losing leading zeros. If I use seven digits (as I planned), I will still have about 900k combinations that will work. – Kevin Bright Jul 14 '16 at 15:25

0 Answers0