WP_User_Query:prepare_query() bug leads to user meta query with blog_id 0

[stephdau]

In WP_User_Query:prepare_query(), one can find the following line, to "Prevent extra meta query" (SIC):

$qv['blog_id'] = $blog_id = 0;

Setting the blog_id to 0 was, in previous versions of that code (from r32207), to make the code not proceed with further test cases afterward, as one can find if ( $blog_id && peppered throughout this method.

In 4.6, this method has changed (r37360) to have more processing after this blog_id reset happens, leading to a query being place with AND blog_id = 0 instead of the intended proper blog_id that exists before that reset. Said query therefore becomes "invalid", since it will not return the expected blog users.

In my experience, this happens at least when wp_dropdown_users() tries to get a list of the blog's users in the post/page edit screen.

Both r32207 and r37360 by @boonebgorges :)

Sample data lifespan (note how blog_id becomes 0):

get_users(): array (
  'blog_id' => '114310708',
  'include' => '',
  'exclude' => '',
  'orderby' => 'display_name',
  'order' => 'ASC',
  'who' => 'authors',
  'fields' => 
  array (
    0 => 'ID',
    1 => 'user_login',
    2 => 'display_name',
  ),
  'count_total' => false,
)


User query construct: array (
  'blog_id' => '114310708',
  'include' => '',
  'exclude' => '',
  'orderby' => 'display_name',
  'order' => 'ASC',
  'who' => 'authors',
  'fields' => 
  array (
    0 => 'ID',
    1 => 'user_login',
    2 => 'display_name',
  ),
  'count_total' => false,
)


Bad WP_User_Query attempt. Args:
Array
(
    [blog_id] => 0
    [role] => 
    [role__in] => Array
        (
        )

    [role__not_in] => Array
        (
        )

    [meta_key] => 
    [meta_value] => 
    [meta_compare] => 
    [include] => 
    [exclude] => 
    [search] => 
    [search_columns] => Array
        (
        )

    [orderby] => display_name
    [order] => ASC
    [offset] => 
    [number] => 
    [paged] => 1
    [count_total] => 
    [fields] => Array
        (
            [0] => ID
            [1] => user_login
            [2] => display_name
        )

    [who] => authors
    [has_published_posts] => 
)

[boonebgorges]

Hi @stephdau - Thanks for the report.

At a glance, I'm not seeing how to reproduce the problem. Here's the WP_User_Query object (including its request and query results), as generated at the end of prepare_query(), when created via wp_dropdown_users() on the Author metabox of a single post on a secondary site in a multisite network:

 WP_User_Query Object
(
    [query_vars] => Array
        (
            [blog_id] => 0
            [role] => 
            [role__in] => Array
                (
                )

            [role__not_in] => Array
                (
                )

            [meta_key] => 
            [meta_value] => 
            [meta_compare] => 
            [include] => 
            [exclude] => 
            [search] => 
            [search_columns] => Array
                (
                )

            [orderby] => display_name
            [order] => ASC
            [offset] => 
            [number] => 
            [paged] => 1
            [count_total] => 
            [fields] => Array
                (
                    [0] => ID
                    [1] => user_login
                    [2] => display_name
                )

            [who] => authors
            [has_published_posts] => 
        )

    [results:WP_User_Query:private] => 
    [total_users:WP_User_Query:private] => 0
    [meta_query] => WP_Meta_Query Object
        (
            [queries] => Array
                (
                    [0] => Array
                        (
                            [key] => wp_2_user_level
                            [value] => 0
                            [compare] => !=
                        )

                )

            [relation] => 
            [meta_table] => wp_usermeta
            [meta_id_column] => user_id
            [primary_table] => wp_users
            [primary_id_column] => ID
            [table_aliases:protected] => Array
                (
                    [0] => wp_usermeta
                )

            [clauses:protected] => Array
                (
                    [wp_usermeta] => Array
                        (
                            [key] => wp_2_user_level
                            [value] => 0
                            [compare] => !=
                            [alias] => wp_usermeta
                            [cast] => CHAR
                        )

                )

            [has_or_relation:protected] => 
        )

    [request] => 
    [compat_fields:WP_User_Query:private] => Array
        (
            [0] => results
            [1] => total_users
        )

    [query_fields] => wp_users.ID,wp_users.user_login,wp_users.display_name
    [query_from] => FROM wp_users INNER JOIN wp_usermeta ON ( wp_users.ID = wp_usermeta.user_id )
    [query_where] => WHERE 1=1 AND ( 
  ( wp_usermeta.meta_key = 'wp_2_user_level' AND wp_usermeta.meta_value != '0' )
)
    [query_orderby] => ORDER BY display_name ASC
    [query_limit] => 
)

The blog_id "zeroing" happens inside of the who block. The idea here (introduced in [17088]) is that if you're doing who=authors, then further role queries are not relevant. Even after [37360], this still seems to be the case. You mention that after [37360], "more processing" is taking place after the reset; but as far as I can see, the *opposite* is true, since the reset was moved later in the method. Indeed, in my tests, the correct users are showing up in the dropdown. (See also ​http://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/user/query.php, which contains a number of who_authors tests that seem to say the opposite of what you're saying here.)

Can you give more details about exactly what you're seeing on the Author dropdown, and other places where you're experiencing the issue? Is the dropdown being populated with *all* users from the network? Or is it specifically looking for users who are members of blog_id=0 (ie, users who are not members of any sites)?

One thing that is true - though doesn't appear to be new to 4.6 - is that the reset blog_id is being passed to the 'pre_user_query' hook and other filters. As such, it's possible that a callback is expecting to get the original blog_id, but is getting 0 instead, and is doing something weird. Is it possible that something like this is happening in your case?

[stephdau]

@boonebgorges: OK, got it, it's more subtle than that. :)

In my case, the "issue" was brought up by a query analyzing tool we use, hooking into WP_User_Query:prepare_query(). It gets triggered because the query is being prepared as stated, but the query is ultimately not run in the method, thanks to further if ( $blog_id && tests. It gets tripped because there is now potential for the query to be run, not because it actually is.

So, it's not a problem as the method currently stands, but might be something to think about as far as its maintainability in a cooperative OSS project, as it's probably pretty prone to future errors. :)

[stephdau]

Hmmm, there's more to it than that, still looking.

[stephdau]

I'm calling this one good. The query is valid, it's our query analyzer that needs updating. Thanks for looking, sorry for the false positive. :)