Form Validation using AJAX with PHP fallback

Question

I've used this PHP/AJAX method of validation for a bit, but I think it could use a little peer review. The basic idea was to create a way of verifying forms in a nice manor using AJAX, but also fall-back to PHP when the user disables JavaScript. Anyway, here's the code with some explanation as to what is happening. One extra note: jQuery is used throughout the JavaScript files.
field_validation.php

if (isset($_POST['field']) && isset($_POST['value'])) {
    require_once('includes/functions.php'); // Allows connection to the database
    $field = $_POST['field'];
    $value = $_POST['value'];
    echo validate_input($field, $value);
}

function validate_input($field, $input) {
    switch ($field) {
        case 'email':
            if ($input != '') {
                // We need the validate_email() function, so we now require the file with it in. (Not included as I'm happy with it's validation and this is only used as an example)
                require_once ('validate_email.php');
                if (validate_email($input) == false) {
                    return 'Please enter a valid email';
                }
            } else {
                return 'Please enter an email';
            }
            // Check email address is not already taken
            $db = mysqlConnect();
            $stmt = $db->stmt_init();
            $stmt->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE email  = ?)");
            $stmt->bind_param('s', $input);
            $stmt->execute();
            $stmt->bind_result($alreadyTaken);
            $stmt->fetch();
            $stmt->close();
            $db->close();
            if ($alreadyTaken) {
                return 'Email address already in use, you cannot sign up again';
            }
            return true;
            break;
        case 'password':
            if (strlen($input) <= 6) {
                return 'Please enter a password more than 6 characters in length';
            }
            return true;
            break;
        case 'twitter':
            if (strlen($input) < 1 ) {
                return true;
            }
            if (strlen($input) >= 16) {
                return 'Twitter Usernames cannot be more than 15 cahraters in length (excluding the @ symbol)';
            }
            $apiurl = 'http://api.twitter.com/1/users/show.xml?screen_name=' . $input;
            if (!@fopen($apiurl, 'r'))
            {
                return 'Username does not exist, please enter a valid username';
            }
            // Check twitter name is not already taken
            $db = mysqlConnect();
            $stmt = $db->stmt_init();
            $stmt->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE twitter  = ?)");
            $stmt->bind_param('s', $input);
            $stmt->execute();
            $stmt->bind_result($alreadyTaken);
            $stmt->fetch();
            $stmt->close();
            $db->close();
            if ($alreadyTaken) {
                return 'Twitter name already in use, please chose another name';
            }
            return true;
            break;
        case 'displayname':
            if (strlen($input) < 1) {
                return 'Please enter a Display Name';
            }
            if (strlen($input) > 64) {
                return 'Please enter a Display Name shorter than 65 charaters';
            }
            $db = mysqlConnect();
            $stmt = $db->stmt_init();
            $stmt->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE display_name  = ?)");
            $stmt->bind_param('s', $input);
            $stmt->execute();
            $stmt->bind_result($alreadyTaken);
            $stmt->fetch();
            $stmt->close();
            $db->close();
            if ($alreadyTaken) {
                return 'Display Name already in use, please chose another name';
            }
            return true;
            break;
        case 'termsandconditons':
            if ($input != 'on') {
                return 'You must accept the Terms and Conditions';
            }
            return true;
            break;
        default:
            return true;
            break;
    }
}
function showErrors($errors) {
    ?>
    <div id="errors">
        Errors<br>
        <ul>
            <?
            for ($i = 0; $i < count($errors); $i++) {
                if (isset($errors[$i]['name'])) {
                    // Highlight the field
                    ?>
                    <script>
                        var cross = '<img src="/images/cross.png">';
                        $(document).ready(function(){
                            $('#<? echo $errors[$i]['name']; ?>hint').html(cross);
                        });
                    </script>
                    <?
                }
                ?>
                <li>
                    <? echo $errors[$i]['message']; ?><br >
                </li>
                <?
            }
            ?>
        </ul>
    </div>
    <?
}

This is included in each of the pages that require validation. I have removed some of the extra cases since they are pretty much the same as the ones shown.
validatefields.js

var cross = '<img src="/images/cross.png">';
var tick = '<img src="/images/tick.png">';
var loading = '<img src="images/loading.gif">';

// Send the data from the form to the PHP file 'jqueryvalidate.php', where they will call the function to validate the data
$('input:text,input:password,input[type=email]').blur(function() {
    // Get the input from the form
    var fieldid = $(this).attr('id');
    var fieldvalue = $(this).attr('value');
    $('#' + fieldid + 'hint').html(loading);
    $.ajax ({
        url: 'field_validation.php',
        data: {field: fieldid, value: fieldvalue},
        type: 'post',
        cache: false,
        success: function(message) {
            if (message != true) {
                // Error was returned, show a cross
                $('#' + fieldid + 'hint').html(cross + message);
            } else {
                // No error, show a tick
                $('#' + fieldid + 'hint').html(tick);
            }
        }
    });
});

