Replacing one variable handler for another to ensure proper conditional check occurs 100%.

[chadschulz]

I have a likely non-reproduceable problem that, nonetheless, still indicates a failure of a conditional check in wp-class.php.

I'd been receiving log file non-critical php errors/warnings for months referencing the rawurlencode() on line 528 getting an array variable. Which is not allowed for that function, hence the error.

When I changed line 526's conditional check from

if ( !is_scalar($this->query_vars[$wpvar]) )

to

if ( ( !is_scalar($this->query_vars[$wpvar]) || is_array($this->query_vars[$wpvar]) ) )

the errors went away.

Bad plugin/theme code, notwithstanding, the fact that the conditional check fails at all indicates a need for "refinement". That's the whole point of these checks, to ensure bad code doesn't take down a website.

I'm even wondering if a more streamlined code change for line 526 to

if ( !is_string($this->query_vars[$wpvar]) )

is more appropriate as string variables are all that rawurlencode() allows.

I hated changing any core file. But, I was able to quickly address a small issue that shouldn't have been allowed by the core. I don't think this change would impact anyone, the function build_query_string() this code falls within has been depreciated, anyway.

BTW, I'm not sure why is_scalar() is failing. However, I came across a few instances across the web with similar quirks. Everyone of those problems was addressed in a similar fashion (i.e. using is_array() or !is_string() instead of !is_scalar()).

Source code from wp-class.php:

521	        public function build_query_string() {
522	                $this->query_string = '';
523	                foreach ( (array) array_keys($this->query_vars) as $wpvar) {
524	                        if ( '' != $this->query_vars[$wpvar] ) {
525	                                $this->query_string .= (strlen($this->query_string) < 1) ? '' : '&';
526	                                if ( !is_scalar($this->query_vars[$wpvar]) ) // Discard non-scalars.
527	                                        continue;
528	                                $this->query_string .= $wpvar . '=' . rawurlencode($this->query_vars[$wpvar]);
529	                        }
530	                }

[chadschulz]

Patch 1

[chadschulz]

Oddly, I just found a possible source of the "bad code". There's an issue inside HHVM that affects multidimensional arrays using http_build_query with enc_type = PHP_QUERY_RFC3986 which might allow them to slip past !is_scalar().

Turns out one of the plugins I'm using, Shield WordPress Security, uses that kind of function. So this might all be a (fixable) bug inside HHVM.

However, this sort of "glitch" should still not be allowed through core as the point of this particular !is_scalar() conditional is to filter out junk that won't pass rawurlencode(). And !is_string() isn't any slower that !is_scalar()--initial benchmarks show it might actually be a tad faster. It's also more specific to this purpose.

--None of this, it turns out, is relevant. As after this HHVM issue is fixed the php errors persist.

So, I stand by my initial patch request.

[chadschulz]

Decided it's not worth the effort. Obviously a small issue that only affects me.

[chadschulz]

Applied the bug patch for HHVM and the error persists.

\nWarning: rawurlencode() expects parameter 1 to be string, array given in /wordpress/wp-includes/class-wp.php on line 528

Reapplying my patch, once again, alleviates this issue for me. So...

[chadschulz]

Further reading reveals that HHVM may handle strict type checking differently than PHP. This could lead to occasional warnings when types aren't exactly as expected even if PHP under similar circumstances wouldn't throw any errors/warnings.

Not sure if this changes anything as this particular conditional is looking to ween out any non-strings. And !is_scalar() is not specific enough (for either PHP or HHVM) while !is_string() does exactly what is required for rawurlencode().