I'm kinda new to PHP and I'm trying to make a script to validate a HTML form only with PHP (I know there are options to do so with JS, but I guess it's better to have a "cover" with a server-side script).
Project folder structure:
project/ classes/ #contains classes to validate and process the HTML form DB.php #contains the connection to the MySQL server Rules.php #contains all rules to validate the form Validate.php #contains methods to validate each field Register.php #is the registration class ini.php #contains the spl_autoload_register() function index.php #contains the HTML form
HTML form code, which is located in the project folder:
<?php include_once 'lib/csrf-magic/csrf-magic.php; ?>
<form method="POST" action="classes/Register.php">
<fieldset>
<legend><strong>register:</strong></legend>
* <input type="text" name="fname" placeholder="First Name"/><br/>
<div style="float:left;">
<?php
// I want to put here all the validation errors for the fname field, if any
?>
</div>
<input type="submit" name="submit" value="Register"/>
</fieldset>
</form>
classes/ini.php
error_reporting(E_ALL);
ini_set('display_errors','On');
spl_autoload_register(function($class){
require_once $class.'.php';
});
classes/DB.php
class DB
{
private $driver = 'mysql';
private $host = '127.0.0.1';
private $user = 'root';
private $pass = '';
private $name = 'project';
public $db;
public function __construct()
{
try
{
$this->db = new PDO("$this->driver:host=$this->host;dbname=$this->name;charset=utf8", $this->user, $this->pass);
//Error reporting. Throw exceptions
$this->db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
//Use native prepared statements
$this->db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
//Don't use persistent connections across multiple server sessions
$this->db->setAttribute(PDO::ATTR_PERSISTENT, false);
//Set default fetch mode
$this->db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_OBJ);
//Return the connection object
return $this->db;
} catch (PDOException $e)
{
#echo 'Sorry. We have some problemes. Try again later!';
echo $e->getMessage();
}
}
}
classes/Rules.php
class Rules
{
/**
* used for displaying in the page the user input text
*/
public static function escape($str)
{
return htmlspecialchars(trim($str),ENT_QUOTES,'UTF-8');
}
/**
* used to allow only numbers, letters (lc+uc), a space, an underscore, and a dash for a username. this will be stored into db
*/
public static function filter($str)
{
return preg_replace('#[^0-9a-z _-]#i', "", trim($str));
}
/**
* used to determine if the input has a certain minimum length
*/
public static function minlen($str, $value)
{
return mb_strlen(trim($str)) < $value ? true : false;
}
/**
* used to determine if the input has a certain maximum length
*/
public static function maxlen($str, $value)
{
return mb_strlen(trim($str)) > $value ? true : false;
}
/**
* used to determine if the password has a certain minimum length
*/
public static function passLen($str)
{
return mb_strlen(trim($str)) < 6 ? true : false;
}
/**
* used to determine if two passwords are equal
*/
public static function equals($pass1, $pass2)
{
return trim($pass2) === trim($pass1) ? true : false;
}
/**
* used to determine if a textbox contain a valid email
*/
public static function email($email)
{
return filter_var(trim($email),FILTER_VALIDATE_EMAIL) ? true : false;
}
/**
* used to determine if a user has wrote some date into the textbox
*/
public static function required($str)
{
return trim($str) === '' ? true : false;
}
}
classes/Validation.php
require 'ini.php';
class Validation{
public static function validateName($name){
$errors['name'] = [];
$name = Rules::filter($name);
if (Rules::required($name)) {
$errors['name'][] = 'The Name field is mandatory<br/>';
}
if (Rules::minlen($name, 3)) {
$errors['name'][] = 'The Name field is too short<br/>';
}
if (Rules::maxlen($name, 50)) { // this field is 50 chars long into the db table
$errors['name'][] = 'The Name field is too long<br/>';
}
if (isset($errors['name'])) {
return $errors['name'];
} else {
return true;
}
}
}
classes/Register.php
require_once 'ini.php';
class Register
{
private $db;
public function __construct(DB $db)
{
$this->db = $db->db;
}
public function reg()
{
$fname = isset($_POST['fname']) ? $_POST['fname'] : ''; // I' use HTMLPurifier for this variable
if (isset($_POST['submit'])) {
if (Validation::validateName($fname)) {
$stmt = $this->db->prepare('INSERT INTO users SET fname=:fn, lname=:ln');
$stmt->bindValue(':fn', $fname, PDO::PARAM_STR);
$stmt->exeecute();
} else {
foreach (Validation::validateName($fname) as $e) {
return $e;
}
}
}
}
$d = new DB();
$r = new Register($d);
echo $r->reg();
Now, I want to put all the validation errors that will occur when the user submits the form w/o filling accordingly all the fields required, into the <div>
located down under the <input type="text" name="fname">
field. The errors are in the foreach
loop located into the Register
class. Another major aspect is the security: how should I improve my code?
1 Answer 1
Use boolean expressions directly
Rules.php
is full of ternaries that return true
or false
,
like this one:
public static function minlen($str, $value) { return mb_strlen(trim($str)) < $value ? true : false; }
You can return boolean expressions directly:
return mb_strlen(trim($str)) < $value;
Don't repeat yourself
All the rules trim
the input.
Instead of writing trim
in all of them,
it would be simpler to trim
once before using the rule methods,
so that you don't have to repeatedly write trim
so many times in every single method.
Don't return two types of values
The validateName
may return two kinds of values:
an array of errors, or true
(boolean).
if (isset($errors['name'])) { return $errors['name']; } else { return true; }
This is poor design. Return one type of value. If there are no errors, return an empty array. That way the return type will be consistently an array, which is a good thing. The code snippet above will become simpler too:
return $errors['name'];
-
\$\begingroup\$ I used the "return true" as the return type for
validateName
method, so I can test in myRegister
reg()
method: if there are no errors (return true
), then insert data into db, else I need to somehow redirect the errors (for all fields) intoindex.php
file. Any idea about how should I do that? Thanks! \$\endgroup\$Dan Costinel– Dan Costinel2015年07月06日 10:18:41 +00:00Commented Jul 6, 2015 at 10:18 -
\$\begingroup\$ Yes, I suggest to return an array always. When there are no errors, the array will be naturally empty. Users of this method can check the size of the returned array, empty array means success. \$\endgroup\$janos– janos2015年07月06日 11:32:25 +00:00Commented Jul 6, 2015 at 11:32