WP_Query->parse_tax_query() can't handle arrays from URL querystring

[Veraxus]

When using querystring variables to filter taxonomies (default WordPress behavior) WP_Query->parse_tax_query() assumes it's getting a string from the URL (?tax=1,2,3,4).

However, if using HTML form-based filtering, such as a checkbox list, an array is passed instead. When this happens, not only does the search/filter not work, a slew of warnings are generated since WP_Query->parse_tax_query() is only capable of handling strings.

Fortunately, this is easily fixed with just three lines. If the array is detected and imploded before being handled, everything works as expected/designed without generating any errors or warnings.

Patch file is attached.

[Veraxus]

parse_tax_query() correctly detects & implodes arrays

[johnbillion]

Thanks for the patch Veraxus.

This will need unit tests to show that it fixes the issue at hand and that it doesn't have any unintended side effects.

[Veraxus]

Unit tests for WP_Query->parse_tax_query() patch.

[Veraxus]

I've included unit tests for single term and multi-term use cases.

Pre-patch:

• Single term unit test passes
• Multi-term unit test fails (error)

With patch:

• Single term unit test passes
• Multi-term unit test passes

[Veraxus]

Is anything else needed for this? It would be ideal if we got this bug squished before 4.3

[obenland]

@johnbillion looks like this only needs a final review and commit?

[boonebgorges]

In 33095:

In WP_Query::parse_tax_query(), allow taxonomy querystring to be formatted as an array.

Props Veraxus.
Fixes #32454.

[ocean90]

@boonebgorges Looks like the PHPUnit tests doesn't pass in PHP > 5.2, see ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/69794056.

[Veraxus]

Taking a look... although at upload time I ran the tests successfully on 5.2.16 and 5.6.9

[ocean90]

I think replacing assertEquals() with assertEqualSets() should fix this, see 32454-ut.patch​.

[Veraxus]

Replying to ocean90:

I think replacing assertEquals() with assertEqualSets() should fix this, see 32454-ut.patch​.

I'm not able to reproduce the errors on 5.3, 5.4, 5.5, or 5.6 with the latest trunk - and I manually checked the values being passed to assertEquals() as well.

Generally speaking, assertEqualSets() shouldn't be necessary… unless wp_list_pluck() is returning it's array out of order for some reason. It's a safe alternative in this case, at least.

[boonebgorges]

Thanks, @ocean90. assertEqualSets() is correct here. The issue is that local instances of phpunit often run more quickly than those on Travis VMs, with the result that race conditions appear on Travis that aren't possible to reproduce locally.

[boonebgorges]

In 33102:

Use assertEqualSets() in WP_Query::parse_tax_query() tests.

Props ocean90.
See #32454.