PHP PDO / MySQL helper/wrapper functions

Question

Further to my previous post for a user login script I've been learning PDO in order to migrate from mysql_* functions. As before with mysql_*, I wanted to wrap the PDO code into wrapper functions, so this is what I've done here.

I've created two functions:

1. db_connect(), which creates a connection and will be called at the start of each script
2. db_single_prepared_query(), which will run a single, prepared query, supporting just the four CRUD operations. I'll create more functions for other uses as required.

Since I'm new to PDO I would like feedback on a) any errors in my general usage of PDO and b) the helper functions:

function db_connect() 
    {
    // Define connection as a static variable, to avoid connecting more than once
    static $dbh;

    // Try and connect to the database, if a connection has not been established yet
    if(!isset($dbh)) 
        {
        try 
            {
            $dsn = 'mysql:dbname=' . MYSQL_DBNAME . ';host=' . MYSQL_HOSTNAME . ';charset=utf8';
            $dbh = new PDO($dsn, MYSQL_USERNAME, MYSQL_PASSWORD);
            $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);//which tells PDO to disable emulated prepared statements and use real prepared statements. This makes sure the statement and the values aren't parsed by PHP before sending it to the MySQL server (giving a possible attacker no chance to inject malicious SQL).
            $dbh->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            } 
        catch(PDOException $e)
            {
            print "Cannot connect to database<P>";
            log_this("db_connect - " . $e->getMessage(), "error.log");
            exit();

            }
        }

    return $dbh;
    }

db_single_prepared_query() which calls db_connect(), which only connects once per script life:.

function db_single_prepared_query($query, $parameters=array())
    {
    // see what type of query this is, use this later to customise return values
    if (preg_match('/^ *(select|update|insert|delete)/i', $query, $matches)) 
        {
        $crud_op = strtoupper($matches[1]);
        }
    else
        {
        print "Database query failed<P>";
        log_this("db_single_prepared_query_exec() - could not determine CRUD op for " . $query . "\n"  , "error.log");
        exit();
        }


    // re-connect with static $dhb inside of db_connect(), thus allowing us to call this function [db_single_prepared_query()]
    // from within another function without having to worry about passing $dbh
    $dbh = db_connect();

    try 
        {

        $stmt = $dbh->prepare($query);

        for ($i=0;$i<sizeof($parameters);$i++) 
            {
            $stmt->bindValue($parameters[$i]['label'], $parameters[$i]['value'], $parameters[$i]['type']);
            }

        $stmt->execute();

        $ret = array();

        switch ($crud_op) 
            {
            case "SELECT":
                // rowCount not reliable for select http://php.net/manual/en/pdostatement.rowcount.php
                // but http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers says OK to use?? trying both for comparison
                $rows = $stmt->fetchAll(PDO::FETCH_ASSOC);
                $ret['ROWS'] = $rows;
                $ret['ROWCOUNT'] = sizeof($rows);
                $ret['ROWCOUNTvia_rowCount'] = $stmt->rowCount();
                break;
            case "INSERT":
                $ret['ROWCOUNT'] = $stmt->rowCount();
                $ret['LASTID'] = $dbh->lastInsertId();;
                break;
            case "UPDATE":
            case "DELETE":
                $ret['ROWCOUNT'] = $stmt->rowCount();
            }
        }
    catch(PDOException $e)
        {
        print "Database query failed<P>";
        log_this("db_single_prepared_query_exec() - " . $query . "\n" .  $e->getMessage() , "error.log");
        exit();
        }
    return $ret;
    }

log_this() is another wrapper which writes to a log file in a safe location.

Usage:

$query = " select username, ext_email from users where college_id=:cid and user_id<:maxuid";


$cid=4;
$maxuid=10000;

$parameters = array(
    array('label' => ':cid', 'value'=> $cid, 'type' => PDO::PARAM_INT),
    array('label' => ':maxuid', 'value'=> $maxuid, 'type' => PDO::PARAM_INT)
    );
$info = db_single_prepared_query($query, $parameters);

print "<pre>\n";
print_r($info);
print "</pre>\n";

Output of mysald log showing multiple queries only connect once:

150304 16:52:52       125 Connect     development@localhost on xxxxx
          125 Prepare     [1] select username, ext_email from users where college_id=? and user_id<?
          125 Execute     [1] select username, ext_email from users where college_id=4 and user_id<10000
          125 Prepare     [2] select username, ext_email from users where college_id=? and user_id<?
          125 Execute     [2] select username, ext_email from users where college_id=4 and user_id<10000
          125 Prepare     [3] select username, ext_email from users where college_id=? and user_id<?
          125 Execute     [3] select username, ext_email from users where college_id=4 and user_id<10000
          125 Prepare     [4] select username, ext_email from users where college_id=? and user_id<?
          125 Execute     [4] select username, ext_email from users where college_id=4 and user_id<10000
          125 Quit

Research links:

• Blogspot
• Wiki

And of course the PHP docs

Questions

1. In the bindValue() data_type such as PDO::PARAM_INT, if I supply that but bind a string to it like so:

   $cid="4we";
       $maxuid=10000;
   
       $parameters = array(
           array('label' => ':cid', 'value'=> $cid, 'type' => PDO::PARAM_INT),
           array('label' => ':maxuid', 'value'=> $maxuid, 'type' => PDO::PARAM_INT)
           );
       $info = db_single_prepared_query($query, $parameters);
   

   This is sent to the server and returns rows. Am I missing something? I would have thought the point of using prepared statements, and specifying data types to bind, is to reject things like passing a string in place of an int.

       150304 17:04:01     126 Connect     development@localhost on obrienApp
               126 Prepare     [1] select username, ext_email from users where college_id=? and user_id<?
               126 Execute     [1] select username, ext_email from users where college_id='4we' and user_id<10000
               126 Quit
   

2. Is $stmt->rowCount(); reliable for MySQL?

3. Do I really not need to worry about my incoming data? Is it really safe from injection?

Answer 1

Here are some of my observations. This is code review so the observations are about your code, but I will also answer your questions along the way.

1: Functions, not classes. Nothing wrong with that, just unexpected. Classes are better at compartiamentilizing names. They are the preferred choice for most programming tasks, but functions will do just fine in this case.

2: Wrapping means restricting yourself. But you restrict yourself unnecessarily. You can only ever have one database connection with these functions. This might seem fine now, but in the future you might want to connect to more than one database. Yes, you will. And defining the database access parameters as constants has the same problem.

3: In virtually all cases the third parameter of bindValue() is not needed. Given what you try to do, you could easily leave it out. This would simplify your code quite a lot. To change your example:

 $parameters = array(':cid'    => $cid, 
                     ':maxuid' => $maxuid);

 $info = db_single_prepared_query($query, $parameters);

and this will work just fine. I would also leave the : out of the parameter keys and add it later, but it can be argued it should be there.

4: I would not use rowCount() for SELECT, since it might not always give the correct result. You have count($rows), so why use it?

5: Errors will occur when accessing a database. For now it's fine to just through a wobbler and exit with an error message. This is not acceptable in a production environment. Errors should be handled properly, or at least your function should give the user the ability to do so.

6: All incoming data should be treated with the utmost suspicion. The fact that you bind might help to prevent SQL injection in these functions, but lots of other types of attacks exist. Hackers will try to change the input and see what happens. All input should therefore be filtered and clear restrictions on their values should always be imposed.