$('input:checkbox').click(function() {
    // Get the input from the form
    var fieldid = $(this).attr('id');
    if ($(this).attr('checked') == 'checked') {
        var fieldvalue = 'on';
    } else {
        var fieldvalue = 'off';
    }
    $('#' + fieldid + 'hint').html(loading);
    $.ajax ({
        url: 'jqueryvalidate.php',
        data: {field: fieldid, value: fieldvalue},
        type: 'post',
        cache: false,
        success: function(message) {
            if (message != true) {
                // Error was returned, show a cross
                $('#' + fieldid + 'hint').html(cross + message);
            } else {
                // No error, show a tick
                $('#' + fieldid + 'hint').html(tick);
            }
        }
    });
});

As you can see this has 2 different functions for the validation. One for text, password and email fields and another for checkboxes. This could probably be tidied up a little?
Finally, an example form (mypage.php)

if($_SERVER['REQUEST_METHOD'] == "POST") {
    $errors = array();
    // Form has been submitted
    foreach($_POST as $key => $value) {
        $$key = $value; // Set the title of the field to its name
        if (validate_input($key, $value) != 'true') {
            // Value is invalid
            $error['message'] = validate_input($key, $value);
            $error['name'] = $key;
            array_push($errors, $error);
        }
    }
    if (!isset($_POST['termsandconditions'])) {
// This could be improved
            if (validate_input('termsandconditions', 'off') != 'true') {
                // Value is invalid
                $error['message'] = validate_input('termsandconditions', 'off');
                $error['name'] = 'termsandconditions';
                array_push($errors, $error);
            }
        }
        if (count($errors) > 0) {
            // Input data failed validation
            showErrors($errors);
        } else {
    // The form has successfully validated
    }
    <form action="mypage.php" method="POST">
        <label for="displayname">*Display Name: </label>
        <input type="text" name="displayname" id="displayname" <? if (isset($_POST['displayname'])) echo 'value="' . $_POST['displayname'] . '"' ?> ><br>
        <div id="displaynamehint"></div><br>
        <label for="email">*Email: </label>
        <input type="email" name="email" id="email" <? if (isset($_POST['email'])) echo 'value="' . $_POST['email'] . '"' ?> ><br>
        <div id="emailhint"></div><br>
        <label for="password">*Password: </label>
        <input type="password" name="password" id="password"><br>
        <div id="passwordhint"></div><br>
        <label for="twitter">Twitter Username: </label>
        <span class="twitterinput">@<input type="text" name="twitter" id="twitter" <? if (isset($_POST['twitter'])) echo 'value="' . $_POST['twitter'] . '"' ?> ></span><br>
        <div id="twitterhint"></div><br>
        <input type="checkbox" name="termsandconditions" id="termsandconditions" <? if (isset($_POST['termsandconditions'])) echo 'checked="checked"'?> ><label for="termsandconditions">*I have fully read, understand and agree to <a href="termsandconditions.php" target="_blank">the terms and conditions</a></label><div id="termsandconditionshint"></div><br>
        <input type="submit" name="submit" value="Submit">
    </form>

Well, that was a lot of code to look through, so if you've looked through it, thank you! This was my first post to Code Review, so I'm hoping I've not missed anything vital.

I'm feeling too lazy at the moment to write a proper answer, so a few pointers: Don't compare a boolean to it's string equivalent. Either do if ($a) or if ($a === true) or if (!$a) or if ($a === false) (if you know it's a boolean, I'd tend towards the === options). Pass your DB instance to the validate_input function. You're unnecessarily tying your validate_input function to mysqlConnect(). A random function should not be responsible for setting up a DB connection. And lastly, validate_input is doing way too much. Try to break it into smaller logical functions.

mseancole · Accepted Answer · 2012-05-17 16:35:20Z

Code Separation

Break up your functions! That has got to be the largest function I've ever seen!

There's no reason to type that MySQL block over and over again. Make it a function and it will go a long way towards cleaning this up.

function alreadyTaken($input, $where) {
    $db = mysqlConnect();
    $stmt = $db->stmt_init();
    $stmt->prepare("SELECT EXISTS(SELECT 1 FROM users WHERE $where = ?)");
    $stmt->bind_param('s', $input);
    $stmt->execute();
    $stmt->bind_result($alreadyTaken);
    $stmt->fetch();
    $stmt->close();
    $db->close();
    return $alreadyTaken;
}

I'm still not thrilled with the extra large switch statement and would suggest making the code in each of those cases their own function. Sure, you aren't going to reuse them, but at least it will be cleaner. Also, if all of your cases return true if nothing is wrong, you can just end the validate_input() function with return true; and remove all those other return true; statements. Then you can remove the default case because that eventuallity would already be covered.

Sanitizing As Well As Validating

If your PHP version is >= 5.2 you can use PHP's filter_var() or filter_input() function to sanitize your user input. You should always sanitize your user input before using it. Never trust it. If you do not have PHP >= 5.2 you'll have to look around for the best way of sanitizing your input.

PHP Short Tags

Some servers do not accept PHP short tags. You should always expand <? to <?php for compatibility.

Foreach > For

No need to use a for loop for what you can more easily do with a foreach loop. I would rewrite your showErrors() function to something more like this...

function showErrors($errors) {
?>
    <script>
        var cross = '<img src="/images/cross.png">';
        $(document).ready(function(){
<?php foreach(array_keys($errors) as $name) { echo "$('#{$name}hint').html(cross);"; } ?>
        });
    </script>
    <div id="errors">
        Errors<br />
        <ul>
<?php foreach($errors as $message) { echo "<li>$message</li>"; } ?>
        </ul>
    </div>
<?php
}

Not to say this works 100%. I haven't tested it myself, but it should. I'm unclear about the JQuery because I haven't done much with it, but from what I remember it should still work. If not you should only have to recreate that function, not the cross variable and script blocks. You probably wont even need the cross variable as you do indeed define it again on the next script "validatefields.js".

Also, after coming to the end of your scripts I noticed that you always set $error['name']. Why do you check if it is set? Remove the isset() if it is not necessary I wrote the above revision to showErrors() with this in mind...

Variable Templates

You can do this in PHP too, not just JavaScript. Those links are to their respective documentation pages.

var template = '<img src="/images/%s">';

var cross = sprintf(template, 'cross.png');
var tick = sprintf(template, 'tick.png');
var loading = sprintf(template, 'loading.gif');

I'm not going to look at the rest of your JavaScript as it appears to be mostly JQuery and I'm too unfamiliar with that to lend a hand. You'll have to get someone else to help you with that.

getError Function and Variable Comparisons

if (validate_input($key, $value) != 'true')

Does indeed return as you would expect, but only because none of your statements returns a string with the value of "true". What you have now is not a boolean comparison but a string comparison. != and == are for loose comparison where the string "true" (or any other none empty string or non "false" string) is equivalent to boolean TRUE. !== and === are for absolute comparison, where only two variables of the same type with the same value ever return TRUE ("true" is not true). It should be written with an absolute comparison because what you are looking for is not the string "true" but the boolean TRUE.

if (validate_input($key, $value) !== true)

Now, because you seem to set your errors the same every time I would make it a function to reduce the need to type it.

function getError($name, $message) {
    $error = array();

    $error['message'] = validate_input('termsandconditions', 'off');
    $error['name'] = $name;

    return $error;
}

Finally, since you reuse the same validate_input() line for your error message, if it has one, I would set it as a variable before the if statement and then check the variable before setting the error with the new getError() function.

$isValid = validate_input($key, $value);
if($isValid !== true) { array_push($errors, getError($key, $isValid)); }

Variable Variables

$$key = $value; // Set the title of the field to its name

$$key is a variable variable. And most people agree that these are bad. Not only are they almost impossible to spot (I almost missed it) but they are impossible to document. No IDE that I know of will even recognize them. Not only that, but you don't even use it as far as I can tell. My advice, stay away from variable variables. They only cause headaches.

Thank you for all your comments, they are very useful. As for the variable variables, they are used once the form has been submitted, so if I want to put a field into the database, I just use $fieldname e.g. the input field displayname's value would be $displayname. Other than manually assigning each field, is there a way to improve this? (I don't want this to seem lazy, it's more that I was after a "catch all" type of system!)
One more thing before answer, you've set those variables before even determining if they were valid. That was also bad. But I WOULD use that function I mentioned, filter_input(), to get a clean version of it before passing it to the validate function. Once in the if statement, then I'd set them. I wouldn't really call it "lazy", because the "one" time they are considered "ok" is when used in controllers in MVC to do what you are trying to do. You could take a look at that, or you could set those to a new array and just extract() them out in the new file.

asked	1 year ago
viewed	4222 times
active	11 months ago

Form Validation using AJAX with PHP fallback

1 Answer

Your Answer

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

Community Bulletin

Form Validation using AJAX with PHP fallback

1 Answer

Your Answer

Sign up or log in

Post as a guest

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

Community Bulletin

Related