Stop passing boolean values to mysqli_fetch_object()

[markjaquith]

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.

[pento]

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 return false without setting an error. So, the check for an error in wpdb::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?

[nacin]

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.

[pento]

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?

[markjaquith]

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.

[DrewAPicture]

@markjaquith: Do you still want to push this in for 4.0 or punt?

[michalzuber]

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 should be 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++;
		}
	}
}

[michalzuber]

$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.

[markjaquith]

Towards getting 4.0 out the door, I'm punting this and will take another look for 4.1.

[SergeyBiryukov]

#30555 was marked as a duplicate.

[petermantos]

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++;
}
}
}

[pento]

In 30677:

WPDB: Before fetching results, make sure we have a valid result resource to fetch them from.

Fixes #27982.

[pento]

#30477 was marked as a duplicate.

[pento]

#30595 was marked as a duplicate.

[mark8barnes]

I just came to report this error when using the SET command. Glad to see it's fixed in 4.1, and was able to patch 4.0.1 with the fix, too.