ICJ
ICJ

Reputation: 307

PDO Prepared Statement Function

I am using PDO prepared statements, while working on my current project i decided to just create several function for each action that i would need. Below is the example for fetch

function db_fetchAll($sql, $param) {
    global $db;

    $stmt = $db->prepare($sql);

    if (empty($param)) {
        $stmt->execute();
    } else {
        $stmt->execute($param);
    }

    $count = $stmt->rowCount();

    if ($count == 0) {
        $result = "";
    } elseif ($count == 1) {
        $result[] = $stmt->fetch();
    } elseif ($count > 1) {
        $result = $stmt->fetchAll();
    }

    return $result;
}

Example

$database = db_fetchAll("SELECT * FROM database_table WHERE id=:id", array(':id' => $id));

It only condenses lines query's from 3 lines to 1, but with the amount of information required for each page it was worth condensing.

I am going back through the project for one final pass through and i just wanted a second opinion on the security of this. If there is anything that i should add, etc. All user_input is passed through this function

function user_input($input) {
    $input = trim($input);  
    $output = strip_tags($input);

    return $output;
}

and all output uses htmlspecialchars.

So the question in a nutshell is: Is this safe? Is there anything else that i could do to prevent any other form of injection etc?

I completely understand how prepared statements work, i am just being thorough, version 1 of this site was a nightmare, tons of injections, gaining access to admin accounts, etc.

Upvotes: 6

Views: 1578

Answers (1)

Your Common Sense
Your Common Sense

Reputation: 157897

+1 for such an intention. A quite embarrassing number, but only one out of thousand users asking questions regarding PDO here ever think of such a natural thing like a helper function.

Just a few notes.

  • First of all, there is an essential flaw with your code which is trying to detect the result type automatically. I tried it myself and I can assure you it's just a source of disaster. The less magic in your code the more hair you keep on your head. So, instead of such unreliable magic, just make different functions to return different result sets.
  • Second, you have to make default value for the parameters? to be able to run query without placeholders.
  • Third, some code is just superfluous. There is no need to check $param variable.

Finally, your function should be looking like

function db_fetchAll($sql, $param = array()) {
    global $db;

    $stmt = $db->prepare($sql);
    $stmt->execute($param);
    return $stmt->fetchAll();
}

and always return the array of rows as the name suggests

As of the other functions with different result sets, it seems that PDO, if very slightly tweaked, can offer exactly the same convenience without any helper function at all. You may take a look at the PDO wrapper I made to ease the pain of transition from old mysql ext

Your code will remain almost the same

$data = DB::query("SELECT * FROM database_table")->fetchAll();

yet it will let you to use all the poser of PDO different resultset methods:

$row = DB::prepare("SELECT * FROM table WHERE id=?")->execute([$id])->fetch();

or even

$sql  = "SELECT name FROM table WHERE id=?";
$name = DB::prepare($sql)->execute([$id])->fetchColumn();

and so on

Upvotes: 3

Related Questions