Make WordPress Core

Opened 2 years ago

Closed 6 days ago

#61936 closed defect (bug) (fixed)

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

Reported by: marian1 Owned by: westonruter
Priority: normal Milestone: 7.2
Component: Taxonomy Version: 2.3
Severity: normal Keywords: has-unit-tests
Cc: Focuses:

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 (6)

#1 @swissspidy
2 years ago

  • Keywords needs-patch added
  • Milestone Awaiting ReviewFuture Release
  • Severity majornormal
  • Version 6.6.12.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.


2 years ago
#2

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

#3 @marian1
2 years ago

  • Keywords has-patch removed

This is a first step. However, 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" will never occur, except for 'hierarchical', which is part of both arrays. However, I am unsure whether this overriding should occur at all. If the array_merge worked as intended, each taxonomy could have a different 'fields' value, resulting in a relatively unpredictable return type.

The function new 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's also necessary to review the other functions that call wp_get_object_terms().

Version 1, edited 2 years ago by marian1 (previous) (next) (diff)

#4 @westonruter
8 days ago

  • Milestone Future Release7.2
  • Owner set to westonruter
  • Status newreviewing

@westonruter commented on PR #7278:


8 days ago
#5

(Sorry for the rebase. Things got a bit messy with the force-push that happened 2 months ago it seems.)

#6 @westonruter
6 days ago

  • Resolutionfixed
  • Status reviewingclosed

In 63488:

Taxonomy: Fix wp_get_object_terms() when requesting a count.

Requesting a count passed the numeric string get_terms() returns straight to array_merge(), which warns and yields null on PHP 7.4 and throws a TypeError on PHP 8.0 and later. The count is now cast before it is merged, and summed once every taxonomy has been queried, so the function returns the numeric string its documentation has described since r49947. An empty object or taxonomy list returns '0' for the same reason, rather than the empty array that contradicted the documented type.

A taxonomy registered with an args array is queried by a separate recursive call, and those results were merged with array_merge() unconditionally. For the id=> values of fields the term IDs are integer array keys, which array_merge() renumbers, so such a taxonomy came back keyed from zero. The recursive branch now uses the union operator for those, as the merge for the remaining taxonomies has done since r41809.

The functions get_terms(), wp_get_object_terms(), wp_count_terms(), wp_get_post_categories(), wp_get_post_tags() and wp_get_post_terms() each gain a conditional return type that resolves the result from the fields value, falling back to what that function's own default answers with, as was done for WP_Term_Query::query() in r63358.

Developed in https://github.com/WordPress/wordpress-develop/pull/7278.
Follow-up to r38667, r40513, r41809, r49947, r63358.

Props marian1, westonruter, swissspidy.
See #65817, #66049.
Fixes #61936.

Note: See TracTickets for help on using tickets.