0

The is the current code I have:

$resultSet = $link->query("SELECT kudos.sale_id as TheID, kudos.ident_id AS TheIdent from kudos,sales_gdpr where kudos.sale_id = $id AND sales_gdpr.id = kudos.sale_id");
                                    
if($stmt = $link -> prepare("SELECT COUNT(*) FROM kudos WHERE sale_id=? AND ident_id=?")) 
{
    $stmt -> bind_param("is", $id, $myIdent);
    $stmt -> execute();
    $stmt -> bind_result($count);
    $stmt -> fetch();
    $stmt -> close();
}

if ($count == 0) { // Not liked
    echo "<a style='color:#FFFFFF' class='btn'>  $resultSet->num_rows </a>";
} else { // Has liked
    echo "<b style='color:#FFFFFF' class='btn'>  $resultSet->num_rows </b>";
}

The SELECT COUNT I have made as a prepared statement, but I can't figure out how to make the $resultSet as a prepared statement. The code works all fine as it is now, but because of the kudos.sale_id = $id part, I guess it it vulnerably to SQL Injection. Can someone help me out please?

Slava Rozhnev
  • 5,534
  • 3
  • 14
  • 27
  • Are you asking how to write a join clause with mysql? ...I am pretty sure has HUNDREDS of examples already. You should avoid using the old-skool comma-joins because they are not easily read by other developers. We need to see somple sample schemas and the expected result. – mickmackusa Sep 20 '20 at 08:53
  • Hi, @mickmackusa. No, he actually thought that, in order to avoid sql injections, his sql statement (`SELECT kudos.sale_id...`) could be somehow prepared before calling the `mysqli::query` method, but he didn't know how to do it. At least that was my assumption when I wrote the answer based on the content of his SO question. – dakis Sep 20 '20 at 16:53
  • So are we talking about this? https://stackoverflow.com/a/51259779/2943403 – mickmackusa Sep 20 '20 at 21:08

1 Answers1

1

