Can Someone Critique My PHP Form Validation?

[cybl00]

Hey all:

I have a little background in programming (C/C++, ADA, and JES version of Python) but I am by no means an expert on anything. This is the first time I’ve “programmed” in PHP and it’s taken me about two weeks to get it to work. Would any of you guys be willing to critique my code so that I can further my learning experience from this project?

Some of the parts I was a little iffy on were using strip_tags() and htmlspecialchars() together – is it overkill, what is the best form of practice? I also think that there is probably a faster/better/shorter way of handling displaying errors. I did a lot of Google searching but was having a hard time understanding the tutorials.

Thanks!

Form HTML:

[code]

Subject Request Service Comment Question Report a Site Problem

Full Name

E-mail

Message

[/code]

Hello.php
[php]

<?php /* Variables */ $name = strip_tags(htmlspecialchars($_POST['name'], ENT_QUOTES)); $email = strip_tags(htmlspecialchars($_POST['email'], ENT_QUOTES)); $subject = $_POST['subject']; $message = strip_tags(htmlspecialchars($_POST['message'], ENT_QUOTES)); $form_errorFlag = array(0,0,0); /*Verify all fields completed*/ if (empty($name) || empty($email) || empty($message)) { $form_errorFlag[0] = 1; } else { $form_errorFlag[0] = 0; } /*Check Subject*/ switch ($_POST['subject']) { case 'request': case 'comment': case 'question': case 'report': break; default: $form_errorFlag[1] = 1; break; } /*Check E-mail*/ $patternEmail = '/^([\w\d.-]+)@([\w\d-]+)\.([\w\d.-]+)$/'; /*valid e-mail characters*/ if (!preg_match($patternEmail, $email)) { $form_errorFlag[2] = 1; } $form_error = array('empty' => 'Some or all fields are missing. Please fill out all fields.', 'subject' => 'Invalid option selection for subject.', 'email' => 'Invalid or missing e-mail address', ); /*Print Statements*/ if ( $form_errorFlag[0] == 0 && $form_errorFlag[1] == 0 && $form_errorFlag[2] == 0) { echo "Thank you! We will reply as soon as possible."; } elseif ($form_errorFlag[0] == 1 && $form_errorFlag[1] == 0 && $form_errorFlag[2] == 0) { echo $form_error['empty']; echo "
Please click your browser's back button to fix form."; exit; } elseif ($form_errorFlag[0] == 1 && $form_errorFlag[1] == 1 && $form_errorFlag[2] == 0) { echo $form_error['empty']; echo $form_error['subject']; echo "
Please click your browser's back button to fix form."; exit; } elseif ($form_errorFlag[0] == 1 && $form_errorFlag[1] == 0 && $form_errorFlag[2] == 1) { echo $form_error['empty']; echo $form_error['email']; echo "
Please click your browser's back button to fix form."; exit; } elseif ($form_errorFlag[0] == 1 && $form_errorFlag[1] == 1 && $form_errorFlag[2] == 1) { echo $form_error['empty']; echo $form_error['subject']; echo $form_error['email']; echo "
Please click your browser's back button to fix form."; exit; } elseif ($form_errorFlag[0] == 0 && $form_errorFlag[1] == 1 && $form_errorFlag[2] == 1) { echo $form_error['subject']; echo $form_error['email']; echo "
Please click your browser's back button to fix form."; exit; } elseif ($form_errorFlag[0] == 0 && $form_errorFlag[1] == 0 && $form_errorFlag[2] == 1) { echo $form_error['email']; echo "
Please click your browser's back button to fix form."; exit; } else { echo $form_error['subject']; echo "
Please click your browser's back button to fix form."; exit; } /*Format of Email sent from form*/ mail( "[email protected]", "Contact Form Submission", "From: $email Name: $name Subject: $subject Comments: $message"); ?>[/php]

[smokeys_13]

Hi there,

Nice code, but could be done much quicker. I personally tend to use code along the lines of:

[php]
$defaults = array(
“name” => “”,
“email” => “”,
“message” => “”
);
if(isset($_POST[‘contact_sub’]))
{
$errors = array();
$clean = array();
$clean[‘name’] = trim(strip_tags($_POST[‘name’]));
$clean[‘email’] = trim(strip_tags($_POST[‘email’]));
$clean[‘message’] = trim(strip_tags($_POST[‘message’]));

/*
 * Validate the name field
 */
if($clean['name'] == "")
{
	$errors[] = "Please enter a name.";
}
else if(strlen($clean['name']) < 3)
{
	$errors[] = "The name must be at least 3 characters.";
}
else
{
	$defaults['name'] = $clean['name'];
}

/*
 * Validate the email field
 */
if($clean['email'] == "")
{
	$errors[] = "Please specify an e-mail address.";
}
else if(!preg_match("/[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})/", $clean['email']))
{
	$errors[] = "Please enter a valid e-mail address.";
}
else
{
	$defaults['email'] = $clean['email'];
}

/*
 * Validate the message field
 */
if($clean['message'] == "")
{
	$errors[] = "Please type a message.";
}
else if(strlen($clean['message']) < 10)
{
	$errors[] = "Please enter a message that is 10 characters or longer.";
}
else
{
	$defaults['message'] = $clean['message'];
}


$email = "[email protected]";
$subject = "Contact Form Submission";
$message = "The contact form on example.com has been submitted:
			<br />
			<br />
			<strong>Name:</strong> ".$clean['name']."
			<br />
			<strong>Email:</strong> ".$clean['email']."
			<br />
			<strong>Message:</strong> ".$clean['message'];

if(empty($errors))
{
	$headers  = 'MIME-Version: 1.0' . "\r\n";
	$headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
	$headers .= 'From: My Website <[email protected]>' . "\r\n";
	$headers .= 'Cc: [email protected]' . "\r\n";
	$success = mail($email, $subject, $message, $headers);
	$output = ($success) ? "Thank you. An e-mail has been sent." : "There was an internal server problem. Please try again later.";
	if($success === true)
	{
		$defaults =	array(
			"name" => "",
			"email" => "",
			"message" => ""
		);
	}
}
else
{
	$output = implode("\n<br />\n",$errors);
}

}
[/php]

and then in the value of each input (or between textarea tags) i would put:

<?php echo $defaults['name']; ?>

(or whichever $defaults key is appropriate)

Hope this helps!