Setting a 'NOT EXISTS' tax query in 'pre_get_posts' action triggers a fatal error in wp-includes/canonical.php

[codesdnc]

Under PHP8, the following code will fail on taxonomy archives: (is cut-down to its minimal form)

<?php
add_action( 'pre_get_posts', function( \WP_Query $query ){
  $query->set( 'tax_query', array(
    array(
      'taxonomy' => 'post_format',
      'operator' => 'NOT EXISTS',
    ),
  ) );
} );

Digging in the code, on line 334 of wp-includes/canonical.php, it assumes that for each element in WP_Tax_Query->queried_terms, there will be a 'terms' entry that is an (a) defined, and (b) an array. However, WP_Tax_Query::sanitize_query() (the only place I can see where queried_terms is set) does NOT guarantee this for cases like above -- it simply doesn't set a 'terms' entry for 'operator' => 'NOT EXISTS' queries (which makes sense). This is only a nuisance before PHP8, which makes passing non-arrays to count() throw an error.

As discussed in 55955, a NOT EXISTS tax_query causes a warning (PHP <= 7.4) or an error (PHP >= 8.0) in the redirect_canonical function because there is a count( $tax_query['terms'] ) check in that function that does not verify whether the terms property is present. It will not be present in NOT EXISTS queries because there is no purpose in specifying individual terms when we are querying for posts which have no terms at all.

Trac ticket: https://core.trac.wordpress.org/ticket/55955

[kadamwhite]

We ran into this one while upgrading a site to PHP 8 today, thank you @codesdnc for the clear ticket! It saved us a lot of time understanding why the feeds were failing.

I paired up with @miguelaxcar to debug this and create the linked PR.

I'm not certain whether there's a better way to set up the global $wp_query than in the unit test I added, but when we tried manipulating the global query directly the change didn't seem to trigger the error. We validated locally by setting LOCAL_PHP=8.0-fpm in our .env file when running unit tests.

[kadamwhite]

For anybody running into this between now and when the patch goes live, we've worked around it in our application by passing

    'terms' => array( 0 ),

within the tax_query array. Looking at how NOT EXISTS queries get parsed, it seems like this terms parameter is not used at all, so it seems to mitigate the PHP8 fatal without compromising application logic...

[peterwilsoncc]

I did a run of @kadamwhite's ​tests without the fix to ensure the tests are correct. They are.

I think this is ready for commit. I'll do so for trunk but leave it up to @desrosj and @jeffpaul to decide if they would like to put this in the minor release.

@MiguelAxcarHM are you able to link your WordPress.org account to your GitHub account so I can ensure you are given props correctly? This can be done in the GitHub username section of ​https://profiles.wordpress.org/me/profile/edit/group/1/

[peterwilsoncc]

In 54785:

Canonical: Protect against error for term not exists queries.

Prevent term NOT EXISTS queries causing redirect_canonical() to throw a fatal error in PHP 8 and above, or a warning in earlier versions.

This ensures the tax_query's terms property both exists and is countable before attempting to count it.

Props codesdnc, SergeyBiryukov, kadamwhite, costdev, miguelaxcar.
Fixes #55955.

Merged in https://core.trac.wordpress.org/changeset/54785 / b9304148f17278dbe9a4199a1143b083188f0d61

[peterwilsoncc]

Reopening for backport consideration.

If decided against, please move to the 6.2 milestone and reclose.

Done @peterwilsoncc , thank you! :)

[desrosj]

I think this is worth including. Going to backport. Thanks everyone!

[desrosj]

In 54793:

Canonical: Protect against error for term not exists queries.

Prevent term NOT EXISTS queries causing redirect_canonical() to throw a fatal error in PHP 8 and above, or a warning in earlier versions.

This ensures the tax_query's terms property both exists and is countable before attempting to count it.

Props codesdnc, SergeyBiryukov, kadamwhite, costdev, miguelaxcar.
Merges [54785] to the 6.1 branch.
Fixes #55955.