A code in which external values are directly passed into an SQL statement (like SELECT ... FROM ... WHERE id = $id) is open to SQL injections. In order to avoid them, an SQL statement receiving external values should always be prepared for execution. Terefore, the use of mysqli::query should be avoided (since it doesn't allow any preparation).

Here are two examples on "how to make the $resultSet as a prepared statement", depicting the two appliable methods for preparing an SQL statement in MySQLi, conditioned by the presence of mysqlnd ("MySQL Native Driver").

Method 1 - If mysqlnd driver is installed:

This method uses mysqli_stmt::get_result and mysqli_result::fetch_all.

In order to install the mysqlnd driver, the entry "extension=php_mysqli_mysqlnd.dll" (or similar) must be uncommented in the PHP config file ("php.ini") and the web server must be restarted, along with the mysql service.

<?php
require 'connection.php';

/*
 * Save the values, with which the database data will be filtered, into variables.
 * These values will replace the parameter markers in the sql statement.
 * They can come, for example, from a POST request of a submitted form.
 */
$saleId = 1;

/*
 * The SQL statement to be prepared. Notice the so-called markers,
 * e.g. the "?" signs. They will be replaced later with the
 * corresponding values when using mysqli_stmt::bind_param.
 *
 * @link http://php.net/manual/en/mysqli.prepare.php
 */
$sql = 'SELECT 
            k.sale_id AS saleId,
            k.ident_id AS identId 
        FROM 
            kudos AS k,
            sales_gdpr AS s 
        WHERE 
            k.sale_id = ? AND 
            s.id = k.sale_id';

/*
 * Prepare the SQL statement for execution - ONLY ONCE.
 *
 * @link http://php.net/manual/en/mysqli.prepare.php
 */
$statement = $connection->prepare($sql);

/*
 * Bind variables for the parameter markers (?) in the
 * SQL statement that was passed to prepare(). The first
 * argument of bind_param() is a string that contains one
 * or more characters which specify the types for the
 * corresponding bind variables.
 *
 * @link http://php.net/manual/en/mysqli-stmt.bind-param.php
 */
$statement->bind_param('i', $saleId);

/*
 * Execute the prepared SQL statement.
 * When executed any parameter markers which exist will
 * automatically be replaced with the appropriate data.
 *
 * @link http://php.net/manual/en/mysqli-stmt.execute.php
 */
$statement->execute();

/*
 * Get the result set from the prepared statement.
 *
 * NOTA BENE:
 * Available only with mysqlnd ("MySQL Native Driver")! If this
 * is not installed, then uncomment "extension=php_mysqli_mysqlnd.dll" in
 * PHP config file (php.ini) and restart web server (I assume Apache) and
 * mysql service. Or use the following functions instead:
 * mysqli_stmt::store_result + mysqli_stmt::bind_result + mysqli_stmt::fetch.
 *
 * @link http://php.net/manual/en/mysqli-stmt.get-result.php
 * @link https://stackoverflow.com/questions/8321096/call-to-undefined-method-mysqli-stmtget-result
 */
$result = $statement->get_result();

/*
 * Fetch all data at once and save it into an array.
 *
 * @link http://php.net/manual/en/mysqli-result.fetch-all.php
 */
$fetchedData = $result->fetch_all(MYSQLI_ASSOC);

/*
 * ...or fetch and save one row at a time.
 *
 * @link https://secure.php.net/manual/en/mysqli-result.fetch-array.php
 */
// while ($row = $result->fetch_array(MYSQLI_ASSOC)) {
//     $fetchedData[] = $row;
// }

/*
 * Free the memory associated with the result. You should
 * always free your result when it is not needed anymore.
 *
 * @link http://php.net/manual/en/mysqli-result.free.php
 */
$result->close();

/*
 * Close the prepared statement. It also deallocates the statement handle.
 * If the statement has pending or unread results, it cancels them
 * so that the next query can be executed.
 *
 * @link http://php.net/manual/en/mysqli-stmt.close.php
 */
$statement->close();

/*
 * Close the previously opened database connection.
 *
 * @link http://php.net/manual/en/mysqli.close.php
 */
$connection->close();
?>

<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
        <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=yes" />
        <meta charset="UTF-8" />
        <!-- The above 3 meta tags must come first in the head -->

        <title>Demo</title>

        <style type="text/css">
            body { padding: 10px; font-family: "Verdana", Arial, sans-serif; }
            .result-set { border-collapse: separate; border: 1px solid #ccc; }
            .result-set thead th { padding: 10px; background-color: #f3f3f3; }
            .result-set tbody td { padding: 5px; }
        </style>
    </head>
    <body>

        <h3>
            Result set
        </h3>

        <table class="result-set">
            <thead>
                <tr>
                    <th>Sale ID</th>
                    <th>Ident ID</th>
                </tr>
            </thead>
            <tbody>
                <?php
                if ($fetchedData) {
                    foreach ($fetchedData as $item) {
                        $saleId = $item['saleId'];
                        $identId = $item['identId'];
                        ?>
                        <tr class="result-set-record">
                            <td><?php echo $saleId; ?></td>
                            <td><?php echo $identId; ?></td>
                        </tr>
                        <?php
                    }
                } else {
                    ?>
                    <tr>
                        <td colspan="2">
                            No records found
                        </td>
                    </tr>
                    <?php
                }
                ?>
            </tbody>
        </table>

    </body>
</html>

Method 2 - If mysqlnd driver is not/can not be installed:

This method uses mysqli_stmt::store_result, mysqli_stmt::bind_result and mysqli_stmt::fetch.

<?php
require 'connection.php';

$saleId = 1;

$sql = 'SELECT 
            k.sale_id AS saleId,
            k.ident_id AS identId 
        FROM 
            kudos AS k,
            sales_gdpr AS s 
        WHERE 
            k.sale_id = ? AND 
            s.id = k.sale_id';

$statement = $connection->prepare($sql);

$statement->bind_param('i', $saleId);

$statement->execute();

/*
 * Transfer the result set resulted from executing the prepared statement.
 * E.g. store, e.g. buffer the result set into the (same) prepared statement.
 *
 * @link http://php.net/manual/en/mysqli-stmt.store-result.php
 * @link https://stackoverflow.com/questions/8321096/call-to-undefined-method-mysqli-stmtget-result
 */
$result = $statement->store_result();

/*
 * Bind the result set columns to corresponding variables.
 * E.g. these variables will hold the column values after fetching.
 *
 * @link http://php.net/manual/en/mysqli-stmt.bind-result.php
 */
$statement->bind_result($boundSaleId, $boundIdentId);

/*
 * Fetch results from the result set (of the prepared statement) into the bound variables.
 *
 * @link http://php.net/manual/en/mysqli-stmt.fetch.php
 */
$fetchedData = [];
while ($statement->fetch()) {
    $fetchedData[] = [
        'saleId' => $boundSaleId,
        'identId' => $boundIdentId,
    ];
}

/*
 * Free the stored result memory associated with the statement,
 * which was allocated by mysqli_stmt::store_result.
 *
 * @link http://php.net/manual/en/mysqli-result.free.php
 */
$statement->free_result();

$statement->close();

$connection->close();
?>

<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
        <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=yes" />
        <meta charset="UTF-8" />
        <!-- The above 3 meta tags must come first in the head -->

        <title>Demo</title>

        <style type="text/css">
            body { padding: 10px; font-family: "Verdana", Arial, sans-serif; }
            .result-set { border-collapse: separate; border: 1px solid #ccc; }
            .result-set thead th { padding: 10px; background-color: #f3f3f3; }
            .result-set tbody td { padding: 5px; }
        </style>
    </head>
    <body>

        <h3>
            Result set
        </h3>

        <table class="result-set">
            <thead>
                <tr>
                    <th>Sale ID</th>
                    <th>Ident ID</th>
                </tr>
            </thead>
            <tbody>
                <?php
                if ($fetchedData) {
                    foreach ($fetchedData as $item) {
                        $saleId = $item['saleId'];
                        $identId = $item['identId'];
                        ?>
                        <tr class="result-set-record">
                            <td><?php echo $saleId; ?></td>
                            <td><?php echo $identId; ?></td>
                        </tr>
                        <?php
                    }
                } else {
                    ?>
                    <tr>
                        <td colspan="2">
                            No records found
                        </td>
                    </tr>
                    <?php
                }
                ?>
            </tbody>
        </table>

    </body>
</html>

connection.php (included in both examples above):

For details, this article shows how to report errors in mysqli.

<?php

/*
 * This page contains the code for creating a mysqli connection instance.
 */

// Db configs.
define('HOST', 'localhost');
define('PORT', 3306);
define('DATABASE', 'tests');
define('USERNAME', 'anyusername');
define('PASSWORD', 'anypassword');

/*
 * Enable internal report functions. This enables the exception handling,
 * e.g. mysqli will not throw PHP warnings anymore, but mysqli exceptions
 * (mysqli_sql_exception).
 *
 * MYSQLI_REPORT_ERROR: Report errors from mysqli function calls.
 * MYSQLI_REPORT_STRICT: Throw a mysqli_sql_exception for errors instead of warnings.
 *
 * @link http://php.net/manual/en/class.mysqli-driver.php
 * @link http://php.net/manual/en/mysqli-driver.report-mode.php
 * @link http://php.net/manual/en/mysqli.constants.php
 */
$mysqliDriver = new mysqli_driver();
$mysqliDriver->report_mode = (MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

/*
 * Create a new db connection.
 *
 * @see http://php.net/manual/en/mysqli.construct.php
 */
$connection = new mysqli(HOST, USERNAME, PASSWORD, DATABASE, PORT);

Notes / Suggestions:

I didn't test the codes, but they should work.

I took the liberty to use my naming/coding conventions.

I strongly suggest you to switch from MySQLi to PDO. Here is a very good PDO tutorial.

You tried to apply some error handling by using if($stmt = $link -> prepare(...)){...}. Though, in order to be able to properly handle any kind of errors that may be raised by your present and future codes, I suggest you to read this tutorial thoroughly, along with all articles related to MySQLi found on that website.

And, at last, avoid printing (like with echo, for ex.) HTML code from PHP code, as much as possible. Try to separate PHP from HTML in a clean way. So, instead of something like this:

if ($count == 0) {
    echo "<a style='color:#FFFFFF' class='btn'>  $resultSet->num_rows </a>";
} else {
    echo "<b style='color:#FFFFFF' class='btn'>  $resultSet->num_rows </b>";
}

you could do this:

<?php
    //...

    $count = ...;
    $numRows = ...;
?>

<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
        <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=yes" />
        <meta charset="UTF-8" />
        <!-- The above 3 meta tags must come first in the head -->

        <title>Demo</title>

        <style type="text/css">
            body { padding: 10px; font-family: "Verdana", Arial, sans-serif; }
            .num-rows-link { color: #fff; background-color: #f3f3f3; border: 1px solid #ccc; }
            .num-rows-span { color: #fff; background-color: #f3f3f3; }
        </style>
    </head>
    <body>

        <?php
            if ($count == 0) {
        ?>
            <a class="btn num-rows-link"> <?php echo $numRows; ?></a>
        <?php
            } else {
        ?>
            <span class="btn num-rows-span"> <?php echo $numRows; ?></span>
        <?php
            }
        ?>

    </body>
</html>
dakis
  • 225
  • 1
  • 6
  • 32
  • Using the method 2, how can I get result-number of rows? I tried `echo $fetchedData->num_rows` but got error: `Notice: Trying to get property 'num_rows' of non-object in / ...` – Benedict Sep 20 '20 at 11:40
  • @Benedict Good question. I checked the code and I changed the line `$statement->store_result();` to `$result = $statement->store_result();` (in method 2). You can now apply `$numRows = $result->num_rows;` in method 2 as well, since `mysqli::store_result` returns a `mysqli_result` object - just like `mysqli_stmt::get_result` (from method 1). But you should thoroughly read these two docs when trying to get the number of rows: [mysqli::store_result](https://www.php.net/manual/en/mysqli.store-result.php) and [mysqli_result::$num_rows](https://www.php.net/manual/en/mysqli-result.num-rows.php) – dakis Sep 20 '20 at 12:36
  • Hm, weird. When I do `echo $result->num_rows;` it still gives me the same error: `Trying to get property 'num_rows' of non-object in / `. What am I doing wrong? I am using the same code as in method 2. – Benedict Sep 20 '20 at 13:44
  • @Benedict Before printing or assigning `$result->num_rows` to some variable you should check if the `$result` object is created at all, first. So, for testing purposes, right after `$result = $statement->store_result();` either call `var_dump($result); exit();`, or apply `echo '
    ' . print_r($result, true) . '
    '; exit();` to display the `$result` object on screen.
    – dakis Sep 20 '20 at 16:35
  • @Benedict It should look similar to this: `mysqli_result Object ( [current_field] => 0 [field_count] => 2 [lengths] => [num_rows] => 1 [type] => 0)`. Then you'll know where to start looking further. Again, it is important to read the two official documentation pages in-depth, e.g. [mysqli::store_result](https://www.php.net/manual/en/mysqli.store-result.php) and [mysqli_result::$num_rows](https://www.php.net/manual/en/mysqli-result.num-rows.php). – dakis Sep 20 '20 at 16:41