-2
\$\begingroup\$

Need some advices to create class that handles sql query operations with given array (having full control of the array and obtaining record information). My code works properly (it is just example), but need some check it is safe, or there's a better way to reach result. I want to note that all dangerous characters before adding to array are cleared with other function (to prevent injection). So basic idea is to create array with query structure, then pass to function and do sql query operation. Thanks, appreciate all answers.

** THIS IS FOR LEARNING PURPOSES ONLY **

Created class that handle operations:

class my_op
{
    private $pdo;
    private $safe_code = "@@@";

    public function __construct($pdo)
    {
        $this->pdo = $pdo;
    }


    private function safe_chars($first_string, $second_string)
    {
        $safe_chars = array(
            "A","B","C","D","E","F","G", "H","I","J","K","L","M","N","O","P","Q","R","S",
            "T","U","V","W","X","Y","Z","a","b","c","d","e","f","g","h","i","j","k",
            "l","m","n","o","p","q","r","s","t","u","v","w","x","y","z","1",
            "2","3","4","5","6","7","8","9","0",",","'","?","!"," ","_","-",":","(",
            ">","<",")"
        );
        $unsafe_count = null;
        if (strlen($first_string) > 0):
            foreach (str_split($first_string) as $char_check)
            {
                if (!in_array($char_check, $safe_chars))
                {
                    $unsafe_count++;
                }
            }
        endif;
        if (strlen($second_string) > 0):
            foreach (str_split($second_string) as $char_check)
            {
                if (!in_array($char_check, $safe_chars))
                {
                    $unsafe_count++;
                }
            }
        endif;
        return $unsafe_count;
    }

