Simple Registration Form Errors

Beginners - Learning PHP
#1

Hey guys, I have recently decided to start learning PHP, so I did what any sane person would do (votes are still out for this!) and I bought the PHP for dummies book, as it has exactly what I’m after to start with, a simple registration and login script.

Now, every thing seems to be working okay, except the key feature, the registering… I’ve followed every thing in the book, but it still seems to be going wrong.

Any help would be greatly appreciated, heres the page I think the issue is with:

[code]<?php

require_once(“functions.inc”);

//prevent access if user has not submitted the form
if (!isset($_POST[‘submit’])) {
die(header(“Location: registration.php”));
}

$_SESSION[‘formAttempt’] = true;

if (isset($_SESSION[‘error’])) {
unset($_SESSION[‘error’]);
}
$_SESSION[‘error’] = array();

$required = array(“username”, “email”, “password1”, “password2”, “fname”, “lname”, “mii_name”);

//Check required fields
foreach ($required as $requiredField) {
if (!isset($_POST[$requiredField]) || $_POST[$requiredField]
== “”) {
$_SESSION[‘error’][] = $requiredField . " is
required.";
}
}
//Makes sure Username is okay
if (!preg_match(’/^[\w .]+$/’,$_POST[‘username’])) {
$_SESSION[‘error’][] = “Username must be letters and numbers only”;
}
//Makes sure First Name is okay
if (!preg_match(’/^[\w .]+$/’,$_POST[‘fname’])) {
$_SESSION[‘error’][] = “First Name must be letters and numbers only”;
}
//Makes sure Last Name is okay
if (!preg_match(’/^[\w .]+$/’,$_POST[‘lname’])) {
$_SESSION[‘error’][] = “Last Name must be letters and numbers only”;
}
//Makes sure 3DS Mii Name is okay
if (!preg_match(’/^[\w .]+$/’,$_POST[‘mii_name’])) {
$_SESSION[‘error’][] = “Mii Avatar Name must be letters and numbers only”;
}
//Makes sure the email is valid
if (!filter_var($_POST[‘email’],FILTER_VALIDATE_EMAIL)) {
$_SESSION[‘error’][] = “Invalid e-mail address”;
}
//Makes sure passwords match
if ($_POST[‘password1’] != $_POST[‘password2’]) {
$_SESSION[‘error’][] = “Passwords do not match”;
}

//final disposition
if (count($_SESSION[‘error’]) > 0) {
die(header(“Location: registration.php”));
} else {
if(registerUser($_POST)) {
unset($_SESSION[‘formAttempt’]);
die(header(“Location: success.php”));
} else {
error_log(“Problem registering user: {$_POST[‘email’]}”);
$_SESSION[‘error’][] = “Problem registering account”;
die(header(“Location: registration.php”));
}
}

function registerUser($userData) {
$mysqli = new mysqli(DBHOST,DBUSER,DBPASS,DB);
if ($mysqli->connect_errno) {
error_log("Cannot connect to MySQL: " . $mysqli->connect_error);
return false;
}
$email = $mysqli->real_escape_string($_POST[‘email’]);

//Check to see if email exists
$findUser = “SELECT id from Users where email = ‘($email)’”;
$findResult = $mysqli->query($findUser);
$findrow = $findResult->fetch_assoc();
if (isset($findRow[‘id’]) && $findRow[‘id’] != “”) {
$_SESSION[‘error’][] = “A user with that e-mail already exists”;
return false;
}

//Check to see if username exists
$findUser = “SELECT id from Users where username = ‘($username)’”;
$findResult = $mysqli->query($findUser);
$findrow = $findResult->fetch_assoc();
if (isset($findRow[‘id’]) && $findRow[‘id’] != “”) {
$_SESSION[‘error’][] = “This username has already been taken”;
return false;
}

$lastName = $mysqli->real_escape_string($_POST[‘lname’]);
$firstName = $mysqli->real_escape_string($_POST[‘fname’]);

$cryptedPassword = crypt($_POST[‘password1’]);
$password = $mysqli->real_escape_string($cryptedPassword);

if (isset($_POST[‘username’])) {
$username = $mysqli->real_escape_string($_POST[‘username’]);
} else {
$username = “”;
}

if (isset($_POST[‘mii_name’])) {
$mii_name = $mysqli->real_escape_string($_POST[‘mii_name’]);
} else {
$mii_name = “”;
}

if (isset($_POST[‘friend_code’])) {
$friend_code = $mysqli->real_escape_string($_POST[‘friend_code’]);
} else {
$friend_code = “”;
}

$query = “INSERT INTO Users (email,create_date,password,last_name,first_name,mii_name,friend_code) " .
" VALUES (’{$email}’,NOW(),’{$password}’,’{$lastName}’,’{$firstName}’,’{$username}’,’{$mii_name}’,’{$friend_code}’)”;
if ($mysqli->query($query)) {
$id = $mysqli->insert_id;
error_log(“Inserted {$email} as ID {$id}”);
return true;
} else {
error_log(“Problem inserting {$query}”);
return false;
}
} //end function registerUser

