No true result return in insert query

Question

This is the class I have created which I am using for the queries:

<?php
mysqli_report(MYSQLI_REPORT_INDEX | MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

class DBConnect {
    private $dbcon;
    private $paramquery;
    private $result;

    public function __construct() {
        try {
            $this->dbcon = mysqli_init();
            mysqli_real_connect($this->dbcon, '127.0.0.1', '', '', '', 3306, '', MYSQLI_CLIENT_COMPRESS);
            $this->paramquery = $this->dbcon->stmt_init();
        } catch (mysqli_sql_exception $e) {
            exit('Database Connection Failed');
        }
    }
    public function dbquery($querysql, $querydata) {
        try {
            mysqli_ping($this->dbcon);
            $this->paramquery->prepare($querysql);

            array_walk($querydata, function(&$escval){$escval = mysqli_real_escape_string($this->dbcon, $escval);});   //Problem
            call_user_func_array(array($this->paramquery, 'bind_param'), $querydata);   //Problem

            $this->paramquery->execute();
        } catch (mysqli_sql_exception $e) {
            exit('Database Query Failed');
        }

        $this->result = $this->paramquery->get_result();  // problem

        if ($this->result) {
            $drs = $this->result->fetch_array();
            $this->result->free_result();
            return $drs;
        }
    }
    public function __destruct() {
        if (($this->dbcon !== null) && ($this->paramquery !== null) && ($this->result !== null)) {
            $this->paramquery->close();
            $this->dbcon->close();
        }
        unset($this->result);
        unset($this->paramquery);
        unset($this->dbcon);
    }
}
?>

The index.php file code is this:

<?php

require_once('connection.php');

$DBX = new DBConnect();

$DBX->dbquery('INSERT INTO `xathx_key` (`license`, `client`, `server`, `uniquex`) VALUES (?, ?, ?, ?)', array('ssss', '1', '3', '5', '7'));

var_dump($DBX);
unset($DBX)
?>

I am trying to do an INSERT query in this instance. And I want to get a success result or flag when the query is executed successfully. But in the var_dump of the object I get some irrelevant data and if I use echo I get an error that the object cannot be converted to a string. I just want to get a 0 for query execution failure, corruption or problem and a 1 for completion, success, ok status. When am I going wrong in the code?

EDIT: Can you guys just tell me what are the things that are wrong with this simple script? The main goal of this script is to connect to mysql server and execute all possible queries as fast as possible, as securely as possible.

Full Project Source: https://github.com/FSMySQL/PHP-FSMySQL

Answer 1

The main goal of this script is to connect to mysql server and execute all possible queries as fast as possible, as securely as possible.

The goal is a good one but the implementation could benefit from many improvements.

Disclaimer: there will be a lot of links to my own site because I am helping people with PHP for 20+ years and got an obsession with writing articles about most common issues.

The concept of error reporting

First of all, you need to change the concept of error reporting. Your exit() approach would be a nightmare for a programmer, as error messages are a vital source of information when something goes wrong. A programmer should go at any lengths trying to get the error message in the full detail. In my article, PHP error reporting, I do explain how to make error reporting both programmer- and user-friendly. In short, you shouldn't catch errors on the spot, but have a single dedicated place to report errors and exceptions, and then it could be easily configured depends on the current server's role.

Although, as suggested in the other answer, you could use a global try-catch block in your index.php file to act as such a global error handler, I would prefer a dedicated error handler script, as explained in the article above. It will make your code better organized and make index.php less bloated.

Besides, your idea of having "a true result return in insert query" contradicts with your intention to use exceptions. When one is using exceptions, there is no point to verify the immediate function's result. In case of error it will just bubble up to the error handler or a catch block, so, it will never reach the condition. A quick example:

function test() {
    throw new Exception("Test");
    return false;
}
$result = test();
if ($result === false) {
    echo "false";
}

The code execution in this example will never reach the condition, therefore making your functions return false on error useless. Which, in turn, makes returning true on success superfluous. Just return a meaningful result but don't use it as flag: simply write your code without any conditions, as though everything is fine. Remember that you have your error handling code elsewhere that will be magically invoked in case of error.

Connection

As explained in my other article, How to connect properly using mysqli, there is a slight chance to reveal connection credentials in case of a connection error. To avoid even a possibility but keep the programmer informed we have to throw a brand new exception, however keeping the error information - so the stack trace will begin from the throw line, and thus contain no sensitive information.

Also, the connection code lacks an essential part - setting the correct charset. Although in MySQL 8 the correct charset is set by default, it's better to make it explicit.

Also, making a mysqli statement a class variable is a grave mistake that will lead to race condition errors. The only state that your class should keep is that related to the connection but not a single class variable should be used for a statement.

So let's rewrite your constructor based on the code from the article above:

public function __construct()
{
    mysqli_report(MYSQLI_REPORT_INDEX | MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
    try {
        $this->dbcon = mysqli_init();
        $this->dbcon->real_connect('127.0.0.1', '', '', '', 3306, '', MYSQLI_CLIENT_COMPRESS);
        $this->dbcon->set_charset('utf8mb4');
    } catch (\mysqli_sql_exception $e) {
        throw new \mysqli_sql_exception($e->getMessage(), $e->getCode());
    }
}

The dbquery function

The function is, frankly, weird. It's a strange mix between prepared statements and escaping. Let's rewrite it based on my mysqli helper function that actually utilizes mysqli prepared statements

public function dbquery($sql, $data = [], $types = "")
{
        $this->dbcon->ping(); // not sure if it's necessary 
        $stmt = $this->dbcon->prepare($sql);
        if ($data) {
            $types = $types ?: str_repeat("s", count($data));
            $stmt->bind_param($types, ...$data);
        }
        $stmt->execute();
        return $stmt->get_result();
}

Now this function fulfills your desire for secure SQL queries

So finally we can rewrite your index.php

<?php

require_once('connection.php');
$DBX = new DBConnect();

$sql = 'INSERT INTO `xathx_key` (`license`, `client`, `server`, `uniquex`) VALUES (?, ?, ?, ?)';
$DBX->dbquery($sql, ['1', '3', '5', '7']);

Just as you learned above, there is no need for a "flag when the query is executed successfully". Just act as though there is always a success. In case of error it will appear without any conditions (an on a live site will be handled properly if you include an error handler script in your index).