Warning / Error in wp-includes/canonical.php when $_GET['author'] is an array

[david.binda]

On a WordPress install with pretty permalinks turned on, there is a PHP warning or error (depends on the PHP version) when viewing an author on frontend, while the $_GET['author'] is an array. For instance:

/author/canonical-author/?author[1]=hello

produces following fatal error on PHP 8+:

Fatal error: Uncaught TypeError: preg_match(): Argument #2 ($subject) must be of type string, array given

and following warning on lower versions of PHP:

Warning: preg_match() expects parameter 2 to be string, array given

Related code line: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/canonical.php?rev=56738#L319

A fix seems to be to check if is_string( $_GET['author'] ) before performing the preg_match.

[antonvlasenko]

I don't have the time to test this now, but I have 2 concerns:

1. I'm wondering why there is an array in the GET parameters. Did it work like that before? If not (i.e. is this a recent bug), what caused an array to appear in the GET parameters?

A fix seems to be to check if is_string( $_GET['author'] ) before performing the preg_match.

2. In the example you provided, hello seems to be a valid author name. If this bug was not caused by some recent change, perhaps it's worth considering getting the first element of the author array and using it as an author name (in case it's a string), as opposed to ignoring arrays.

[david.binda]

I'm wondering why there is an array in the GET parameters. Did it work like that before? If not (i.e. is this a recent bug), what caused an array to appear in the GET parameters?

The concern raised in terms of this ticket is the PHP 8 compatibility of the code. While the code under PHP 7 "works" and "just" produces a warning while displaying the author page (ignoring the GET param), under PHP 8 it produces a fatal error.

So it's not the functionality of an array passed to the author GET param, sorry for not being clear.

[azaozz]

Makes sense to check for and ignore arrays there but don't seem to be able to reproduce this (in trunk). Unless I'm missing something it seems a non-scalar query value for author is ignored and is_author() returns false. See: https://core.trac.wordpress.org/browser/tags/6.4.2/src/wp-includes/class-wp-query.php#L825.

[SergeyBiryukov]

Replying to azaozz:

Makes sense to check for and ignore arrays there but don't seem to be able to reproduce this (in trunk). Unless I'm missing something it seems a non-scalar query value for author is ignored and is_author() returns false.

I can reproduce the issue as described.

It appears that the preg_match( '|^[0-9]+$|', $_GET['author'] ) check in redirect_canonical() runs after the validation in WP_Query::parse_query(), and is unaffected by that validation because it checks the $_GET['author'] value directly, not the parsed value.

[SergeyBiryukov]

In 57232:

Canonical: Check if the author parameter is a string in redirect_canonical().

This avoids a PHP warning or error when viewing an author on the front end, while an array is passed as $_GET['author'].

Follow-up to [12034], [12040], [12202].

Props david.binda, antonvlasenko, azaozz, SergeyBiryukov.
Fixes #60059.

[ironprogrammer]

Though this has already been merged (thanks, @davidbinda and @SergeyBiryukov!), I wanted to share separate confirmation that the patch addresses the issue.

Test Report

Patch tested: attachment:60059.diff​

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 14.2.1
• Browser: Safari 17.2.1
• Server: nginx/1.25.3
• PHP: 8.2.13
• WordPress: 6.5-alpha-56966-src
• Theme: twentytwentyfour v1.0

Actual Results

• ✅ Viewing /author/admin/?author[1]=hello (or variations) does not produce errors outlined in ticket description.