form validation - more efficient way?

[richiejones24]

I currently use the script below to validate a registration form when it is posted to the script, it works well but i know there are better ways to do it, and i want to change what is your advice on the best form validation?

 

if ($_POST['email1'] !== $_POST['email2']) {header ("Location: /reg.php?error=email_no_matchname']"); }
else {
if ($password1 !== $password2) { header ("Location: /reg.php?error=password_no_match"); }
else {
if ($_POST['firstname'] == NULL) { header ("Location: /reg.php?error=firstname_null"); }
else {
if ($_POST['lastname'] == NULL) { header ("Location: /reg.php?error=lastname_null"); }
else {
if ($_POST['phonea'] == NULL) { header ("Location: /reg.php?error=phonea_null"); }
else {
if ($_POST['phoneb'] == NULL) { header ("Location: /reg.php?error=phoneb_null"); }
else {
if ($_POST['username'] == NULL) { header ("Location: /reg.php?error=username_null"); }
else {
if ($_POST['email1'] == NULL) { header ("Location: /reg.php?error=email_null"); }
else {
if (strlen($_POST['password1']) <= 6) { header ("Location: /reg.php?error=password_length"); }
else {
if (strlen($_POST['phoneb']) <= 5) { header ("Location: /reg.php?error=phone_length"); }
else {
if (!is_numeric($_POST['phoneb'])) { header ("Location: /reg.php?error=phonea_numeric"); }
else {
if (!is_numeric($_POST['phoneb'])) { header ("Location: /reg.php?error=phoneb_numeric"); }
else {
if (strlen($_POST['username']) <= 4) { header ("Location: /reg.php?error=username_length"); }
else {
if (preg_match('/[^a*()-z0@£"%&-9.#$-]/i', $_POST['password1'])) { header ("Location: /reg.php?error=pwd_inv_cha"); }
else {
if (preg_match('/[^a*()-z0@£"%&-9.#$-]/i', $_POST['password1'])) { header ("Location: /reg.php?error=usn_inv_cha"); }
else {
if(!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $_POST['email1'])) {
header ("Location: /reg.php?error=email_validation"); }
}}}}}}}}}}}}}}}

[OOP]

Hi there,

There is nothing wrong with your validation code but you don't need to the use the header() to redirect the same page again. In the form action attribute, you can put the value "reg.php" as your script that will handle the validation.

 

if(!isset($_POST['submit'])){
<form action="reg.php" method="post">
the rest of the fields here
<input type="submit" name="submit" />
</form>
}else{

if ($_POST['email1'] !== $_POST['email2']) {
  $error='email_no_matchname';
}
else {
.
.
.
.

}

 

regards

 

 

 

 

 

 

[Psycho]

First of all I would do the validation completely different. But, even with the process you were trying to implement - the actual format is very poorly done.

 

First off, always indent your code to give it a visually logical structure. Makes editing/debugging much, much easier. You should not have all those nested statements.

 

You should have been using elseif() statements. So, instead of

if(condition1)
{
    //Do something
}
else
{
    if(condition2)
    {
          //Do something
    }
    else
    {
        if(condition3)
        {
             //Do something
        }
    }
}

 

You should instead use elseif() statements like this

if(condition1)
{
    //Do something
}
elseif(condition2)
{
    //Do something
}
elseif (condition3)
{
    //Do something
}

 

Plus, instead of putting all the headers() in each condition block, I would just set the error code. Here is your code in a more logical format:

$error = false;
if ($_POST['email1'] !== $_POST['email2']) {
    $error = "email_no_matchname";
} elseif ($password1 !== $password2) {
    $error = "password_no_match";
} elseif ($_POST['firstname'] == NULL) {
    $error = "firstname_null";
} elseif ($_POST['lastname'] == NULL) {
    $error = "lastname_null";
} elseif ($_POST['phonea'] == NULL) {
    $error = "phonea_null";
} elseif ($_POST['phoneb'] == NULL) {
    $error = "phoneb_null";
} elseif ($_POST['username'] == NULL) {
    $error = "username_null";
} elseif ($_POST['email1'] == NULL) {
    $error = "email_null";
} elseif (strlen($_POST['password1']) <= 6) {
    $error = "password_length";
} elseif (strlen($_POST['phoneb']) <= 5) {
    $error = "phone_length";
} elseif (!is_numeric($_POST['phoneb'])) {
    $error = "phonea_numeric";
} elseif (!is_numeric($_POST['phoneb'])) {
    $error = "phoneb_numeric";
} elseif (strlen($_POST['username']) <= 4) {
    $error = "username_length";
} elseif (preg_match('/[^a*()-z0@£"%&-9.#$-]/i', $_POST['password1'])) {
    $error = "pwd_inv_cha";
} elseif (preg_match('/[^a*()-z0@£"%&-9.#$-]/i', $_POST['password1'])) {
    $error = "usn_inv_cha";
} elseif(!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $_POST['email1'])) {
    $error = "email_validation";
}

if($error != false)
{
    header ("Location: /reg.php?error={$error}"); }
}

 

But anyway, the problem I have with your validation logic is that you simply stop validating after the first error. Personally, I hate when a site does that. I submit a form and it tells me that item 'A' is not correct. I fix that and resubmit only for the form to tell me the item 'B' is incorrect. Why didn't it tell me that items 'A' and 'B' were incorrect the first time I submitted the form?

 

I typically have the validation logic for a form in the same script that produces the form. So, I submit forms back to themselves. I run through ALL the validations and store any errors in an array. If the array is empty at the end of the validations then no errors are present and I include the form processing logic. If the array is not empty then I display ALL the errors and redisplay the form with the fields populated with the submitted values.

 

EDIT: You also need to rethink how you are doing your validations. Checking if a POST field is NULL is not appropriate. You need to first trim() the value and then check if the value of that is empty()