Email Form Validation Not Working - Newbie

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
Alex Yeh
Forum Newbie
Posts: 2
Joined: Wed Apr 12, 2006 4:40 pm

Email Form Validation Not Working - Newbie

Post by Alex Yeh »

Thanks for your time. I put together this PHP code to process a form on a website and send an email. It works fine, but the validation doesn't seem to work. Here's the code:

Code: Select all

<?php
// get posted data into local variables
$EmailTo='admin@127.0.0.1';
$EmailFrom=Trim(strip_tags(stripslashes($_POST['from'])));
$Subject='Appointment Request'; 
$first=Trim(strip_tags(stripslashes($_POST['first']))); 
$last=Trim(strip_tags(stripslashes($_POST['last']))); 
$area=Trim(strip_tags(stripslashes($_POST['area']))); 
$phone=Trim(strip_tags(stripslashes($_POST['phone']))); 
$callwhen=Trim(strip_tags(stripslashes($_POST['callwhen'])));
$apptwhen=Trim(strip_tags(stripslashes($_POST['apptwhen'])));
$whatserv=Trim(strip_tags(stripslashes($_POST['whatserv']))); 
$special=Trim(strip_tags(stripslashes($_POST['special']))); 
// validate - this is where I'm having a problem
$validationOK == true;
$hasatmark = (strstr($EmailFrom, \"@"));
if ($hasatmark == false)
	$validationOK == false;
else if (strlen($EmailFrom) < 
	$validationOK == false;
else if (strlen($first) < 2)
	$validationOK == false;
else if (strlen($last) < 2)
	$validationOK == false;
else if (strlen($area) > 3)
	$validationOK == false;
else if (strlen($phone) < 10)
	$validationOK == false;
if ($validationOK == true) {
  print "<meta http-equiv=\"refresh\" content=\"0;URL=error.html\">";
  exit;
}

// this will prepare email body text
$Body = "\n"
		."\n"
		."First Name: "
		.$first
		."\n"
		."\n"
		."Last Name: "
		.$last
		."\n"
		."\n"
		."Area Code: "
		.$area
		."\n"
		."\n"
		."Phone Number: "
		.$phone
		."\n"
		."\n"
		."Best Time To Call: "
		.$callwhen
		."\n"
		."\n"
		."Would Like An Appointment On: "
		.$apptwhen
		."\n"
		."\n"
		."Would Like These Services: "
		.$whatserv
		."\n"
		."\n"
		."Special Requests Are: "
		.$special
		."\n";


// send email 
$success = mail($EmailTo, $Subject, $Body, $EmailFrom);

// redirect to success or failure page 
if ($success){
  print "<meta http-equiv=\"refresh\" content=\"0;URL=ok.html\">";
}
else{
  print "<meta http-equiv=\"refresh\" content=\"0;URL=error.html\">";
}
?>
Any insight here? Thanks again.
User avatar
ambivalent
Forum Contributor
Posts: 173
Joined: Thu Apr 14, 2005 8:58 pm
Location: Toronto, ON

Post by ambivalent »

Code: Select all

$validationOK == true
$validationOK == false
The true/false assignments should be:

Code: Select all

$validationOK = true
$validationOK = false
printf
Forum Contributor
Posts: 173
Joined: Wed Jan 12, 2005 5:24 pm

Post by printf »

Don't you think it's a little silly, doing all that un-needed stuff when each variable might not even exist. Good logic will tell you, that you should only do things that need to be done. Doing more is not only wasteful, it most times leads to other parts of your code being left open to undesired actions. I understand when learning, people tend to read more into security advisories, which leads to more problems than anything good being learned. Also, like with anything you do, there is never just one way to achieve your goal, but following simple logical rules will help you in not learning bad coding habits!


So you should do something like this....

1. check if the SUPER GLOBAL $_POST has any data

Code: Select all

if ( ! empty ( $_POST ) )
{
	// do stuff
}

2. if the $_POST array has data, check if you need use strip slashes on each SUPER GLOBAL array

Code: Select all

if ( get_magic_quotes_gpc () )
{
	function magic_quotes_remove ( &$a )
	{
		if ( is_array ( $a ) )
		{
			foreach ( $a AS $k => $v )
			{
				if ( is_string ( $v ) )
				{
					$a[$k] = stripslashes ( $v );
				}
				else if ( is_array ( $v ) )
				{
					$a[$k] = magic_quotes_remove ( $v );
				}
			}
		}

		return ( $a );
	}

	if ( ! empty ( $_POST ) )
	{
		$_POST = magic_quotes_remove ( $_POST );
	}

	if ( ! empty ( $_GET ) )
	{
		$_GET  = magic_quotes_remove ( $_GET );
	}

	if ( ! empty ( $_COOKIE ) )
	{
		$_COOKIE = magic_quotes_remove ( $_COOKIE );
	}

	if ( ! empty ( $_FILES ) )
	{
		foreach ( $_FILES AS $k => $v )
		{
			$_FILES[$k]['tmp_name'] = str_replace('\\', '/', $v['tmp_name'] );
		}

		$_FILES = magic_quotes_remove ( $_FILES );
	}

	set_magic_quotes_runtime ( 0 );
}
Then after, run your validation

Code: Select all

if ( ! isset ( $_POST['from'] ) || ! preg_match ( '|^([._a-z0-9-]+[._a-z0-9-]*)@(([a-z0-9-_]+.)*([a-z0-9-]+)(.[a-z]{2,4}))$|i', $_POST['from'] ) )
{
	$validationOK = false;
}

// ... next one
By doing this, your code has a good layer of logic, and you don't have to do un-needed things, because you logic contains everything you want the script process to allow!

pif!
Alex Yeh
Forum Newbie
Posts: 2
Joined: Wed Apr 12, 2006 4:40 pm

Post by Alex Yeh »

brilliant!

thanks very much, printf and ambivalent!

:D
Post Reply