Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#61936 new defect (bug)

wp_get_object_terms: Unexpected return type (PHP 7.4)/fatal error (PHP 8.0+) when requesting a count

Reported by: marian1's profile marian1 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.3
Component: Taxonomy Keywords: has-unit-tests
Focuses: Cc:

Description

Similar to get_terms(), which is called by wp_get_object_terms(), and as outlined in the return type description, wp_get_object_terms() should return a count of terms as a numeric string when $args = array('fields' => 'count'). However, requesting a count for an existing object and valid taxonomy results in wp_get_object_terms() returning null and triggering a warning on PHP 7.4, and causing a fatal error on PHP 8.0+.

This issue is caused by the use of the return value of get_terms(), which is a numeric string for $args = array('fields' => 'count'), in array_merge(). See lines 2316-2323 in src/wp-includes/taxonomy.php.

Change History (3)

#1 @swissspidy
7 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from major to normal
  • Version changed from 6.6.1 to 2.3

This just came up in https://github.com/php-stubs/wordpress-stubs/pull/195 and is worth looking into.

This ticket was mentioned in PR #7278 on WordPress/wordpress-develop by @marian1.


6 weeks ago
#2

  • Keywords has-patch has-unit-tests added; needs-patch removed

#3 @marian1
6 weeks ago

  • Keywords has-patch removed

Given the changes made in PR #7278 , the function returns the expected count when requesting a count. Like get_terms() and WP_Term_Query->get_terms(), it does not correct for double counting if the same term is assigned to multiple objects or taxonomies.

It is also necessary to review the other functions that call wp_get_object_terms().

While examining this issue, I found that this entire section seems incorrect:

/*
 * When one or more queried taxonomies are registered with an 'args' array,
 * those parameters override the `$args` passed to this function.
 */
$terms = array();
if ( count( $taxonomies ) > 1 ) {
    foreach ( $taxonomies as $index => $taxonomy ) {
        $t = get_taxonomy( $taxonomy );
        if ( isset( $t->args ) && is_array( $t->args ) && array_merge( $args, $t->args ) != $args ) {
            unset( $taxonomies[ $index ] );
            $terms = array_merge( $terms, wp_get_object_terms( $object_ids, $taxonomy, array_merge( $args, $t->args ) ) );
        }
    }
} else {
    $t = get_taxonomy( $taxonomies[0] );
    if ( isset( $t->args ) && is_array( $t->args ) ) {
        $args = array_merge( $args, $t->args );
    }
}

The 'args' that the taxonomies are registered with are located in $t->args['args'], not $t->args. Therefore, the condition array_merge( $args, $t->args ) != $args is always true, but the incorrect array is merged. The statement "those parameters override the $args passed to this function" only holds for 'hierarchical', which is part of both arrays. As far as I can tell, whether 'hierarchical' is overridden or not does not change the return value of wp_get_object_terms(), as it requires $object_ids to be set.

Fixing this by replacing $t->args with $t->args['args'] is straightforward; however, I am unsure whether this overriding should occur at all. If the array_merge worked as intended, each taxonomy could have, for example, a different 'fields' value, resulting in a relatively unpredictable return type and value. Additionally, one taxonomy could have been registered with $args['args'] = ['fields' => 'count']. When requesting an array of taxonomies, the return value would be a mix of term objects and term counts. This is not only unpredictable but would also result in WP_Term_Query::format_terms() accessing non-existent term properties. I suggest at least excluding 'fields' when merging $args with $t->args['args'].

Given that this is actually a separate issue, should I open a new ticket to address it?

Last edited 6 weeks ago by marian1 (previous) (diff)
Note: See TracTickets for help on using tickets.