    private function build_query($my_array)
    {

        $instances = array(
            "UPDATE","DELETE FROM","INSERT INTO","WHERE","SET","FROM","VALUES","SELECT",
            "ORDER BY","LIMIT",'INTERSECT','MINUS','UNION','UNION ALL','OR','BETWEEN',
            'DISTINCT','GROUP BY','EXISTS','JOIN','LIKE','TRUNCATE','IN','TOP','MINUS',
            'IS NULL','IS NOT NULL','EXCEPT','AVG','COUNT','MAX','MIN','SUM'
        );

        $OOP = [10, 2, 4]; //max arrays,least,most
        $KEYS = ["SET-STRUCTURE", "SET-PARAMETERS", "SET-BIND", "SET-EXECUTE", "SET-FMODE"];
        if (is_array($my_array) && sizeof($my_array) > 0):
            $secure_purposes = implode('|', $instances);
            $filtered_array_my_op = filter_var_array($my_array, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH);
            $response = false;
            $fetch_response = false;
            try
            {
                if (sizeof($my_array) <= $OOP[0]):
                    $f_mode_type = null;
                    $gresponse_arr = null;
                    $exe = [null, null, null, null, "on", "on", "bindvalue"]; //execute,binding,bindingval
                    foreach (is_array($filtered_array_my_op) || is_object($filtered_array_my_op) ? $filtered_array_my_op : array() as $splited_array):
                        $parsed_values = array();
                        $opc = [null, null, null, null];

                        foreach ($splited_array as $parsed_keys_g => & $parsed_values_g):

                            foreach ($parsed_values_g as $direct_key => & $direct_value):

                                switch ($parsed_keys_g)
                                {

                                    case $KEYS[0]:
                                        if ($parsed_keys_g == $KEYS[0] && strlen($direct_value) > 0 && in_array($direct_key, $instances)):
                                            if ($this->safe_chars($direct_key, $direct_value) <= 0):
                                                $opc[2] .= " $direct_key $direct_value ";
                                                $opc[0]++;
                                            else:
                                                $exe[1]++;
                                            endif;
                                        elseif ($parsed_keys_g == $KEYS[0] && strlen($parsed_s_value) < 0 || $parsed_keys_g == $KEYS[0] && !in_array($parsed_s_key, $instances)):
                                            $exe[2]++;
                                        endif;
                                        break;

                                    case $KEYS[1]:
                                        if ($parsed_keys_g == $KEYS[1] && strlen($direct_value) > 0):
                                            if ($this->safe_chars($direct_key, $direct_value) <= 0):
                                                $parsed_values[$direct_key] = $direct_value;
                                                $opc[3] .= " $direct_key $direct_value";
                                                $opc[1]++;
                                            else:
                                                $exe[1]++;
                                            endif;
                                        elseif ($parsed_keys_g == $KEYS[1] && strlen($direct_value) < 0):
                                            $exe[2]++;
                                        endif;
                                        break;
                                    case $KEYS[2]:
                                        $exe[5] = $direct_value;
                                        $exe[6] = $direct_key;
                                        break;
                                    case $KEYS[3]:
                                        $exe[4] = $direct_value;
                                        break;
                                    case $KEYS[4]:
                                        $exe[3] = $direct_value;
                                        $f_mode_type = $direct_key;
                                        break;

                                    }
                                endforeach;
                            endforeach;
                            $exe[0]++;
                            if ($opc[0] >= $OOP[1] && $exe[4] == "on" && $opc[0] <= $OOP[2] && $opc[1] > 0 && preg_match("($secure_purposes)", $opc[2]) && array_key_exists($KEYS[0], $splited_array) && $exe[2] <= 0):
                                try
                                {
                                    $this
                                        ->pdo
                                        ->beginTransaction();
                                    $my_operation = $this
                                        ->pdo
                                        ->prepare($opc[2]);
                                    $bindcount = 0;
                                    switch ($exe[5])
                                    {
                                        case "on":
                                            foreach ($parsed_values as $value_key_parsed => $value_parsed)
                                            {
                                                if ($exe[6] == "bindvalue"):
                                                    $my_operation->bindValue($value_key_parsed, $value_parsed);
                                                elseif ($exe[6] == "bindparam"):
                                                    $my_operation->bindParam($value_key_parsed, $value_parsed);
                                                endif;
                                                $bindcount++;
                                            }
                                            break;
                                        case "off":
                                            //dosomething
                                            break;
                                        }

                                        switch ($exe[4])
                                        {
                                            case "on":
                                                if ($exe[5] == "on"):
                                                    if ($my_operation->execute()):
                                                        $status = "Success";
                                                    else:
                                                        $status = "Failed";
                                                    endif;
                                                else:
                                                    if ($my_operation->execute($parsed_values)):
                                                        $status = "Success";
                                                    else:
                                                        $status = "Failed";
                                                    endif;
                                                endif;
                                                break;
                                            case "off":
                                                //dosomething
                                                break;
                                            }
                                            if (strlen($f_mode_type) > 0):
                                                while ($gresponse_arr = $my_operation->$f_mode_type($exe[3]))
                                                {
                                                    $fetch_response[] = $gresponse_arr;
                                                }
                                                else:
                                                    throw new Exception("Fetch building failed.");
                                                endif;

                                                $my_operation = null;
                                                $this
                                                    ->pdo
                                                    ->commit();
                                                $this->pdo = null;
                                                $response[] = ["operation" => ['status' => $status, 'fmode' => $exe[3], 'op_made' => $exe[0], 'binded_param' => $bindcount, 'sizeof' => sizeof($my_array) , 'scount' => $opc[0], 'vcount' => $opc[1], ], "structure" => ['srecord' => $opc[2]], "setparams" => ['precord' => $opc[3]]];

                                            }
                                            catch(Exception $e)
                                            {
                                                $this
                                                    ->pdo
                                                    ->rollback();
                                                throw new Exception('Something wrong: ' . $e->getMessage());
                                            }
                                            else:
                                                if ($exe[4] == "off"):
                                                    throw new Exception("Execution is disabled.");
                                                endif;
                                                if ($exe[1] > 0):
                                                    throw new Exception("Unsafe chars cannot be tolerated ");
                                                endif;
                                                if ($exe[2] > 0):
                                                    throw new Exception("Some keys or values do not match requirements. //v $opc[1] //s $opc[0]");
                                                endif;
                                                if ($s <= $OOP[1]):
                                                    throw new Exception("Structure keys count failed (least). //v $opc[1] //s $opc[0]");
                                                endif;
                                                if ($s >= $OOP[2]):
                                                    throw new Exception("Structure keys count failed (most). //v $opc[1] //s $opc[0]");
                                                endif;
                                                if ($v <= 0 && preg_match("($secure_purposes)", $opc[2]) && array_key_exists($KEYS[1], $splited_array) && array_key_exists($KEYS[0], $splited_array)):
                                                    throw new Exception("Missing some structure.");
                                                endif;
                                            endif;
                                        endforeach;
                                    else:
                                        throw new Exception("Limited operations.");
                                    endif;
                                }
                                catch(Exception $e)
                                {
                                    throw new Exception('Something wrong: ' . $e->getMessage());
                                }
                                if (sizeof($response) > 0 && sizeof($fetch_response) > 0):
                                    $output = array_merge($response, $fetch_response);
                                elseif (sizeof($response) > 0):
                                    $output = $response;
                                else:
                                    $output = false;
                                endif;
                                return $output;
                            else:
                                return false;
                            endif;
                        }
                        public function testas()
                        {
                            $test = "testas";
                            return $test;
                        }
                        public function get_parsed($arr, $key)
                        {
                            $val = null;
                            foreach ($arr as $v2):
                                if (!empty($v2[$key]))
                                {
                                    $val = $v2[$key];
                                }
                                foreach ($v2 as $v):
                                    if (!empty($v[$key]))
                                    {
                                        $val = $v[$key];
                                    }
                                endforeach;
                            endforeach;

                            return $val;
                        }
                        public function r($op)
                        {
                            $r = $op;
                            foreach ($op as $u):
                                if (!empty($u))
                                {
                                    if (is_array($u))
                                    {
                                        foreach ($u as $u2):
                                            if (!empty($u2))
                                            {
                                                $r[] = $u2;
                                            }
                                        endforeach;
                                    }
                                }
                            endforeach;
                            return $r;
                        }
                        public function get_info($arr, $key)
                        {
                            $val = null;
                            $infokey = null;
                            if ($key == "status" || $key == "op_made" || $key == "binded_param" || $key == "sizeof" || $key == "scount" || $key == "vcount"):
                                $infokey = "operation";
                            elseif ($key == "srecord"):
                                $infokey = "structure";
                            elseif ($key == "precord"):
                                $infokey = "setparams";
                            endif;
                            foreach ($arr as $v):
                                if (!empty($v[$infokey][$key])):
                                    $val = $v[$infokey][$key];
                                endif;
                            endforeach;

                            return $val;
                        }
                        public function handle_query($my_array, $safe_code)
                        {
                            $private_safe_code = $this->safe_code;
                            try
                            {
                                if ($private_safe_code == $safe_code): // some user security check
                                    $get_private_func = $this->build_query($my_array);
                                    return $get_private_func;
                                else:
                                    throw new Exception("Something wrong.");
                                endif;
                            }
                            catch(Exception $e)
                            {
                                return $e->getMessage();
                            }
                        }
                    }

