ansaurus

Question

Answer 1

+6 A:

The code seems correct, but I'd highly recommend adding in some data validation. You'll want to make sure all required fields are filled out with valid info. Also be sure to encode/strip any HTML, JS, etc for security/readability purposes.

Lastly, you should also consider using CAPTCHA to guard against spam. I've got an old site running code similar to this and used to get over 500 spam emails a day!

Colin O'Dell 2009-12-13 23:40:26

Perhaps you can mention some headers for your email

Robert Cabri 2009-12-13 23:55:41

Answer 2

+3 A:

That's pretty much it, maybe on successful completion you can do a header() redirect to a confirmation page, but as far as processing the form what you have is pretty standard.

Also, you want to sanitize your data as a standard practice of accepting any user input.

You might want to look into implementing a CAPTCHA to prevent the bots from hammering your form as well.

PHP Captcha

jaywon 2009-12-13 23:41:57

Good idea, thanks for the link!

Davey 2009-12-14 00:02:33

Answer 3

+3 A:

One thing you definitely want to do is make the data a bit safer to send in the email. I would at least run the htmlentities and strip_tags on the input data but you should definitely look in to doing further validation.

Also instead of isset($_POST["SUBMIT"]) I would maybe do something like...

if ($_SERVER["REQUEST_METHOD"] == "POST") {
    // body code here
}

Chris Gutierrez 2009-12-13 23:43:01

Is this to make it more readable? Or is there another benefit? Just curious.

Kevin 2009-12-14 02:38:42

I find it best to do it this way because you don't have to rely on PHP knowing the name of your submit button. Its more of a personal preference I guess. I've also worked with developers that don't put names on submit buttons.

Chris Gutierrez 2009-12-14 03:08:55

Answer 4

+3 A:

I would HIGHLY recommend looking up some information about PHP mail() hijacking and making sure you are not going to leave your script vulnerable to such an attack. Also what everyone else suggested is very good to do as well.

Jordan Messina 2009-12-13 23:51:28

Answer 5

+1 A:

In the question, you had 2 separate files processing the form. The problem is if you get a validation error, you are left with little choice but the awful "Please click you back button" solution.

Consider this template PHP file that will handle it all on one page, provide for data validation, errors, re-submitting, and the whole 9 yards.

<?php

// Read input variables from _POST
$FormAction = (isset($_POST['FormAction']) ? $_POST['FormAction'] : '');
$FirstName = trim(isset($_POST['FirstName']) ? $_POST['FirstName'] : '');
...

// Define script variables
$Errors = array();

// Process input if data was posted.
switch($FormAction)
{ 
   case 'Process':
      // validation code

      if(empty($FirstName) or strlen($FirstName) > 20)
          $Errors[] = "First name is required.";

      ...

      if(count($Errors) > 0)
         break;

      // Here we have valid data..  Do whatever...


      // Now, redirect somewhere.
      header('Location: http://www.next.com/whatever');
      exit;

 }

 ?>
 <html>
    <body>
       <?php if(count($Errors)) { ?>
          <div class="Error">
              <?php foreach($Error as $Error) { ?>
                  <div><?php echo htmlspecialchars($Error); ?></div>
              <?php } ?>
          </div>
       <?php } ?>

       <form method="POST" action="<?php echo htmlspecialchars($_SERVER['REQUES_URI'], ENT_QUOTES); ?>" />
           <input type="hidden" name="FormAction" value="Process" />

           First Name: 
           <input type="text" name="FirstName" value="<?php echo htmlspecialchars($FirstName, ENT_QUOTES); ?>" />

           ...

           <input type="submit" />
       </form>

    </body>
 </html>

gahooa 2009-12-14 00:23:02

ansaurus

tags:

views:

answers:

php contact form clean code

related questions