SQL query transition from mysql to procedural style error: Commands out of sync; you can't run this command now

Question

currently I edit my code from mysqli style to mysqli_stmt in order to be able to have better protection against SQL injections. I rewrote my code but when I want to inspect how many rows a certain query has, I receive this error: Commands out of sync; you can't run this command now, so my table will not show any data, but in reality it has.

I have read about this error for hours, but I do not know how to tweak it in order to be able to make it work.

Moreover, I am aware that in this case I do not have variables, but I tend to use the same style in all of my code.

Here is the code I use currently (which is not working):

$stmt = mysqli_prepare($conn,"SELECT * FROM `assets`");
    mysqli_stmt_execute($stmt);
    mysqli_stmt_store_result($stmt);

    if (!($result = mysqli_stmt_get_result($stmt))){ 
        die("Could not show the required data");}
    elseif (empty(mysqli_stmt_num_rows($result))){
                echo "Currently there are no assets to be shown.";}
    elseif (mysqli_stmt_num_rows($result) > 0){
        echo '<table id="table" class="display"><thead>
        <tr>
        <th>Asset name</th>
        <th>Classification</th> 
        <th>Tag</th>
        <th>Department</th> 
        <th>Next review date</th>   
        <th>Owner</th>  
        <th>Update</th>
        </tr></thead><tbody>';

        while($result2=mysqli_fetch_array($result, MYSQLI_NUM))
        {   $class="";
            if ($result2["Review_date"] < date('Y-m-d')){$class=" old";} elseif($result2["Review_date"] < date('Y-m-d', strtotime('+6 days'))){$class="notnew";} else {$class="new";}
                    echo '<tr class="'.$class.'">
                        <td>'.$result2["Asset_name"].'</td>
                        <td>'.$result2["Classification"].'</td>
                        <td>'.$result2["Tag"].'</td>
                        <td>'.$result2["Department"].'</td>
                        <td>'.$result2["Review_date"].'</td>
                        <td>'.$result2["Responsible"].'</td>
                        <td><a href="editassets.php?id='.$result2['id'].'">Edit</a> | <a href="deleteassets.php?id='.$result2['id'].'" onclick="return confirm(\'Do you want to delete this asset?\');">Delete</a></td>
                        </tr>';
        }
        echo '</tbody></table>';
    }

I know that my problem is with the first elseif statement, I have tried to use mysqli_stmt_free_result($stmt) but in this case I don't know whether it would be the solution, and if yes, where and how to put it.

Thank you for your help!

Try to remove `mysqli_stmt_store_result($stmt);` from your code. You shouldn't mix it with `mysqli_stmt_get_result()`. — Paul Spiegel, Jul 20 '19 at 14:35
mysqli_stmt_num_rows() expects parameter 1 to be mysqli_stmt, object given --> I received this error if that row is not included — Szabolcs Magyar, Jul 20 '19 at 14:44
Try `mysqli_stmt_num_rows($stmt)` or `mysqli_num_rows($result)` - BTW please read the [docs](https://www.php.net/manual/de/book.mysqli.php) for functions you are using. — Paul Spiegel, Jul 20 '19 at 14:50

Paul Spiegel · Accepted Answer · 2019-07-20T18:16:18.900

Here are my suggestions:

Don't use mysqli_stmt_store_result() ever, unless you know exactly what it's good for. Use mysqli_stmt_get_result() instead. Anyway - You should not mix those two functions.
Don't use mysqli_stmt_num_rows() on a result. Use mysqli_num_rows($result) instead.
Switch to object methods instead of global function calls. E.g.: $result->num_rows instead of mysqli_num_rows($result). The code will be shorter and you will not be able to use functions with wrong parameter types like mysqli_stmt_num_rows($result).
Use MYSQLI_ASSOC instead of MYSQLI_NUM. Or Use mysqli_fetch_assoc().

Your code should then look something like:

$stmt = $conn->prepare("SELECT * FROM `assets`");
$stmt->execute();
$result = $stmt->get_result()

if (!$result) { 
    die("Could not show the required data");
} elseif ($result->num_rows == 0) {
    echo "Currently there are no assets to be shown.";
} else {
    echo '<table id="table" class="display"><thead> [..]';

    while($result2 = $result->fetch_assoc()) {
        $class="";
        if ($result2["Review_date"] < date('Y-m-d')){
            $class=" old";
        } elseif($result2["Review_date"] < date('Y-m-d', strtotime('+6 days'))){
            $class="notnew";
        } else {
            $class="new";
        }
        echo '<tr class="'.$class.'">
            <td>'.$result2["Asset_name"].'</td>
            [..]
            </tr>';
    }
    echo '</tbody></table>';
}

Thank you very much, it worked! The only change I needed to make is I needed to delete the () in the end of $result->num_rows, but thanks. By using this solution, is my page safe from SQL injection? — Szabolcs Magyar, Jul 20 '19 at 17:45
You are right `num_rows` is an attribute - not a method. This code is save from SQL-injections. But your query is "constant" and has no parameters. You could even just use `$conn->query("...")` without `prepare()` and it would be safe though. The interesting part begins, when you have to include variables in the query. `SELECT * FROM users WHERE email = {$email}` is not safe even with prepare. You should use `$stmt = $conn->prepare("SELECT * FROM users WHERE email = ?")` - then bind the parameter and execute. You are safe as long as you don't use variables in the query string. — Paul Spiegel, Jul 20 '19 at 18:25
But better just read: [how-can-i-prevent-sql-injection-in-php](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Paul Spiegel, Jul 20 '19 at 18:25

SQL query transition from mysql to procedural style error: Commands out of sync; you can't run this command now

1 Answers1