2
\$\begingroup\$

Ok, so I have been looking for resources on how to make a registration form work with input validation. I want the error message show up under the table of the registration form and not on top of the website. Here is what I have working so far, but I am still learning and I would like to improve:

<?php

$host = 'localhost';
$user = 'root';
$pass = '';
$db = 'restaurantdb';

try {
$db_conn = new mysqli("$host", "$user", "$pass", "$db");
$db_conn->set_charset("utf8");
} catch (Exception $e) {
echo "Connection failed: " . $e->getMessage();
}

if (isset($_POST['register'])) {


session_start();
$forename = filter_input(INPUT_POST, 'forename', FILTER_SANITIZE_STRING);
$surname = filter_input(INPUT_POST, 'surname', FILTER_SANITIZE_STRING);
$username = filter_input(INPUT_POST, 'username', FILTER_SANITIZE_STRING);   
$password = filter_input(INPUT_POST, 'password', FILTER_SANITIZE_STRING);
$age = filter_input(INPUT_POST, 'age', FILTER_SANITIZE_STRING);
$email = filter_input(INPUT_POST, 'email', FILTER_SANITIZE_STRING);
$password = password_hash($password, PASSWORD_BCRYPT);

if(FindErrors($username, $password)) {
$query = "INSERT INTO users(firstname, lastname, username, password, age, email) 
VALUES('$forename', '$surname', '$username', '$password', '$age', '$email')";
$result = $db_conn->query($query);
if(!empty($result)) {
    $error_message = "";
    $success_message = "Registration successful";
    unset($_POST);
} else die($db_conn->error);
}
}


$forename = $surname = $username = $password = $age = $email = "";

function FindErrors($username, $password){


if (empty($forename)) {
    echo "First name is required\n";
} else {
    $forename = fix_string($_POST["forename"]); 
}

if (empty($surname)) {
    echo "Last name is required\n";
} else {
    $surname = fix_string($_POST["surename"]);
}

if (empty($username)) {
    echo "Username is required\n";
    return false;
} else {
    $password = fix_string($_POST["username"]);
}

if (empty($password)) {
    echo "Password is required\n";
    return false;
} else {
    $password = fix_string($_POST["password"]);
}

if (empty($age)) {
    echo "Age is required\n";
} else {
    $age = fix_string($_POST["age"]);
}

if (empty($email)) {
    echo "Email is required\n";
} else {
    $email = fix_string($_POST["email"]);
}
}

$fail = validate_forename($forename);
$fail .= validate_surname($surname);
$fail .= validate_username($username);
$fail .= validate_password($password);
$fail .= validate_age($age);
$fail .= validate_email($email);

echo "<!DOCTYPE html\n<html><head><title>An Example Form</title>";

if ($fail == "")
{
echo "</head><body>Form data successfully validated:
$forename, $surname, $username, $password, $age, $email.</body></html>";

// This is where you would enter the posted fields into a database.
// Preferably using hash encryption for the password.
exit;
}

echo <<<END_OF_BLOCK

<!--THIS IS HTML/JAVASCRIPT SECTION -->

<style>
    .signup {
        border: 1px solid #999999;
        font: normal 14px helvetica;
        color: #444444
    }
</style>

<script>
    function validate(form) {
        fail = validateForename(form.forename.value)
        fail += validateSurname(form.surname.value)
        fail += validateUsername(form.username.value)
        fail += validatePassword(form.password.value)
        fail += validateAge(form.age.value)
        fail += validateEmail(form.email.value)

        if (fail == "") return true
        else { alert(fail); return false }
    }

    function validateForename(field) {
        return (field == "") ? "No forename was entered.\n" : ""
    }

    function validateSurname(field) {
        return (field == "") ? "No surname was entered.\n" : ""
    }

    function validateUsername(field) {
        if (field == "") return "No Username was entered.\n"
        else if (field.length < 5)
            return "Usernames must be at least 5 characters.\n"
        else if (/[^a-zA-Z0-9_-]/.test(field))
            return "Only a-z, A-Z, 0-9, - and _ allowed in Usernames.\n"
        return ""
    }

    function validatePassword(field) {
        if (field == "") return "No password was entered.\n"
        else if (field.length < 6)
            return "Passwords must be at least 6 characters.\n"
        else if (!/[a-z]/.test(field)) || !/[A-Z]/.test(field) || !/[0-9]/.test(field))
        return "Passwords require one each of a-z, A-Z and 0-9.\n"
        return ""
    }

    function validateAge(field) {
        if (field == "" || isNaN(field)) return "No age was entered.\n"
        else if (field < 18 || field > 110)
            return "Age must be between 18 and 110.\n"
        return ""
    }

    function validateEmail(field) {
        if (field == "") return "No email was entered.\n"
        else if (!((field.indexOf(".") > 0) && (field.indexOf("@") > 0) || /[^a-zA-Z0-9.@_-]/.test(field)))
            return "The Email address is invalid.\n"
        return ""

    }

</script>
</head>
<body>
<header>
    <h1>Sun Sun Chinese Restuarant</h1>
</header>
<nav>
    <hr />
    <a href="index.html">Home</a>
    <a href="menu.html">Menu</a>
    <a href="contactus.html">Contact Us</a>
    <a href="validate.html">Sign Up</a>
    <hr />
</nav>

