PHP Form validation logic is a little off

Question

I am validating an HTML Form with PHP. All of my code is fine, except the message field. If I put something in the message field and the other fields are displaying errors, it still submits. But, if I put something into the other fields and the other fields have errors, it won't submit, which is correct. I suspect it has something to do with the last if-else of my code. Thank you in advance.

contact.php
<?php

include 'includes/config.php';
$errors = FALSE;
$displayErrors = NULL;

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

$first = $_POST['first'];
$last = $_POST['last'];
$email = $_POST['email'];
$subject = $_POST['subject'];
$message = $_POST['message'];

//Connect to MYSQL Database server
$connect = mysql_connect(DB_HOST, DB_USER, DB_PASS) or die("Could not connect to MYSQL Database.");
$result = mysql_select_db(DB_NAME, $connect) or die("Could not connect to MYSQL table.");

//Clean Data to prevent malicous injections
mysql_real_escape_string(strip_tags(stripcslashes(trim($first))));
mysql_real_escape_string(strip_tags(stripcslashes(trim($last))));
mysql_real_escape_string(strip_tags(stripcslashes(trim($email))));
mysql_real_escape_string(strip_tags(stripcslashes(trim($subject))));
mysql_real_escape_string(strip_tags(stripcslashes(trim($message))));

if (empty($first)) {
    $errors = TRUE;
    $displayErrors .= 'First name is invalid.<br/>';
}
if (empty($last)) {
    $errors = TRUE;
    $displayErrors .= 'Last name is invalid.<br/>';
}
if (empty($email) || !filter_var($email, FILTER_VALIDATE_EMAIL)) {
    $errors = TRUE;
    $displayErrors .= 'Email is invalid.<br/>';
}
if (empty($subject)) {
    $errors = TRUE;
    $displayErrors .= 'Subject is invalid.<br/>';
}
if (empty($message)) {
    $errors = TRUE;
    $displayErrors .= 'Message is invalid.<br/>';
} else {
    $errors = FALSE;
          //Database insertion goes here
    echo 'Form submission successful. Thank you ' . $first . '.';

}

}
?>

I'd remove the last else and just do another if ($errors == TRUE) //do something else submit — Ronnie, May 25 '12 at 22:53
Yep, simple newbie mistake :) Make sure you accept an answer when you can — Ronnie, May 25 '12 at 22:57
mysql_real_escape_string() did not modify the passed string, it returns the modified string, so all the calls there are useless, because they don't have any effect to the given variables. — Dr.Molle, May 25 '12 at 22:59
yeah he's right. Something like `$message = mysql_real_escape_string(strip_tags(stripcslashes(trim($_POST['message']))));` — Ronnie, May 25 '12 at 23:02

score 2 · Answer 1 · answered May 25 '12 at 22:55

Basically you are saying, if the message is not empty then $errors=FALSE, but you don't take into account all the other fields.

I'd suggest putting the errors in an array like:

$errors['email'] = true;

Then checking that array at the end using a foreach

score 2 · Answer 2 · edited May 27 '12 at 00:20

2

Try turning that else into

if($errors == FALSE) {//just corrected the equality operator
    //Database insertion goes here
    echo 'Form submission successful. Thank you ' . $first . '.'
}

edited May 27 '12 at 00:20

Narayan Singh

1,234
2
13
26

answered May 25 '12 at 22:55

barro32

2,559
23
36

This solution worked too! I knew it was a small problem ahaha. Thanks a lot. – Sephiroth May 25 '12 at 22:57

score 0 · Answer 3 · edited May 26 '12 at 21:22

0

Make all your ifs else ifs that way your last else will not mess up your validation.

edited May 26 '12 at 21:22

dplante

2,445
3
21
27

answered May 25 '12 at 22:56

Tschallacka

27,901
14
88
133

score 0 · Answer 4 · answered May 25 '12 at 22:56

0

Before all of those ifs, you should put $errors = FALSE. Then instead of the else at the end of your code, you can put if(!$errors){ //database insertion here }

answered May 25 '12 at 22:56

luke

407
2
12

Lawrence Cherone · Accepted Answer · 2017-12-24T16:33:57.593

First off, Don't use mysql_* functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial.

I've tidied your logic, instead of using $error=true ect you should build an error array (containing your error text) and then check if thats empty at the end, I also cleaned up the magic quotes problem it seems your having, and the horrid multiple mysql_real_escape_string's, hope it helps

contact.php

<?php
include 'includes/config.php';
//Connect to MYSQL Database server
$connect = mysql_connect(DB_HOST, DB_USER, DB_PASS) or die("Could not connect to MYSQL Database.");
$result = mysql_select_db(DB_NAME, $connect) or die("Could not connect to MYSQL table.");

$errors = array();

if ($_SERVER['REQUEST_METHOD']=='POST') {

    function clean(&$value){
        if (get_magic_quotes_gpc()){
            $value = mysql_real_escape_string(strip_tags(trim($value)));
        }else{
            $value = mysql_real_escape_string(trim($value));
        }
    }
    array_walk($_POST,'clean');


    if (empty($_POST['first'])) {
        $errors['first']= 'First name is invalid.<br/>';
    }else{
        $first = $_POST['first'];
    }

    if (empty($_POST['last'])) {
        $errors['last']= 'Last name is invalid<br/>';
    }else{
        $last = $_POST['last'];
    }

    if (empty($_POST['email']) || !filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) {
        $errors['email'] = 'Email is invalid.<br/>';
    }else{
        $email = $_POST['email'];
    }

    if (empty($_POST['subject'])) {
        $errors['subject']= 'Subject is invalid.<br/>';
    }else{
        $subject = $_POST['subject'];
    }

    if (empty($_POST['message'])) {
        $errors['message']= 'Message is invalid.<br/>';
    }else{
        $message = $_POST['messsage'];
    }

    if(empty($errors)){
        //Database insertion goes here
        echo 'Form submission successful. Thank you ' . htmlentities($first) . '.';
    }else{
        //Errors, so your have $errors['name'] ect to output
        //so somthing like:
        echo (isset($errors['name'])?$errors['name']:null);

    }

}
?>

You should also perhaps add a condition that the value is a min certain length, with strlen($_POST['first']) > 2 ect.

PHP Form validation logic is a little off

5 Answers5