Form sending mail despite validation errors

I have just built my first PHP contact form with validation and yet it is sending an email regardless. I must have missed something. Can anyone spot it?

<?php if($_SESSION['instance'] == '1') {

    $email = $_POST['ENQemail'];
    $firstname = $_POST['ENQfirst_name'];
    $lastname = $_POST['ENQla开发者_Go百科st_name'];
    $message = $_POST['ENQmessage'];
    $secword = $_POST['ENQsecword'];



  if (!ereg("^[^@]{1,64}@[^@]{1,255}$", $email)) {
    $ERRemail = 'invalid email address';
  }
  if (strlen($firstname < 2)) {

    $ERRfirstname = 'Please enter your first name';
  }
  if (strlen($lastname < 2)) {

    $ERRlastname = 'Please enter your surname';
  }
  if (strlen($message < 50)) {

    $ERRmessage = 'Your message must be at least 50 characters';
  }
  if ($secword == $_SESSION['instance']) {

    $ERRsecword = 'Your security word did not match the image';
  }

  else {

      $to = "enquire@divethegap.com";
  $subject = "DTG Enquiry - ".$firstname." ".$lastname ;
  $message = $message;
  $headers = "From: ".$firstname." ".$lastname." ".$email. "\r\n" .
             "Content-type: text/html" . "\r\n";

  mail($to, $subject, $message, $headers);


  }
    }
?>

Any ideas?

The else statement that contains the email-sending code is only associated with the previous if statement. This means that the only time it will not be called is if $secword == $_SESSION['instance'] evaluates to false. It doesn't matter whether the other validation checks succeed or not.

One strategy is to keep track of all the errors that occurred by storing them in an array. If the array is empty, then you know all the fields are OK and that it is safe to send the email:

$errors = array();
if (!ereg("^[^@]{1,64}@[^@]{1,255}$", $email)) {
  $errors[] = 'invalid email address';
}
if (strlen($firstname < 2)) {
  $errors[] = 'Please enter your first name';
}
//...

if (count($errors) == 0){
  //send the email
  mail(...);
} else {
  //display the error messages
}

Your code looks pretty good otherwise! Here are some other suggestions:

• In order to use sessions in PHP, you must first call the session_start() function. This should be the very first thing your PHP script does.

• The ereg() function has been deprecated in the latest version of PHP. This means that the function may be removed in a future version of PHP. It is recommended that you use the preg_match() function instead (note that with preg_match(), the regex string must begin and end with a / character).

At the bottom is a fixed version of the logic. You missed the "else if"s.

Also the following line looks like it should check for inequality but it depends on your code so you would know better:

if ($secword == $_SESSION['instance'])

Here is the full fix:

    <?php if($_SESSION['instance'] == '1') {

    $email = $_POST['ENQemail'];
    $firstname = $_POST['ENQfirst_name'];
    $lastname = $_POST['ENQlast_name'];
    $message = $_POST['ENQmessage'];
    $secword = $_POST['ENQsecword'];



  if (!ereg("^[^@]{1,64}@[^@]{1,255}$", $email)) {
    $ERRemail = 'invalid email address';
  }
  else if (strlen($firstname < 2)) {

    $ERRfirstname = 'Please enter your first name';
  }
  else if (strlen($lastname < 2)) {

    $ERRlastname = 'Please enter your surname';
  }
  else if (strlen($message < 50)) {

    $ERRmessage = 'Your message must be at least 50 characters';
  }
  else if ($secword == $_SESSION['instance']) {

    $ERRsecword = 'Your security word did not match the image';
  }

  else {

      $to = "enquire@divethegap.com";
  $subject = "DTG Enquiry - ".$firstname." ".$lastname ;
  $message = $message;
  $headers = "From: ".$firstname." ".$lastname." ".$email. "\r\n" .
             "Content-type: text/html" . "\r\n";

  mail($to, $subject, $message, $headers);


  }
    }
?>

You're doing all the validations independent of each other and you send out the email if the security image is correct irrespective of whether everything else passed the check or not because your else to send the email matches up with the if ($secword == $_SESSION['instance']) {

You could do one of the following:

1. Change all the ifs but the first to elseifs so that only if all checks pass will the last else (which sends the email) be entered

   if (!ereg("^[^@]{1,64}@[^@]{1,255}$", $email)) {
       $ERRemail = 'invalid email address';
   }
   elseif (strlen($firstname < 2)) {
       $ERRfirstname = 'Please enter your first name';
   }
   elseif (strlen($lastname < 2)) {
       $ERRlastname = 'Please enter your surname';
   }
   elseif (strlen($message < 50)) {
       $ERRmessage = 'Your message must be at least 50 characters';
   }
   elseif ($secword == $_SESSION['instance']) {
       $ERRsecword = 'Your security word did not match the image';
   }
   else {
     $to = "enquire@divethegap.com";
     $subject = "DTG Enquiry - ".$firstname." ".$lastname ;
     $message = $message;
     $headers = "From: ".$firstname." ".$lastname." ".$email. "\r\n" . "Content-type: text/html" . "\r\n";
   
     mail($to, $subject, $message, $headers);
   }
   

2. Set a flag indicating if any check failed. And at the end, only send the email if everything worked out:

   $isValid = true;
   
   if(invalidEmail){
       $isValid =false;
   }
   
   if(invalidName){
       $isValid =false;
   }
   
   //finally send mail if all validation passed
   if($isValid) {
      //send email
   }