form validation, beginner's attempt

[turpentyne]

I'm trying to wrap my head around form validation and I want good security. I've looked at a few tutorials to get the idea of what's out there, and came up with this "beginner's script." Before I go blowing up a page to implement it, would this work? Is it secure? Also, I'm not sure how to code in the error text for each one, so that I can print all the errors out at the end, i.e. "you didn't fill out your name" and "your email is incorrect", etc.

 

 

<?php

$error=0;

if ((isset($_POST['fname'])) && ($_POST['fname'] !='') && ( (strlen(trim($_POST[fname])) !< 3) || (strlen(trim($_POST[fname])) !> 25)))
{   
if(!preg_match("/[^a-zA-Z\.\-\Ä\ä\Ö\ö\Ü\ü\ ]+$/s",$_POST['fname']))
    {
    echo "There was an error in the first name field";
    $error++; // $error=$error+1;
    
    } else {
$fname = $_POST['fname'];
$fname = mysql_real_escape_string(htmlspecialchars(strip_tags($fname)));
return $fname;
$_SESSION['s_fname'] = $fname;
}
} else { echo "We need your full name";
    $error++; // $error=$error+1;
}
   
    // do the same for another 5 fields, then...


   if ($error > 0) { echo "you have not filled out the form properly:
   // error stack here?
   } else  {
?>
<!-- rest of page/php code -->
<?php
   }
?>

[scootstah]

This is one of those times where reusing code is of massive benefit. Otherwise, you're going to have repeated logic all over the place, which is not a good way to work. So, in lieu of that I whipped up this class. I did this in only a few minutes so, I'm sure there's some catastrophic failure to be had. However, from my few tests it appears to work alright. If anything, it's a place to start.

class FormValidation
{
private $errors = array();

private $rules = array();

private $error_msgs;

public function __construct()
{
	// set some error messages
	$this->error_msgs = array(
		'required' => '%s is a required field'
	);
}

public function setRules($rules)
{
	// add some rules
	$this->rules = array_merge($this->rules, $rules);
}

public function getErrors($field = null)
{
	// if no field is specified, get all errors
	if ($field == null) {
		return $this->errors;
	}

	// otherwise try to return an error for the specific field
	return isset($this->errors[$field]) ? $this->errors[$field] : null;
}

public function run()
{
	// if POST is empty, do nothing
	if (empty($_POST))
		return true;

	// loop through the rules
	foreach($this->rules as $rules)
	{
		$field = $rules['field'];
		$label = $rules['label'];

		$rules = explode('|', $rules['rules']);

		// loop through and test each rule
		foreach($rules as $rule)
		{			
			// do we have a method to handle this rule?
			if (method_exists($this, $rule)) {
				$result = $this->$rule($field);

			// nope. how about a function?
			} else if (function_exists($rule)) {					
				if (isset($_POST[$field])) {
					$result = $rule($_POST[$field]);
				}
			}

			// no problems, NEXT!
			if (!isset($result))
				continue;

			// if $result is a bool and FALSE, set an error 
			if ($result === false) {
				$this->setError($field, $label, $rule);

			// otherwise, set the $_POST value to the one returned
			} else {
				$_POST[$field] = $result;
			}
		}
	}

	// return a bool
	return empty($this->errors);
}

private function setError($field, $label, $rule)
{
	// set an error
	$this->errors[$field] = sprintf($this->error_msgs[$rule], $label);
}

private function required($field)
{
	return isset($_POST[$field]) && !empty($_POST[$field]);
}
}

 

And to use it

// instantiate the class
$val = new FormValidation;

// set some rules
// each array represents a single field
// the "field" is the name of the form element
// the "label" is a friendly name (what appears in the error)
// the "rules" is the criteria it must meet to pass
// you can separate rules with a vertical pipe ( | ),
// and use any function that accepts a single parameter
// or any methods for the FormValidation class
$val->setRules(array(
array('field' => 'firstname', 'label' => 'First Name', 'rules' => 'required'),
array('field' => 'lastname',  'label' => 'Last Name',  'rules' => 'required')
));

if ($val->run()) {
echo 'PASSED';
} else {
echo 'DIDNT PASS';

print_r($val->getErrors());
}

[Muddy_Funster]

@ turpentyne : The function that you took your code from isn't being used any more, so you don't need the

return $fname;

line in there any more...unless this code is all from within a sanitise function, you have chopped off some rather sizeable chunks.  Try scootstah's class, and have a read through it, he's taken time out to comment (as too few do these days - myself included most of the time) what it's doing so you can learn a bit more from using it.

 

To display all the errors you would just build an error array and loop through it at the end:

$error =  array();
if(...){
...
}else{
...
$error[] = "first error message";
}
if(...){
...
}else{
...
$error[] = "second error message";
}
if(...){
...
}else{
...
$error[] = "third error message";
}
if(count($error >= 1){
echo "The following errors occured when processing your submition:<br />";
foreach($error as $message){
echo "$message <br />";
}
}

[phppup]

Hard to believe I'm GIVING advice.  I suppose it's testimony to the learning that takes place here.

I noticed in your first entry that you have error messages that state:

"There was an error in the first name field" AND

""You have not filled out the form properly."

 

This is a BAD idea that will aggrivate users.  Instead of telling them that there IS an error, tell them WHAT the error is so that it can be corrected.

 

Use messages that explain WHY the error was triggered:

"You MUST provide your name in the NAME field."

"Only alphabetical letters without spaces can be used when filling out your name."

"Please use ONLY numbers when providing your telephone number," etc.

 

Use what is applicable for each independent error message.