#27982 closed defect (bug) (fixed)
Stop passing boolean values to mysqli_fetch_object()
Reported by: | markjaquith | Owned by: | markjaquith |
---|---|---|---|
Milestone: | 4.1 | Priority: | normal |
Severity: | normal | Version: | 3.9 |
Component: | Database | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
Our call to mysqli_fetch_object()
in wp-db.php sometimes receives a $result
that is a boolean.
This makes our unit tests blow up in HHVM, as their implementation of mysqli_fetch_object()
isn't as forgiving when receiving an unexpected value.
A simple fix is to do an is_object()
test before the call. This lets our unit tests run in HHVM. But also I'd like to know why $result
has a boolean value.
Attachments (2)
Change History (20)
#1
@
10 years ago
- Keywords has-patch 2nd-opinion added
- Milestone changed from Awaiting Review to 3.9.1
- Owner set to pento
- Status changed from new to accepted
#2
@
10 years ago
Is this something specific to mysqli? Is there a concern this could happen with ext/mysql? I'd assume we would have noticed this, but it's also error-suppressed.
#3
@
10 years ago
On further reflection, I'm not entirely sure why it isn't being caught by our existing query-type handling.
Mark, are there any particular unit tests that are consistently breaking here?
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
10 years ago
#5
@
10 years ago
Our query type handling misses "SET" queries. Those hit the else branch. Those have a boolean result. We then try to pass that boolean to mysqli_fetch_object()
.
I think we can safely skip the while loop if is_bool( $this->result )
. Passing a boolean to `mysqli_fetch_object() is bad behavior.
#9
@
10 years ago
Couldn't find boolean result source, worth mentioning I'm not a C guru :)
Looked at:
According to me from PHP source it shouldn't return boolean only if it's array
if (into_object && Z_TYPE_P(return_value) == IS_ARRAY)
ext/mysqli/mysqli.c#L1285
Checked why might mysql_fetch_object
be error suppressed wp-includes/wp-db.php#L1617 and
found that the initial class author https://github.com/WordPress/WordPress/blob/43e4a8a930be8b3d0359e517e4a6ef12f0d0ba73/wp-includes/wp-db.php#L203 had it.
Yeah, I'm aware that the following isn't the "nicest", but better then it is:
if ( $this->use_mysqli ) { if ( WP_DEBUG ) { while ( $row = mysqli_fetch_object( $this->result ) ) { $this->last_result[$num_rows] = $row; $num_rows++; } } else { while ( $row = @mysqli_fetch_object( $this->result ) ) { $this->last_result[$num_rows] = $row; $num_rows++; } } } else { if ( WP_DEBUG ) { while ( $row = mysql_fetch_object( $this->result ) ) { $this->last_result[$num_rows] = $row; $num_rows++; } } else { while ( $row = @mysql_fetch_object( $this->result ) ) { $this->last_result[$num_rows] = $row; $num_rows++; } } }
#10
@
10 years ago
$this->result
is set at line 1652
According to http://php.net/manual/en/mysqli.query.php there are boolean value situations:
Returns FALSE on failure. For successful SELECT, SHOW, DESCRIBE or EXPLAIN queries mysqli_query() will return a mysqli_result object. For other successful queries mysqli_query() will return TRUE.
#11
@
10 years ago
- Milestone changed from 3.9.2 to Future Release
Towards getting 4.0 out the door, I'm punting this and will take another look for 4.1.
#13
@
10 years ago
wp-db.php has a bug around line 1616 (most recent version as of 11/30/2014)
They error reported is:
Debug Warning: ...\wp-includes\wp-db.php line 1622
mysql_fetch_object(): supplied argument is not a valid MySQL result resource
A similar comes up under different circumstances that basically states that the result is a boolean, not an object.
The solution is to make a test as to whether or not the result is an object using is_object as follows:
$num_rows = 0;
if ( is_object($this->result ) ) {
if ( $this->use_mysqli ) {
while ( $row = @mysqli_fetch_object( $this->result ) ) {
$this->last_result[$num_rows] = $row;
$num_rows++;
}
} else {
while ( $row = @mysql_fetch_object( $this->result ) ) {
$this->last_result[$num_rows] = $row;
$num_rows++;
}
}
}
I've had a bit of a dig through HHVM/PHP, and it seems there might be some cases (with my limited knowledge of how the PHP internals work) where
mysqli_query()
can returnfalse
without setting an error. So, the check for an error inwpdb::query()
doesn't catch it.With attachment:27982.diff,
wpdb::print_error()
will record the error, but with an empty error string - I'm not wild about this behaviour, perhaps it should have a default error string, so we're at least recording something?