Critique and help improve my contact form

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Critique and help improve my contact form

Post by Sindarin »

I want to create a contact form but this time I want to optimize it.

1) Is it secure enough? Can it prevent injections? (I am also going to add captcha in it later)

2) The contact.php form page executes send.php after the submit button is pressed. How can I make it so that the errors like "First name field is not complete." appear at the top of my contact form instead of a new white page? I had an idea for writing those error messages to a file and including them, but it would be a problem if multiple users were using the form at the same time. Isn't a way to pass those messages as variables to the contact form?

send.php

Code: Select all

<?php
 
//Contact Form
 
//the sending address [From]
$from = "info@mydomain.com";
 
//the destination address [To]
$to = "info@mydomain.com";
 
//the message subject
$subject = "New Contact";
 
//ip list filename
$ip_file="ip-list.txt";
 
//function email_validate
function email_validate ($email) 
{
    if (strlen (trim ($email)))
        return (eregi("^[a-z0-9]([_\\.\\-]?[a-z0-9]+)*@((([a-z0-9]+[a-z0-9\\-]*[a\-z0-9]+)|[a-z0-9])+\\.)+[a-z]{2,10}$", $email));
    return false;
}   
 
//get variables from form
 
$first = $_POST['first']; //first name
$last = $_POST['last']; //last name
$day = $_POST['day'];
$month = $_POST['month'];
$year = $_POST['year'];
$address = $_POST['address'];
$number = $_POST['arithm'];
$city = $_POST['city'];
$tk = $_POST['tk']; //zip code
$email = $_POST['email'];
$url = $_POST['url']; //how you learned about us, survey question
$message = $_POST['message']; //user message
 
//get IP address
$ip=$_SERVER['REMOTE_ADDR'];
 
//read file and check the IPs from the array
 
$ip_id=fopen($ip_file,"r");
$ip_array=fread($ip_id,9999);
fclose($ip_id);
$ip_array = explode("\n", $ip_array);
 
//check the IP
if(in_array($_SERVER['REMOTE_ADDR'], $ip_array)) 
{
//blocked IP returned, display error and stop submission
echo "<center><font family='Verdana'><font >We are sorry but your IP address <font color='red'><b>$ip</b></font> has been blocked from using the contact form.<br /></font></center>";
}
else
{
 
//IP not in blocked IP list, allow contact
 
//check the required fields and validate email
$email_is_valid=email_validate($email);
if ($first=="" || $last=="" || $message=="" || $email=="" || $email_is_valid==false)
{
//some of the required fields are not filled
if ($first==""){echo "First name field is not complete.<br />";}
if ($last==""){echo "Surname field is not complete.<br />";}
if ($message==""){echo "Message field is not complete.<br />";}
if ($email_is_valid==false){echo "The email address "; if ($email==""){echo "is not complete and ";} echo "not valid.<br />";}
}
else
{
//all required fields are completed
 
//wrap characters so the message shows shorter
$wrappedmessage =wordwrap($message, 50, "<br />\n");
    
//set the message content
$form_message = "
 
This is the email that is delivered. It's going to have all variables passed from the form in it.
 
";
    
//set the mail headers
$headers = "MIME-Version: 1.0\r\n";
$headers .= "Content-type: text/html; charset=utf-8\r\n";
$headers .= "To: <$from> \r\n";
$headers .= "From: <$to>\r\n";
    
//send the message
mail($to, $subject, $form_message, $headers);
 
    //output success html to the user
echo "<center><p style='a:link {color: #840E64;text-decoration: none;}'><font color='black'><center>message success<!center></font><br /><a href='javascript&#058;history.back(-1);'><b>[Back]</b></a></p></center>";
 
}
}
 
?>
 
timsewell
Forum Newbie
Posts: 21
Joined: Tue Feb 26, 2008 5:43 am
Location: Hove, England

Re: Critique and help improve my contact form

Post by timsewell »

I'm new to this, but on an error I redirect back to the content page with all the form data and highlighted (with a '-!!') error-containing fields passed in the $_SESSION array. Once back at the contact form I foreach through that array, removing the '-!!' and assigning a new css class to the fields containing errors with a generic error message above saying that the entries are incomplete or invalid. The fields, of course, contain the user-entered information.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: Critique and help improve my contact form

Post by Benjamin »

I rewrote parts of this but it could still use some work. View the comments that start with a #

Code: Select all

 
<?php
 
# I'd define these as constants for clarity, organization and to help prevent unexpected bugs
//the sending address [From]
//$from = "info@mydomain.com";
define('EMAIL_FROM', "info@mydomain.com");
 
//the destination address [To]
//$to = "info@mydomain.com";
define('EMAIL_TO', "info@mydomain.com");
 
//the message subject
//$subject = "New Contact";
define('EMAIL_SUBJECT', "New Contact");
 
//ip list filename
//$ip_file="ip-list.txt";
define('IP_LIST_PATH', "ip-list.txt");
 
//function email_validate
# eregi is being phased out
function email_validate($email)
{
    if (strlen (trim ($email)))
    {
        return (eregi("^[a-z0-9]([_\\.\\-]?[a-z0-9]+)*@((([a-z0-9]+[a-z0-9\\-]*[a\-z0-9]+)|[a-z0-9])+\\.)+[a-z]{2,10}$", $email));
    }
    return false;
}
 
function remove_line_feeds($var)
{
    return str_replace(array("\r", "\n"), '', $var);
}
 
