Opened 2 years ago
Closed 12 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
@
2 years ago
- Keywords needs-patch added
- Milestone Awaiting Review → Future Release
- Severity major → normal
- Version 6.6.1 → 2.3
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
Trac ticket: https://core.trac.wordpress.org/ticket/61936
#3
@
2 years 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?
@westonruter commented on PR #7278:
2 weeks ago
#5
(Sorry for the rebase. Things got a bit messy with the force-push that happened 2 months ago it seems.)
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
This just came up in https://github.com/php-stubs/wordpress-stubs/pull/195 and is worth looking into.