?>[/code]

#2

What goes wrong? Where does it fail? Do you get any errors?

Btw: which book is it? Seems like it would fit well on the shelf of shame (outdated database handling)

#3

Well, I click register, and instead of adding the info to the data base and redirecting to the success.php page, it just refreshes the registration page

http://samtree.co.uk/3ds/registration.php

Also, it’s the PHP, MySQL, JavaScript and HTML5 for Dummies book

#4

A point of injection,

I wouldn’t really call MySQLi really outdated JimL, but then again I know how dedicated you are with the PDO. I’ve done all of my sites and applications with MySQLi and haven’t had a problem; and trust me I’ve tried using injection code. While PDO is quickly becoming the standard way of doing databases, if you know what you are doing you can use MySQLi and be fine.

BTW, usually the dummies series are usually a little bit outdated. When buying books on coding make sure it’s been created within the last year or so since, with technology, things change fast. A book past 2012 would be considered outdated at this point so take a look at the copyright and publication dates.

Cheers!

#5

I never said mysqli was outdated, but the fact they aren’t using parameterized queries is outdated practise. :slight_smile:

Yeah, but the quality of the tutorial/book is also important. Mysqli/PDO and parameterized queries came 10 years ago, yet we still see new publications refusing to adopt it… Not sure if it’s ignorance or that they just don’t know better.

#6

Hello,

SPKuja do me a favor and change

[php]if($_POST[‘submit’])[/php]

…to

[php]if($_POST)[/php]

…and see if the code works from there. The $_POST[‘submit’] goes on the assumption that you have the submit button on your form named submit and worded properly and since you aren’t showing the page with the form, I’m taking that into consideration.

JimL, sorry I misinterpreted you. I agree that these books need to include and take into consideration security practices when dealing with databases. However, since it is a beginner’s guide, they may have thought it to be too out of scope. Of course they may or may not have mentioned it for completeness.

Cheers!

#7

Hi guys, thanks for the replies!

I found what was causing the problem, the book told me to type “last_name” and first_name" when it should have “lname” and “fname”…

My issue now, is it will still let me register with the same email address, over and over again, the code that I am using is this:

//Check to see if email exists $findUser = "SELECT id from Users where email = '($email)'"; $findResult = $mysqli->query($findUser); $findrow = $findResult->fetch_assoc(); if (isset($findRow['id']) && $findRow['id'] != "") { $_SESSION['error'][] = "A user with that e-mail already exists"; return false; }

I don’t get any errors, or any thing, it just adds it into the database again

#8

Do yourself a favor and throw that tutorial in file 13 and get a different one. It really should use prepared statements as JimL has already stated.

While the following is done in OOP I think you get the gist how elegant using prepared statements can be:

[php] public function isEmailUsed() {

    // Connect to PDO database:
    $db = Database::getInstance();
    $pdo = $db->getConnection();


    $query = "
        SELECT
            1
        FROM users
        WHERE
            email = :email1
    ";

    $query_params = array(
        ':email1' => $this->email
    );

    // These two statements run the query against your database table.
    $stmt = $pdo->prepare($query);
    $result = $stmt->execute($query_params);

    // The fetch() method returns an array representing the "next" row from
    // the selected results, or false if there are no more rows to fetch.	   	   
    return $row = $stmt->fetch();
    // If a row was returned, then we know a matching email was found in
    // the database already and we should return a boolean value back.	  	   	      
}[/php]

You can find good tutorials on the internet on how to do this, just search for tutorials that have mysqli using prepared statements or PDO using prepared statements (my recommendation). Oh, one thing about security, never tell a user that an email address is taken by another user, just state that particular email address is unavailable and leave it at that. Though I don’t know why a user would have another person’s email address, it’s just a good rule of thumb never to divulge more information than is needed.

#9
Oh, one thing about security, never tell a user that an email address is taken by another user, just state that particular email address is unavailable and leave it at that. Though I don't know why a user would have another person's email address, it's just a good rule of thumb never to divulge more information than is needed.

Very good point. I think I will have to change my ways :slight_smile:

#10

Here is my suggestion …

the DB

CREATE TABLE IF NOT EXISTS `users` ( `id` int(11) NOT NULL AUTO_INCREMENT, `username` varchar(255) NOT NULL, `password` varchar(255) NOT NULL, `first_name` varchar(255) NOT NULL, `last_name` varchar(255) NOT NULL, `email` varchar(255) NOT NULL, PRIMARY KEY (`id`), UNIQUE KEY `username` (`username`), UNIQUE KEY `email` (`email`) ) ENGINE=InnoDB DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

The DB info
db_const.php

