PHP PDO Helper Functions

Question

Just recently I've wanted to update the PHP scripts I use to fetch information from a database for an iOS application. I've got around to it today, and I had a few questions that'd I like a second opinion on before I use these scripts.

1. Will this method of operation work? As in, since the query and connection are in two different try/catch blocks, will the query work differently?
2. Is this safe against SQL injection?
3. Can this support multiple/no parameters?
4. Overall, is this an efficient way of querying a database?

databaseHelper.php

<?php

error_reporting(E_ALL);
ini_set('display errors', 1);

$connection = // redacted
$username = // redacted
$password = // redacted

function initiateConnection() {
    try {
        $dbh = new PDO($connection, $username, $password);
        $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        return $dbh;
    } catch(PDOException $e) {
        return null;
    }
}

function performQuery($connection, $queryString, $parameters) {
    try {
        $query = $connection->prepare($queryString);
        $query->execute($parameters);

        if($sth->rowCount()) {
            $row = $sth->fetchAll(PDO::FETCH_ASSOC);
            return $row;
        } else {
            return null;
        }
    } catch(PDOException $e) {
        return null;
    }
}

?>

This is how I would plan on utilizing these functions:

<?php

include '../php_include/database.php';

$connection = initiateConnection();
if($connection != null) {
    $response = preformQuery($connection, 'SELECT * FROM users WHERE username = ?', 'test') // the username will NOT be hardcoded, it will be found in the $POST variable, this is just for example purposes
    if($response != null) {
        // echo json_encode($response);
    } else {
        // print false   
    }
} else {
    // print error message 
}

?>

Answer 1

PHP allows you to do this:

if($connection = initiateConnection()) 
{
  $query = 'SELECT * FROM users WHERE username = ?';
  if ($response = preformQuery($connection,$query,'test')) 
  {  
    echo json_encode($response);
  } 
}

So you can get rid of the != NULL. If you don't like this, this is also allowed:

$connection = initiateConnection();
if($connection) {
  $query    = 'SELECT * FROM users WHERE username = ?';
  $response = preformQuery($connection,$query,'test');
  if ($response) echo json_encode($response); 
}

You're not really 'wrapping' PDO like tim suggests, for that you would need to put it inside a class, you can still access all PDO's functionality. But he's right that you cannot get a useful error message. You can use a helper function to do something with the exception. Something like this:

define('DEBUG_MODE_ON',TRUE);

function exceptionHandler($exception)
{
  if ($DEBUG_MODE_ON) 
  {
    $message = $exception->getMessage();
    $file    = $exception->getFile();
    $line    = $exception->getLine();
    echo "<br><br><b>ERROR</b>: $message in <b>$file</b> on line <b>$line</b><br>";
  }
}

Clearly debugging has to be switched off in production. You could then log the errors. The point is: You need the error information.

As to your questions:

1: Will this method of operation work? As in, since the query and connection are in two different try/catch blocks, will the query work differently?

No, it won't work differently. You're using PDO in a standard way.

2: Is this safe against SQL injection?

As long as you bind parameters that can be influenced by the users you should be safe. Just don't allow them to insert anything directly into the query. This doesn't mean you're totally safe though. You should always stringently check user input. The main questions you should always answer are: a: Is this user authorized to input this? b: Does this value correspond with what was expected? Any other input should always be rejected. SQL injection isn't the only way your database can be compromised. See a small list:

http://www.darkreading.com/risk/hackers-choice-top-six-database-attacks/d/d-id/1129481

3: Can this support multiple/no parameters?

It can, but not really in a reliable manner. You're using questions marks to bind the parameters. This means that you need to match the order of the parameters in the $parameters array with the question marks in $queryString. I'm not saying that this is difficult, but you will get it wrong sometimes. Debugging such an error IS difficult. Believe me, I've been there. I would use named parameters instead of question marks. Then the order of the parameters becomes irrelevant. See:

http://php.net/manual/en/pdostatement.execute.php

$sth = $dbh->prepare('SELECT name, colour, calories
    FROM fruit
    WHERE calories < :calories AND colour = :colour');
$sth->execute(array(':calories' => $calories, ':colour' => $colour));

Overall, is this an efficient way of querying a database?

I don't think this is a relevant question. PDO is doing the work for you.

Answer 2

Wrapping PDO: General Approach

Generally, wrapping PDO without actually adding any functionality is not recommended. See for example here, or pretty much any answer on basic PDO wrapper classes.

Because what happens is that you take a class with a pretty nice (and well known) interface, and then reduce the power of that interface (you can only perform FETCH_ASSOC, if you ever need a different fetch type, you would probably add a parameter/method to your script, if you follow my advice below, you will throw exceptions, etc; at some point, your script will be very close in use to PDO, just with different function names, and slightly limited functionality).

Wrapping PDO: Swallowing Exceptions

The biggest drawback of your solution is that you take perfectly fine exception and turn them into null returns. This has two downsides: It makes your calling code ugly (too many != null checks), and it makes it really hard to debug your code, as you just swallowed all the information, eg on why the database connection could not be established, or why the query returned nothing (is there nothing in the database, or is the query completely wrong? nobody knows now).

Wrapping PDO: Error Reporting

Another thing that's a real problem is that you are setting error reporting in this script. This is really something that needs to be controllable (for PDO as well as for PHP), because you want all the error reporting possible in development mode, but none of it in production. If you just set these values in various scripts, it will be really hard to control.

Your Questions

So I would definitely not recommend this approach, but if you want to go forward in this direction:

Will this method of operation work?

Yes.

Is this safe against SQL injection?

Yes. (assuming it is used correctly, which means no variable input in the querystring ever; and also assuming that this isn't an issue)

Can this support multiple/no parameters?

Multiple queries definitely, and I would assume that execute(emtpyArray) is valid, but haven't tested it.

Overall, is this an efficient way of querying a database?

Performance wise? Sure. Code wise? You don't actually save all that much typing. The PDO instantiation only happens once (and having a method for that seems fine to me), so lets look at the queries:

try {
    $query = $connection->prepare($queryString);
    $query->execute($parameters);
    if($sth->rowCount()) {
        // echo json_encode($sth->fetchAll(PDO::FETCH_ASSOC));
    } else {
        // print false   
    }
} catch(PDOException $e) {
    // print error message 
}

vs

if($connection != null) {
    $response = preformQuery($connection, 'SELECT * FROM users WHERE username = ?', 'test');
    if($response != null) {
        // echo json_encode($response);
    } else {
        // print false   
    }
} else {
    // print error message 
}

So with your approach, you basically saved one line.