Removing old DB function and parameterising my SQL statements

Question

I have inherited some PHP code that is not ideal and I'm not sure what the most efficient way of rectifying it is.

Basically all DB calls are made through a custom function like this:

function dbcommand($req)
{
        global $mysqli;

        $backtrace = debug_backtrace();

        $ret = array();
        $res = mysqli_query($mysqli, $req) or die('SQL Query Error: '. $mysqli->error .', query ['. $req .'] at '. $backtrace[0]['file'] .':'. $backtrace[0]['line']);

        if (strpos(strtoupper($req), 'SELECT') === 0)
        {
                if (mysqli_num_rows($res))
                {
                        while ($row = mysqli_fetch_assoc($res))
                                $ret[] = $row;
                }
                else $ret = array();

                mysqli_free_result($res);

                return $ret;
        }

        if (strpos($req, 'INSERT INTO') === 0)
            return $mysqli->insert_id;

        return $res;
}

Now I don't think I can use mysqli_real_escape_string because of the db-connector issue. Everything goes through that function. This means that avoiding sql injection is left in the hands of filter_vars before variables are mulched into SQL statements. I would like to parameterise my SQL statements and do it properly. But I'm just not sure what the most efficient way of doing it is in this case. Removing this function and converting everything to PDO would be very time consuming. There's a lot of code.

Also, calling debug_backtrace() every time, even thought you only need it in the rare failure case, is likely to be expensive. — Jonathon Reinhart
– Jonathon Reinhart, Commented Oct 22, 2015 at 1:09
Why can't you convert this function to use pdo? You said you don't want to remove it and use pdo so why not just use pdo in the function and add a second parameter which is an input for an array of values for use in binding — Rasclatt
– Rasclatt, Commented Oct 22, 2015 at 3:14

Rasclatt · Accepted Answer · 2015-10-22 06:16:54Z

Here is a PDO-converted version of your function. It requires some helper classes:

// This class is your connection class using a singleton (like a global)
// You would need to populate your credentials in the connect method
class   DatabaseConfig
    {
        private static  $singleton;

        public  function __construct()
            {
                if(empty(self::$singleton))
                    self::$singleton    =   $this->connect();

                return self::$singleton;
            }

        public  function connect($host = "localhost", $username = "username", $password = "password", $database = "database")
            {
                // Create connection
                $opts   =   array(  PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                                    PDO::ATTR_EMULATE_PREPARES => false,
                                    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC);
                $conn   =   new PDO('mysql:host='.$host.';dbname='.$database, $username, $password,$opts);

                return $conn;
            }
    }
// This is a query class that will run your sqls
class   QueryEngine
    {
        private $results;

        private static  $singleton;

        public  function __construct()
            {
                if(empty(self::$singleton))
                    self::$singleton    =   $this;

                return self::$singleton;
            }

        public  function query($sql = false,$bind = false)
            {
                $this->results  =   0;

                $db     =   new DatabaseConfig();
                try {
                        if(!empty($bind)) {
                                $query  =   $db ->connect()
                                                ->prepare($sql);
                                $query->execute($bind);
                            }
                        else {
                                $query  =   $db ->connect()
                                                ->query($sql);
                            }

                        $this->results  =   $query;
                    }
                catch (PDOException $e)
                    {
                        die($e->getMessage());
                    }

                return $this;
            }

        public  function fetch()
            {
                while($row = $this->results->fetch())
                    $result[]   =   $row;

                return (!empty($result))? $result : 0;
            }
    }

// This is your function down to the basics
// Error handling will be in the query class under try
function dbcommand($req,$bind = false)
    {   
        // Create query instance
        $qEngine    =   new QueryEngine();
        // Run the query
        $qEngine->query($req,$bind);
        // If select, fetch array
        if(strpos(strtoupper($req), 'SELECT') === 0)
            return $qEngine->fetch();
        // The query already ran, so you can return whatever you want
        // For ease I am just returning true
        elseif(strpos($req, 'INSERT INTO') === 0)
            return  true;
    }

To use:

// Make sure it include the classes and function above
print_r(dbcommand("select * from `users` where `ID` = :0",array(":0"=>"1")));

This would give you something like (in my db obviously, the table and columns will be different for you):

Array
(
    [0] => Array
        (
            [ID] => 1
            [unique_id] => 20150203190700523616
            [username] => tester
            [password] => $2a$12$xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
            [first_name] => Ras
            [last_name] => Clatt
            [email] => [email protected]
            [usergroup] => 3
            [user_status] => on
            [reset_password] => $2y$10$xxxxxxxxxxxxxxxxxxx
            [timestamp] => 2015-09-25 08:35:09
        )

)

Collectives™ on Stack Overflow

Removing old DB function and parameterising my SQL statements

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related