[wp-trac] [WordPress Trac] #63255: WP_Query normalization can break plugin filters comparing query variables.

[WordPress Trac]

#63255: WP_Query normalization can break plugin filters comparing query variables.
--------------------------------------+------------------------
 Reporter:  peterwilsoncc             |       Owner:  joemcgill
     Type:  defect (bug)              |      Status:  reviewing
 Priority:  normal                    |   Milestone:  6.8.1
Component:  Query                     |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+------------------------
Changes (by joemcgill):

 * milestone:  6.8 => 6.8.1


Comment:

 Thanks @johnjamesjacoby. I've done
 [https://wpdirectory.net/search/01JRJGRVBBT7Y5FWV4ZP833G67 a quick search
 of the plugins directory] for plugins that are directly accessing
 `WP_Query::post_type` and bbPress is the only one I've come across that is
 doing an array comparison with multiple values like this. Most plugins
 that do so are either checking for a single specific post type, or
 checking to see if a value is in the array of post types.

 Regarding a few other points:

 > - the order of array values in any _Query class has simply never
 mattered – it's totally off the radar of everyone
 > - arrays of arguments are allowed to be out-of-order everywhere else
 (into and out-of functions & filters, etc...)

 To my mind, these both are reasons ''not'' to revert the current change.
 The bbPress example is actually using a strict type check (!==) in
 [https://bbpress.trac.wordpress.org/browser/trunk/src/includes/replies/functions.php?marks=2094-2097#L2094
 the code that's been referenced], which is counter to the principles you
 pointed out.

 I think the only way we include this normalization for Query caching
 without any potential for breakage is to separate the normalization for
 caching from the actual values used in the queries, as @spacedmonkey
 suggests. However, as [comment:13 I already mentioned]:

 > This may be doable, but also adds an additional level of complexity to
 cache key generation that seems undesirable.

 There very well may be unknown side effects of this change that we'll need
 to resolve, but I think the only way we'll know for sure is to ship it and
 wait for actual reports, since this has already been in trunk for quite a
 while and this is the first report I've seen about potential breakage.

 I'm going to move this to 6.8.1 and we can revert quickly if this causes
 any major unreported breakage that can't easily be fixed in a follow-up
 release.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/63255#comment:17>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform