Make WordPress Core

Opened 5 weeks ago

Last modified 4 weeks ago

#62003 new defect (bug)

User_Query cache triggers fatal error by ignoring 'fields' query var

Reported by: sermitr's profile sermitr Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.3
Component: Users Keywords:
Focuses: performance Cc:

Description

Method WP_User_Query::query() generates different results depending on whether $this->query_vars['fields'] an array (array of objects) or a string (array of scalar values).

However, if it contains only one field, the generated SQL query will be the same.

Function generate_cache_key() doesn't take into account the fields query var, and relies on the generated SQL query instead. This leads to the unpredictability of the returned data format (array of objects or array of scalar values).

How to reproduce:

  1. Clean the cache: wp cache flush.
  2. Fetch the users passing fields as an array:
    <?php
    $wp_user_search = new WP_User_Query( array( 'fields' => array( 'ID' ), 'count_total' => false ) );
    $users = $wp_user_search->get_results();
    var_dump( $users );
    // Result is an array of objects, which is now cached: array(1) { [0]=> object(stdClass)#1268 (2) { ["ID"]=> string(1) "1" ["id"]=> string(1) "1" } } 
    
  3. Run the same request with the fields being a string: 'fields' => 'ID'. The result will be an array of objects pulled from cache, although array of strings is expected.
  4. Fetch the users by CLI command: wp user list. The result is expected to be an array of strings, so the fatal error is triggered:
    Fatal error: Uncaught TypeError: Illegal offset type in wp-includes/class-wp-user-query.php:892
    
  5. Clean the cache: wp cache flush
  6. Run the request from step 3, where fields is a string:
    <?php
    $wp_user_search = new WP_User_Query(array('fields' => 'ID', 'count_total' => false));
    $users = $wp_user_search->get_results();
    var_dump( $users);
    // Result is an array of strings, which is now cached: array(1) { [0]=> string(1) "1" } 
    
  7. Run the request from step 2, where fields is an array. The result be the same array of strings pulled from cache, which is different from what we had on step 2.
  8. Run the CLI command wp user list. There will be no fatal error, because the data pulled from cache is an array of strings as expected.

Change History (2)

#1 @sermitr
5 weeks ago

The workaround here is cleaning the cache and disabling it for requests where fields is an array of one element:

<?php
add_filter( 'users_pre_query', function( $null, $query ) {
    if ( ! empty( $query->query_vars['fields'] ) && is_array( $query->query_vars['fields'] ) && count( $query->query_vars['fields'] ) === 1 ) {
        $query->query_vars['cache_results'] = false;
    }

    return $null;
}, 10, 2 );

#2 @hellofromTonya
4 weeks ago

  • Focuses performance added
  • Version changed from 6.6.1 to 6.3

Hello @sermitr,

Welcome to WordPress Core's Trac. Thank you for all of the details, including steps to reproduce, in this ticket's report.

I'm doing triage for 6.6.x cycle. The Version that introduced this code was 6.3, via #40613 and changeset [55657]. Updating the Version to reflect when the code was introduced. Also noting, no changes were made in this method or its usage in 6.6.1.

The original enhancement that added this code was marked for performance. Adding that focus here.

To help contributors, sharing the dev note link here for contextual awareness.

Note: See TracTickets for help on using tickets.