//get variables from form
 
# You are assuming these $_POST variables are populated.
# Best practice is to check if they are set
# $var = (isset($_POST['field_name'])) ? trim($_POST['field_name']) : null;
# Added function to remove line feeds
$first = remove_line_feeds($_POST['first']); //first name
$last = remove_line_feeds($_POST['last']); //last name
$day = remove_line_feeds($_POST['day']);
$month = remove_line_feeds($_POST['month']);
$year = remove_line_feeds($_POST['year']);
$address = remove_line_feeds($_POST['address']);
$number = remove_line_feeds($_POST['arithm']);
$city = remove_line_feeds($_POST['city']);
$tk = remove_line_feeds($_POST['tk']); //zip code
$email = remove_line_feeds($_POST['email']);
$url = remove_line_feeds($_POST['url']); //how you learned about us, survey question
$message = $_POST['message']; //user message
 
//get IP address
$ip = $_SERVER['REMOTE_ADDR'];
 
//read file and check the IPs from the array
//$ip_id=fopen($ip_file,"r");
//$ip_array=fread($ip_id,9999);
//fclose($ip_id);
//$ip_array = explode("\n", $ip_array);
# we can do that in one line
$ip_array = explode("\n", file_get_contents(IP_LIST_PATH));
 
//check the IP
if(in_array($_SERVER['REMOTE_ADDR'], $ip_array))
{
    //blocked IP returned, display error and stop submission
    echo "<center><font family='Verdana'><font >We are sorry but your IP address <font color='red'><b>$ip</b></font> has been blocked from using the contact form.<br /></font></center>";
} else {
    //IP not in blocked IP list, allow contact
 
    //check the required fields and validate email
    $email_is_valid=email_validate($email);
 
    if ($first=="" || $last=="" || $message=="" || $email=="" || $email_is_valid==false)
    {
        //some of the required fields are not filled
        if ($first==""){echo "First name field is not complete.<br />";}
        if ($last==""){echo "Surname field is not complete.<br />";}
        if ($message==""){echo "Message field is not complete.<br />";}
        if ($email_is_valid==false){echo "The email address "; if ($email==""){echo "is not complete and ";} echo "not valid.<br />";}
    } else {
        //all required fields are completed
 
        //wrap characters so the message shows shorter
        $wrappedmessage =wordwrap($message, 50, "<br />\n");
   
        //set the message content
        $form_message = "\n\nThis is the email that is delivered. It's going to have all variables passed from the form in it.\n\n";
   
        //set the mail headers
        $headers = "MIME-Version: 1.0\r\n";
        $headers .= "Content-type: text/html; charset=utf-8\r\n";
        # added Return-Path header
        $headers .= "Return-Path: " . EMAIL_FROM . "\r\n";
        $headers .= "To: <" . EMAIL_TO . "> \r\n";
        $headers .= "From: <" . EMAIL_FROM . ">\r\n";
   
        //send the message
        mail(EMAIL_TO, EMAIL_SUBJECT, $form_message, $headers);
 
        //output success html to the user
        echo "<center><p style='a:link {color: #840E64;text-decoration: none;}'><font color='black'><center>message success<!center></font><br /><a href='javascript&#058;history.back(-1);'><b>[Back]</b></a></p></center>";
 
    }
}
 
User avatar
Sindarin
Forum Regular
Posts: 521
Joined: Tue Sep 25, 2007 8:36 am
Location: Greece

Re: Critique and help improve my contact form

Post by Sindarin »

define('EMAIL_FROM', "info@mydomain.com");
so define, declares a constant?
function remove_line_feeds($var)
{
return str_replace(array("\r", "\n"), '', $var);
}
Injection protection I assume?
containing fields passed in the $_SESSION array.
Can someone direct me on how session variables work? Are they some kind of client/server stored global temporary variables?

And how can I implement it in my form? I submit to another php script. Do I need ajax for it to work?
timsewell
Forum Newbie
Posts: 21
Joined: Tue Feb 26, 2008 5:43 am
Location: Hove, England

Re: Critique and help improve my contact form

Post by timsewell »

On the 'sending page', ie. your form processing/error checking script, something like:

Code: Select all

$_SESSION[username] = $_POST[username];
// Assuming $username is your variable
 
On the 'receiving page', ie. your form:

Code: Select all

$username = $_SESSION[username];
 
Then in the HTML I have:

Code: Select all

<input type="text" name="username" value="<?php echo "$username"; ?>" />

If there's nothing in the $username variable, ie. when the form is accessed for the first time, then you just get value="", but if a value has been passed back via the $_SESSION array then that value is displayed.

In practise I usually have:

Code: Select all

foreach($_POST as $key => $value) {
$_SESSION[$key] = $value;
}
Once I've done that I then do my error checking on the $_POSTed values, adding a flag to any of the $_SESSION values which correspond to incorrect fields before redirecting back to the form if errors have been found (which I do by adding 1 to a $error_count variable for each incorrect field so that all the errors can be dealt with in one return trip.

I don't know if this is good coding practise, but it works for me.

*Edited a couple of times for clarity
User avatar
andym01480
Forum Contributor
Posts: 390
Joined: Wed Apr 19, 2006 5:01 pm

Re: Critique and help improve my contact form

Post by andym01480 »

Don't forget

Code: Select all

session_start();
at the very start of both files after <?php and before any browser output!
Post Reply