PDO MySQL `prepared statements` to use or not to use placeholders

Question

I'm on a Web Development class. They teach us to connect to MySQL database with PDO then make some methods within a class to access the PDO connection.

db_model.php

<?php
class DB {
    protected $db;
    function __construct() {  
        $this->db = new PDO("mysql:host=localhost;dbname=blog", "root", "");
    }  

    function executeQuery($query) {
        $statement = $this->db->prepare($query);    
        $statement->execute();
        return $statement;
    }
}

articles_model.php

<?php
require_once "db_model.php";

class ArticlesModel extends DB {
    function getAll() {
        $statement = $this->executeQuery("SELECT * FROM articles");
        return $statement->fetchAll(PDO::FETCH_ASSOC);
    }

    function getArticle($id) {
        $statement = $this->executeQuery("SELECT * FROM articles WHERE id = " . $id);
        return $statement->fetchAll(PDO::FETCH_ASSOC);
    }

    function insertArticle($article) {
        $this->executeQuery("INSERT into articles (title, body, image) values ('".$article["title"]."', '".$article["body"]."', '".$article["file"]."');");
        return $this->db->lastInsertId();
    }

    function updateArticle($article) {
        $statement = $this->executeQuery("UPDATE articles SET title ='".$article["title"]."',body = '".$article["body"]."' WHERE id =".$article["id"]);
        return $statement->rowCount();    
    }

    function deleteArticle($article) {
        $statement = $this->executeQuery("DELETE FROM articles WHERE id =".$article["id"]);
        return $statement->rowCount(); 
    }
}

I'm far from being an advanced PHP programmer but as much as I know is that the good practice is to use real prepared statements with placeholders not just concatenated PHP variables in SQL statements, so I came with this:

db_model.php

<?php

define('DB_NAME', 'blog');
define('DB_HOST', 'localhost');
define('DB_USER', 'root');
define('DB_PASS', '');
define('DB_CHAR', 'utf8');

class DB {
        protected $db;
        function __construct() {
                $opt  = array(
                        PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
                        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
                        PDO::ATTR_EMULATE_PREPARES   => TRUE
                );
                $dsn = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHAR;
                $this->db = new PDO($dsn, DB_USER, DB_PASS, $opt);
        }

        function executeQuery($query) {
                $statement = $this->db->prepare($query);
                $statement->execute();
                return $statement;
        }
}

articles_model.php

<?php

require_once 'db_model.php';

class ArticlesModel extends DB {
        function getAll() {
                $statement = $this->executeQuery('SELECT * FROM articles');
                return $statement->fetchAll(PDO::FETCH_ASSOC);
        }

        function getArticle($id) {
                $statement = $this->executeQuery('SELECT * FROM articles WHERE id = :id');
                $statement->bindParam(':id', $id, PDO::PARAM_INT);
                return $statement->fetchAll(PDO::FETCH_ASSOC);
        }

        function insertArticle($article) {
                $statement = $this->executeQuery('INSERT into articles (title, body, image) values (:title, :body, :image)');
                $statement->bindParam(':title', $article['title'], PDO::PARAM_VAR);
                $statement->bindParam(':body', $article['body'], PDO::PARAM_VAR);
                $statement->bindParam(':image', $article['file'], PDO::PARAM_VAR);
                return $statement->lastInsertId();
        }

        function updateArticle($article) {
                $statement = $this->executeQuery('UPDATE articles SET title = :title, body = :body WHERE id = :id');
                $statement->bindParam(':title', $article['title'], PDO::PARAM_VAR);
                $statement->bindParam(':body', $article['body'], PDO::PARAM_VAR);
                $statement->bindParam(':id', $article['id'], PDO::PARAM_INT);
                return $statement->fetchColumn();
        }

        function deleteArticle($article) {
                $statement = $this->executeQuery('DELETE FROM articles WHERE id = :id');
                $statement->bindParam(':id', $article['id'], PDO::PARAM_INT);
                return $statement->fetchColumn();
        }
}

Which method is safer/more correct to use? Am I wrong? BTW, I could have used cleaner code but wanted to keep my code as close as I could to teacher's code. (e.g. not use bindparam() everywhere but execute(array()))

EDIT:

I think properly would be:

<?php

require_once 'db_model.php';

class ArticlesModel extends DB {
        function getAll() {
                $statement = $this->executeQuery('SELECT * FROM articles');
                return $statement->fetchAll(PDO::FETCH_ASSOC);
        }

        function getArticle($id) {
                $statement = $this->executeQuery('SELECT * FROM articles WHERE id = :id');
                $statement->bindParam(':id', $id, PDO::PARAM_INT);
                return $statement->fetchAll(PDO::FETCH_ASSOC);
        }

        function insertArticle($article) {
                $params = [
                        ':title' => $article['title'],
                        ':body' => $article['body'],
                        ':image' => $article['file']
                ];
                $statement = $this->executeQuery('INSERT into articles (title, body, image) values (:title, :body, :image)', $params);
                return $statement->lastInsertId();
        }

        function updateArticle($article) {
                $params = [
                        ':title' => $article['title'],
                        ':body' => $article['body'],
                        ':id' => $article['id']
                ];
                $statement = $this->executeQuery('UPDATE articles SET title = :title, body = :body WHERE id = :id', $params);
                return $statement->fetchColumn();
        }

        function deleteArticle($article) {
                $params = [':id' => $article['id']];
                $statement = $this->executeQuery('DELETE FROM articles WHERE id = :id', $params);
                return $statement->fetchColumn();
        }
}

In all the methods of `ArticlesModel` class you're first sending the query to `executeQuery()` method and then binding the parameters, which is wrong. Look closely, you're first preparing the query, then executing it right away before binding the parameters. The correct order is, **prepare**, **bind** and *then* **execute**. — Rajdeep Paul, May 15 '16 at 07:48
You're right, I will have to send the parameters as arguments and bind them properly, I forgot that and I didn't want to change a lot my teacher's methods. — bsteo, May 15 '16 at 07:57
The code above won't work with your teacher's flawed methods. — Your Common Sense, May 15 '16 at 08:40

Sgt AJ · Accepted Answer · 2016-05-16T02:53:14.907

While it is safer by far to use prepared statements, the real benefit from them comes when they are applied to user inputs in SQL statements, thus protecting your code from a SQL injection attack. When you are typing values into your SQL statements directly, these benefits are not realized, thus prepared statements are not strictly necessary. You may still benefit from being able to reuse queries, however, which is another benefit to prepared statements.

That being said, it is good programming practice to use a uniform method throughout, and using prepared statements at all times will help prevent 2nd order SQL injection attacks.

PDO MySQL `prepared statements` to use or not to use placeholders

1 Answers1