5
\$\begingroup\$

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?

\$\endgroup\$
7
\$\begingroup\$

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'];
\$\endgroup\$
  • \$\begingroup\$ I used the "return true" as the return type for validateName method, so I can test in my Register 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) into index.php file. Any idea about how should I do that? Thanks! \$\endgroup\$ – Dan Costinel Jul 6 '15 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\$ – Stop ongoing harm to Monica Jul 6 '15 at 11:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.