php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78932 Cannot fetch mysqli_prepare error if $statement variable reused
Submitted: 2019-12-08 21:27 UTC Modified: 2020-12-09 14:09 UTC
From: craig at craigfrancis dot co dot uk Assigned: requinix (profile)
Status: Closed Package: MySQLi related
PHP Version: 7.3.12 OS:
Private report: No CVE-ID: None
 [2019-12-08 21:27 UTC] craig at craigfrancis dot co dot uk
Description:
------------
When running the following test script, the error is not correctly shown.

But if you un-commented the `$statement = false` line, then it works.

It's as though the $statement variable is not being properly replaced.

Test script:
---------------
<?php

	$link = mysqli_connect('localhost', 'username', 'password', 'database');

	$statement = mysqli_prepare($link, 'SELECT 1');

	// $statement = false;

	$statement = mysqli_prepare($link, 'SELECT 1 FROM this_table_does_not_exist');

	if (!$statement) {
		exit(mysqli_errno($link) . ': ' . mysqli_error($link));
	}

?>

Expected result:
----------------
1146: Table 'database.this_table_does_not_exist' doesn't exist

Actual result:
--------------
0: 

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-12-09 08:26 UTC] nikic@php.net
-Summary: mysqli_prepare oddity +Summary: Cannot fetch mysqli_prepare error if $statement variable reused -Status: Open +Status: Verified -PHP Version: 7.4.0 +PHP Version: 7.3.12
 [2019-12-09 08:26 UTC] nikic@php.net
Also reproduces on older PHP versions.
 [2019-12-09 08:36 UTC] nikic@php.net
The problem here is that the original $statement gets destroyed when the next assignment to the variable happens (that is, after the second prepare has finished). When the old $statement is destroyed a close_on_server operation on the statement is issued. This is going to reset the error state, because it performs a number of operations that may in themselves fail (like exhausting the result and closing the statement), and the error result from those operations will be used. As they don't fail, the error ends up being zero.

I don't really know what we should be doing about this. I guess one possibility is to back up the error information before we do an *implicit* close, as opposed to an explicit close with mysqli_stmt_close() (in which case we *do* want to report errors from that operation).
 [2020-12-08 01:02 UTC] dharman@php.net
Although unexpected, this is the correct behaviour. As nikic explained the destructor of mysqli_stmt is called once a new value is assigned to the same variable. The destructor performs a close operation on the MySQL server. Each time a command is sent, the error message is reset. You would need to check the error message before the mysqli_stmt is closed. 

Your code example would be equivalent to the following:

    if (false === ($statement1 = mysqli_prepare($link, 'SELECT 1'))) {
        exit(mysqli_errno($link) . ': ' . mysqli_error($link));
    }
    if (false === ($statement2 = mysqli_prepare($link, 'SELECT 1 FROM this_table_does_not_exist'))) {
        exit(mysqli_errno($link) . ': ' . mysqli_error($link));
    }
    if (false === mysqli_stmt_close($statement1)) {
        exit(mysqli_errno($link) . ': ' . mysqli_error($link));
    }
    if (false === mysqli_stmt_close($statement2)) {
        exit(mysqli_errno($link) . ': ' . mysqli_error($link));
    }

However, the recommended practice would be to enable automatic error reporting and stop worrying about manual error checking. With automatic error reporting an exception is triggered as soon as the error happens. To enable automatic error reporting just add the following line before making a connection. 

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
 [2020-12-08 18:51 UTC] craig at craigfrancis dot co dot uk
-Status: Verified +Status: Closed
 [2020-12-08 18:51 UTC] craig at craigfrancis dot co dot uk
Thanks for the update.

I think I follow, even though it's still a bit weird/unexpected.
 [2020-12-08 20:13 UTC] requinix@php.net
-Assigned To: +Assigned To: requinix
 [2020-12-08 20:13 UTC] requinix@php.net
@craig: Assuming I followed correctly, here's an alternative explanation that might help:

1  $statement = mysqli_prepare(...);
2  $statement = false;
3  $statement = mysqli_prepare(...);

Or in a little more detail,

1a  temp var <- mysqli_prepare(...);
1b  new var $statement <- temp var

2a  clean up the contents of $statement
2b  $statement <- false

3a  temp var <- mysqli_prepare(...)
3b  clean up the contents of $statement
3c  $statement <- temp var

When the prepared statement gets cleaned up during (2b), that process includes closing out the statement with the server, and the result of that gets remembered for mysqli_error/errno. You could then check this if you wanted. In step (3a), the new statement has an error and then that gets remembered.

The important part here is that if you comment out step (2), you still reuse $statement so the same cleanup process still happens with the old statement, but now it happens *after* the new statement was prepared. That means the unsuccessful act of creating the statement in (3a) is overwritten by the successful act of cleaning up the old statement in (3b).

With automatic errors, the error during (3a) would be immediately reported.
 [2020-12-08 20:14 UTC] requinix@php.net
D'oh.

> When the prepared statement gets cleaned up during (2b)
(2a)
 [2020-12-09 14:09 UTC] craig at craigfrancis dot co dot uk
Thanks @requinix, that does make it clearer.

I think the issue I've been having is the abstraction, vs what's happening in the background. Using a single $link variable as the way to get the "last" error is not ideal (considering that it's not clear as to what action you're checking).

Maybe future mysqli implementations/abstractions will make this and other things easier - my main issue being that bind_param() is not easy for new programmers, but that's a different topic for a different day :-)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 14:01:32 2024 UTC