Setting array which generates query.

$operation_examples[] = ["SET-STRUCTURE" => [ // structure // this part safe from inputs
        'SELECT' => "user_id FROM dot_users WHERE user_status = :status LIMIT :limit"],
                            
                   "SET-PARAMETERS" => [ // setting parameters (PDO prepared st.)
                   ":status" => "1",
                   ":limit" => "1"],
                                            
                   "SET-BIND" => ["bindparam" => "on"], // bind options // ON by default bindvalue or bindparam
                   "SET-EXECUTE" => ["on"], //execution  on/off
                   "SET-FMODE" => [fetchAll => PDO::FETCH_ASSOC] //switch fetch mode
                   ];

Call class:

                   $cl = new my_op($pdo);
                   $op = $cl->handle_query($operation_examples, "@@@");

                   echo "Operation status: " . $cl->get_info($op, "status");
                   echo "<br>";
                   echo "Structure record: " . $cl->get_info($op, "srecord");
                   echo "<br>";
                   echo "Params record: " . $cl->get_info($op, "precord");
                   echo "<br>";
                   echo "Last id: " . $cl->get_parsed($op, "user_id");

                   foreach ($cl->r($op) as $u): 
                       if(!empty($u['user_id'])):
                       echo "<br>ID:" . $u['user_id'] . "<br>";
                       endif;
                   endforeach;

                   var_dump($op);

Output:

Operation status: Success
Structure record: SELECT user_id FROM dot_users WHERE user_status = :status LIMIT :limit
Params record: :status 1 :limit 1
Last id: 57
ID:57
array(2) { [0]=> array(3) { ["operation"]=> array(7) { ["status"]=> string(7) "Success" ["fmode"]=> string(1) "2" ["op_made"]=> int(1) ["binded_param"]=> int(2) ["sizeof"]=> int(1) ["scount"]=> int(1) ["vcount"]=> int(2) } ["structure"]=> array(1) { ["srecord"]=> string(72) " SELECT user_id FROM dot_users WHERE user_status = :status LIMIT :limit " } ["setparams"]=> array(1) { ["precord"]=> string(21) " :status 1 :limit 1 " } } [1]=> array(1) { [0]=> array(1) { ["user_id"]=> int(57) } } } 
\$\endgroup\$
7
  • 3
    \$\begingroup\$ Is your code normally indented or is this how you always write it. It is almost unreadable. \$\endgroup\$
    – pacmaninbw
    Mar 6, 2021 at 13:38
  • 5
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Mast
    Mar 7, 2021 at 16:51
  • 1
    \$\begingroup\$ Look like you are trying to write a thing called a Query builder. It's a fair question but in order to ask it, you need to provide two things: 1. What is the reason you think that you need one. What will be the use case? 2. How it's intended to be used with examples. And yes, the most important one: 3. Above all, please learn how to format your code properly. It's the biggest problem you've got at hand. \$\endgroup\$ Mar 8, 2021 at 8:18
  • 1
    \$\begingroup\$ Can you explain, how and why this elaborate code is any better than vanilla PDO? What's the goal? What's the benefit? What problem does it solve? \$\endgroup\$ Mar 8, 2021 at 10:47
  • 1
    \$\begingroup\$ From title and introduction of this question, I have no idea what the code is to accomplish. The code does not seem to contain embedded documentation. \$\endgroup\$
    – greybeard
    Mar 9, 2021 at 20:51

1 Answer 1

7
\$\begingroup\$

You have turned simple SQL prepared statements into this monstrosity that is extremely difficult to read and is open to SQL injection. Remove it! Don't go down this path. It is absolutely unnecessary.

Take a look at how simple prepared statements can be.

<?php

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$mysqli = new mysqli('localhost', 'user', 'password', 'test');
$mysqli->set_charset('utf8mb4'); // always set the charset

$index = 1;
$stmt = $mysqli->prepare('UPDATE my_orders SET status = "waiting" WHERE myindex = ?');
$stmt->bind_param('s', $index);
$stmt->execute();

You don't need that complicated class. All you need are prepared statements. If you want something simpler with more functionality then you can use PDO. I strongly advise using PDO whenever possible.

Same code using prepared statements:

<?php

$pdo = new \PDO("mysql:host=localhost;dbname=test;charset=utf8mb4", 'user', 'password', [
    \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
    \PDO::ATTR_EMULATE_PREPARES => false
]);

$index = 1;
$stmt = $mysqli->prepare('UPDATE my_orders SET status = "waiting" WHERE myindex = ?');
$stmt->execute([$index]);

On an unrelated note, please use proper IDE with static analysis tool and syntax highlighting and formatting. This code is unreadable at the moment.

\$\endgroup\$
10
  • \$\begingroup\$ What if i change mysqli_query to pdo prepared statements, but leave same only_update function just modify it with pdo syntax and parameters? \$\endgroup\$
    – Anarchy
    Mar 6, 2021 at 16:07
  • \$\begingroup\$ What's the point? MySQLi also has prepared statements. Don't create functions that will make programming more difficult. PDO is easier compared to MySQLi but if you keep your current code you won't see much improvement. \$\endgroup\$
    – Dharman
    Mar 6, 2021 at 17:06
  • \$\begingroup\$ My point is to do operations with only functions and arrays. I edit my code, and modify it with pdo, check it out and say what you think. \$\endgroup\$
    – Anarchy
    Mar 7, 2021 at 14:06
  • \$\begingroup\$ @ZujisKarolis It's worse than before. You are still not binding any parameters. The variable names mean absolutely nothing. The code is still not formatted. You have very strange filter_var_array($my_values,FILTER_SANITIZE_STRING,FILTER_FLAG_STRIP_HIGH);. \$\endgroup\$
    – Dharman
    Mar 7, 2021 at 14:10
  • 1
    \$\begingroup\$ @ZujisKarolis Just remove that whole class. Trust me, you really do not want to do it this way. Stick to plain old SQL. As for parameter binding, I have no idea what you are doing, but it is still vulnerable to SQL injection. Even if you manage to bind them correctly, the whole class will still be vulnerable to SQL injection. Don't do it. \$\endgroup\$
    – Dharman
    Mar 7, 2021 at 17:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.