PDO Methods

General PHP Help
#1

Hello, please can some one help me figure this out…

I use pdo and create classes with the methods i need for each page or script. This has become a bit of a pain as now i have many methods inside many classes and i want 90% of them to come from one class that i’ll call CRUD and inside just have insert, select, update, delete methods.

How can i get around sql injections in my select example bellow… The idea is to end up with dynamic selecting etc to cut down the number of methods i have!

class CRUD {

    private $pdo;

    public function __construct(PDO $pdo) {

        $this->pdo = $pdo;

    }

    public function select($table) {

        $sql = "SELECT * FROM ";

        $sql .= $table;

        $stmt = $this->pdo->prepare($sql);

        $stmt->execute();

        return $stmt->fetch();

    }

}

$sitePDO = (new CRUD($sitePDO));

var_dump($sitePDO->select('settings')['site_name']);

Thanks for your time, hope all are well in these crappy times!

#2

One option would be to ensure that your $table variable only contains “safe” characters, something like:

public function select($table) {
    // Check that table only contains letters, numbers or underscore characters...
    if (preg_match('/^\w+$/', $table) !== 1) {
        throw new Exception($table . ' is not a safe table name');
    }

    // Table name is safe, keep doing what you were doing...

I would back up and consider how you’ll use this code though. Having to specify the table name in your client code makes it easy to make mistakes, and difficult to change you db structure later on - imagine having to check every occurence of the word 'settings' in your codebase if you want to change some aspect of it, for example.

If you do take this route, you should have a “settings” class that handles all interaction with your settings storage; that way you only have to change things in one place.

#3

Or you can simply make sure your $table is protected:

protected static string $table = "cms"; // Table Name:

a part of a class:

class Login extends DatabaseObject
{
    protected static string $table = "admins";

    public $id;
    public $first_name;
    public $last_name;
    public $email;
    public $username = [];
    static public $error = [];
    protected string $password;
    static public $last_login;

    public const MAX_LOGIN_AGE = 60*60*24*7; // 7 days:

    public function __construct($args = [])
    {
        static::$searchItem = 'username';
        static::$searchValue = htmlspecialchars($args['username']);
        $this->password = htmlspecialchars($args['hashed_password']);
    }

    public static function full_name(): string
    {
        static::$searchItem = 'id';
        static::$searchValue = $_SESSION['id'];
        $sql = "SELECT first_name, last_name FROM " . static::$table . " WHERE id =:id LIMIT 1";
        $user = static::fetch_by_column_name($sql);

        return $user['first_name'] . " " . $user['last_name'];
    }

and the database object class that actually does it:

public static function fetch_by_column_name($sql)
{
    $stmt = Database::pdo()->prepare($sql);

    $stmt->execute([ static::$searchItem => static::$searchValue ]);
    return $stmt->fetch(PDO::FETCH_ASSOC);
}

public static function fetch_all_by_column_name($sql): array
{
    $stmt = Database::pdo()->prepare($sql);

    $stmt->execute([ static::$searchItem => static::$searchValue ]);
    return $stmt->fetchAll(PDO::FETCH_ASSOC);
}

Anything you DON’T want the user to be able to change have the variable or method private or protected. FYI - even the hashed_password word is unset after the password is check as I just do that for extra protection. I know it’s an oxymoron way of doing it, but it’s better to be safe than sorry.

#4

So i can manipulate the sql query however i want as long as the data going into it is what i expect it to be and sanitized?

#5

This is based off the Active Record Design Pattern, but the more popular pattern that people do is the MVC pattern. The reason I chose that pattern is I don’t think my websites will get that complex or lengthy, but if I do I will switch over to MVC. I really can’t answer that if you are doing it the MVC way, but I would image as long as you are using are creating the the sql link yourself, sanitizing the variables (which I even do this way) and using prepared statements then you should be OK. I just want to point out that I haven’t done any validation yet as right now it’s only me that can login, but if I open it up for everyone then I will have to do the validation. (I get lazy at times when it comes to coding)

#6

Take a look at my Clean Pdo example

#7

You can, but it’s difficult to keep track of what’s been sanitised and what hasn’t. It’s generally best practice to sanitise stuff as late as possible - for PHP, this generally means using prepared statements as provided by the PDO library.

#8

Thanks for all your help, i’ll have a look into benanamen/clean-pdo and post back with what i come up with!

#9

Hi, I realy like your clean-pdo example! I’ve learnt alot from it and i’m best off with the structure I have or i’ll end up with sql all over the place… It made me realise that by creating classes and methods for different jobs is a good idea and keeps all database querys away from my code logic.

Please can you explain what is happening on this line

(string $sql, array $args = NULL): object

I’m not sure what the :object bit is doing as the class itself is already an object? also the string and array operators are obviouse but what do they actualy do?? I tried googleing but i dont know what they are called.

EDIT: I found it, PHP: Type declarations - Manual

So it basicly just makes the data type strict? so it cant be changed… like saying “final function”? Now we can say “array $array” and make sure nothing but arrays come through? and so on…

Also “: string” would mean the function return should be a string?

Thank you :slight_smile:

#10

The return value is what comes after : so it would be return type of object

: object

#11

Course but let’s say I had a method that was returning a string, I could make the return of that method strict by saying : string { ?

Same for int, array etc…

Thank you

#12

That’s right. PHP will then throw an error if your function then returns something that isn’t a string. This makes it more obvious what’s happening for other users of your code.

1 Like
#13

Awesome I can see how this could be over used :stuck_out_tongue:

#14

I can’t think of a place where it would be a bad idea. In the very early stages of figuring out a process then maybe a function won’t have a known return type; but after that, everyone benefits from that extra information.

#15

I have a question still related to dpo methods… How can I create one function or even just small script to collect any and or all error exceptions from my PDO methods coming from several classes…

// Handle Exceptions however you want

function my_exception_handler($e) {

    echo 'Error';

    error_log($e->getMessage());

}

$id = 1;

$db = new DB((new Connection())->connect(require 'dbConfig.php'));

$user = $db->run('SELECT username FROM users WHERE user_id = ?', [$id])->fetch();

var_dump($user);

I do the same sort of thing here but I have methods for each job. Should I be try catching exceptions with alll of my db query methods??? As of now if I pass the wrong data type in/out of my methods the error is not handled… And this is the case for all of my methods as I dont know how to catch the error from outside the classes…

Where I call a method, could I try catch that rather then doing it inside of the method?

so

try {
     $obj->method($data);
} catch (PDOException $e) {
     error_log($e->getMessage());
}

Or should I wrapping the actual code in the try??

 $id = 1;
try {
 $user = $db->run('SELECT username FROM users WHERE user_id = ?', [$id])->fetch();
} catch (PDOException $e) {
 error_log($e->getMessage());
}

The only thing I try is the connection so far to protect the db config…

#16

It depends.

Personally, I only catch exceptions if I can do something with the error - if it’s worth retrying the operation for example. Everything else is left to a global exception handler that logs the error and stops the request.

Another approach is to always catch exceptions, and translate them into an exception that is appropriate to your code.

There are likely other options; the approach you choose will evolve as you get a better idea of how you work.

#17

Thanks :slight_smile:

I could try the method and if it fails log the exception and then redirect with a one time message like “Database Error” ?? This would be better then white screen with error at the top?

#18

No. Visitors to your site don’t need to know anything about most database errors, since they are due to programming mistakes or things like the database server not running. Only for things they can correct, such as inserting/updating duplicate or out of range values. If you tell a hacker that something they did caused a database error, it will just encourage them to do more of the same. All you should do for non-recoverable database errors is let the visitor know that the page they requested isn’t working, via a http 500 error.

Therefore, you should only catch and handle database errors in your code that the visitor to your site can recover from. The catch logic would test the sql error number and if it is for something that the code is designed to handle, set up and output a useful error message for the visitor telling them what exactly was wrong with the data that they submitted. For all other error numbers, just re-throw the PDO exception and let php handle it.

When you let php catch and handle an exception, it will use its error related settings to control what happens with the actual error information. This will cause ALL the error information - the error message, error number, file name, and line number where the error occurred to either get displayed or logged the same as php errors.

#19

Thanks for your reply! All forms on my site go through checking etc before they even reach the sql methods, I just wanted to be sure where and when I should be using the Exceptions… I have just made a little log function so I can log the details and then just redirect with message IE http 500 message…

here is my log function:

// Manage system Log

function siteLog($message, $option = null) {

    $dir = "_site_log/_sysLog.log";

    if ($option == 1) {

        $dir = "_site_log/_sysErrors.log";

    }

    error_log(SITE_DATE_TIME.": ".$message."\n", 3, INCLUDES_BASEDIR.$dir);

}

Is this a good way to go about it?

Sponsor our Newsletter | Privacy Policy | Terms of Service