<?php # mysql db constants DB_HOST, DB_USER, DB_PASS, DB_NAME const DB_HOST = 'SERVER'; const DB_USER = 'USER'; const DB_PASS = 'PASSWORD'; const DB_NAME = 'php_mysql_login_system'; ?>

The registration page
register.php

[code]

User registration form- PHP MySQL Ligin System | W3Epic.com

User registration form- PHP MySQL Ligin System | W3Epic.com

<?php require_once("db_const.php"); if (!isset($_POST['submit'])) { ?> Username:
Password:
First name:
Last name:
Email: 
    <input type="submit" name="submit" value="Register" />
</form>
<?php } else { ## connect mysql server $mysqli = new mysqli(DB_HOST, DB_USER, DB_PASS, DB_NAME); # check connection if ($mysqli->connect_errno) { echo "

MySQL error no {$mysqli->connect_errno} : {$mysqli->connect_error}

"; exit(); } ## query database # prepare data for insertion $username = $_POST['username']; $password = $_POST['password']; $first_name = $_POST['first_name']; $last_name = $_POST['last_name']; $email = $_POST['email']; # check if username and email exist else insert $exists = 0; $result = $mysqli->query("SELECT username from users WHERE username = '{$username}' LIMIT 1"); if ($result->num_rows == 1) { $exists = 1; $result = $mysqli->query("SELECT email from users WHERE email = '{$email}' LIMIT 1"); if ($result->num_rows == 1) $exists = 2; } else { $result = $mysqli->query("SELECT email from users WHERE email = '{$email}' LIMIT 1"); if ($result->num_rows == 1) $exists = 3; } if ($exists == 1) echo "

Username already exists!

"; else if ($exists == 2) echo "

Username and Email already exists!

"; else if ($exists == 3) echo "

Email already exists!

"; else { # insert data into mysql database $sql = "INSERT INTO `users` (`id`, `username`, `password`, `first_name`, `last_name`, `email`) VALUES (NULL, '{$username}', '{$password}', '{$first_name}', '{$last_name}', '{$email}')"; if ($mysqli->query($sql)) { //echo "New Record has id ".$mysqli->insert_id; echo "

Registred successfully!

"; } else { echo "

MySQL error no {$mysqli->errno} : {$mysqli->error}

"; exit(); } } } ?> [/code]

The Login Form
login.php

[code]

User Login Form - PHP MySQL Ligin System | W3Epic.com

User Login Form - PHP MySQL Ligin System | W3Epic.com

<?php if (!isset($_POST['submit'])){ ?> Username:
Password: 
    <input type="submit" name="submit" value="Login" />
</form>
<?php } else { require_once("db_const.php"); $mysqli = new mysqli(DB_HOST, DB_USER, DB_PASS, DB_NAME); # check connection if ($mysqli->connect_errno) { echo "

MySQL error no {$mysqli->connect_errno} : {$mysqli->connect_error}

"; exit(); } $username = $_POST['username']; $password = $_POST['password']; $sql = "SELECT * from users WHERE username LIKE '{$username}' AND password LIKE '{$password}' LIMIT 1"; $result = $mysqli->query($sql); if (!$result->num_rows == 1) { echo "

Invalid username/password combination

"; } else { echo "

Logged in successfully

"; // do stuffs } } ?> [/code]
#11

Your code is vulnerable to sql injection

#12

Looking at this code the glaringly obvious mistake is that $findrow and $findRow are two completely different variables that have no relation to each other…
[php]
$findrow = $findResult->fetch_assoc();
if (isset($findRow[‘id’]) && $findRow[‘id’] != “”) {
[/php]

Red :wink:

#13

To counter balance the mysqli v pdo argument, (as mysqli is my weapon of choice) this function could quite easily do mysqli queries also, it is not limited to PDO. What Strider was pointing out was the ‘elegance’ of the query rather than the driver used.

This is an excellent thing to learn from the start… Nice shout Strider :wink:

Just my two-pence worth…
Red :wink:

#14

But the moment your boss decides to change from mysql you’re SO glad you used PDO :wink:

#15

Good job i’m the boss… :stuck_out_tongue: :slight_smile:

#16

i totally agree, and i was not trying to choose one or the other however simply giving something for him to see the basics better than that silly book that is nothing more than fire fodder … This example is indeed vulnerable to injections however can be made invulnerable with a few tweeks, but yes if you really want to stay ahead of the curve dont waste your time with mysql_ or mysqli_ and learn nothing more than PDO as it is currently the safer option.

#17

Always nice, you can still get the problem with customers insisting running their own db server though ^^

Both PDO And Mysqli support proper parameterized queries, so either one is actually good to use. If (and only if) you use parameterized queries.

They were released 10 years ago, there is no reason to continue doing queries the old way any more. Doing queries the old way is slower, way more insecure, much more work to make secure, it clutters up the code, and it’s not easier.

Sponsor our Newsletter | Privacy Policy | Terms of Service