<table class="signup" border="0" cellpadding="2" cellspacing="5" bgcolor="#eeeeee"> 
    <th colspan="2" align="center">Signup Form</th>
    <form method="post" action="adduser.php" onsubmit="return validate(this)">
        <tr>
            <td>Forename</td>
            <td><input type="text" maxlength="30" name="forename" 
            value="" required></td>
        </tr>
        <tr>
            <td>Surname</td>
            <td><input type="text" maxlength="30" name="surname" 
            value="" required></td>
        </tr>
        <tr>
            <td>Username</td>
            <td><input type="text" maxlength="30" name="username" 
             value="" required></td>
        </tr>
        <tr>
            <td>Password</td>
            <td><input type="password" maxlength="30" name="password" 
            value="" required></td>
        </tr>
        <tr>
            <td>Age</td>
            <td><input type="text" maxlength="6" name="age" 
            value="" required></td>
        </tr>
        <tr>
            <td>Email</td>
            <td><input type="text" maxlength="60" name="email" 
            value="" required></td>
        </tr>

        <tr>
            <td colspan="2" align="center"><input type="submit" name="register"  value="Signup"></td>
        </tr>



    </form>

</table>
<!-----DISPLAY ERROR OR SUCCESS MESSAGE HERE------>


</body>

<footer>
<!-- The footer is a block at the bottom of the page. -->
<hr />
<center>
    <p>
        5301 Forest Lane<br />
        St. Louise, MS 35012<br />
        204-462-4296
    </p>
</center>
</footer>

</html>

END_OF_BLOCK;

//THE PHP Functions

function validate_forename($field)
{
return ($field == "") ? "No Forename was entered<br>": "";
}

function validate_surname($field)
{
    return ($field == "") ? "No Surname was entered<br>" : "";
}

function validate_username($field)
{
if ($field == "") return "No Username was entered<br>";
else if (strlen($field) < 5)
return "Usernames must be at least 5 characters<br>";
else if (preg_match("/[^a-zA-Z0-9_-]/", $field))
return "Only letters, numbers, numbers, - and _ in usernames<br>";
return "";
}

function validate_password($field)
{
if ($field == "") return "No Password was entered<br>";
else if (strlen($field) < 6)
return "Passwords must be at least 6 characters<br>";
else if (!preg_match("/[a-z]/", $field) || 
        !preg_match("/[A-Z]/", $field) ||
            !preg_match("/[0-9]/", $field))
            return "Passwords require 1 each of a-z, A-Z, and 0-9<br>";
            return "";
}

function validate_age($field)
{
    if ($field == "") return "No age was entered<br>";
else if ($field < 18 || $field > 110)
    return "Age must be between 18 and 110<br>";
return "";
}

function validate_email($field)
{
if ($field == "") return "No Email was entered<br>";
else if (!((strpos($field, ".") > 0) && 
(strpos($field, "@") > 0)) || 
preg_match("/[^a-zA-Z0-9.@_-]/", $field))
    return "The email address is invalid<br>";
return "";
}

function fix_string($string)
{
    if (get_magic_quotes_gpc()) $string = stripslashes($string);
return htmlentities ($string);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ FindErrors() will not work as you expect it to because of "function scope". For instance, $forename will always be empty because that variable does not exist inside the function. Your code is broken, you just don't realize it yet. Consider function FindErrors($username, $password){ to be interpretted as function FindErrors($param1, $param2){ ...it won't know what variable you are passing to it. \$\endgroup\$ – mickmackusa May 5 at 16:06
1
\$\begingroup\$

There are many improvements you could make, but you're on the right track. In this answer I will only comment on some of the things I immediately notice.

Coding style

It is difficult to get an overview of your code. There's little organization. I would have split this code up into multiple files. For instance: There will be more PHP files on your server that need the database. Having a separate PHP file that opens the database, and can be included into this one, would help and you would only need to change the database access parameters in one place. Same is true for CSS and Javascript.

Use indentation to make your code more readable. See the code style guides.

Handling errors

If your database connection fails, you do catch the exception to echo a message, but then you just go on as if nothing has happened. What's the point of that? The same is true for the FindErrors() function. It echos after a form submission, but that's all it does. It just carries on and gets your code into real trouble.

User input

You filter user input at the beginning of your script, that is a good idea. However, it is still user input and should be treated with extra care. It is quite likely you will forget about this, now that you've dumped this input into normal PHP variables. Better put them in an array like $userInput or prefix the names like so: $input_forename. That way you remember to treat user input with care.

Another problem in FindErrors() is that you directly access $_POST. What's the point of all the input filtering at the beginning? Don't do this, use the variables, or the array, you created at the beginning.

Do not filter_input(INPUT_POST, 'password', FILTER_SANITIZE_STRING); the password. It could remove things from the password that an user intended to have there. It's a password, the content can be anything and doesn't need filtering.

Don't repeat yourself

You've implemented the same validation functions in PHP and in Javascript. I can understand why, but it does feel like duplication of functionality. I don't have a quick solution at hand, that would require long code examples, but try to avoid this. See: DRY

Database

Do not store plain-text passwords in a database. See: Never store passwords in a database!. Store a hash of the password, see: password_hash().

An obvious security problem is that your database query is open to SQL injection attacks. Use prepared statements instead. This is by far the most common problem we see, and it is actively exploited (because many developers can't be bothered).

Finally

That's my list so far. This is not a complete code review. I haven't given proper advice on how the code could be rewritten. Nevertheless I hope there are some useful tips here for you.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.