cache_users() not defined when calling get_user without field parameter or using all_with_meta or all

[carazo]

Good morning,

I have found that in WordPress 6.1 when I call get_users without the field parameter (or using fields = all or fields = all_with_meta) I get a fatal error.

I have been reading the code and it raise here:

Line 843 in file wp-includes/class-wp-user-query.php: cache_users( $this->results );

It seems this function is not defined there.

Could you have a look?

Thanks!

[subrataemfluence]

Hi @carazo ,

I could not reproduce the issue, it works fine for me even when I invoke get_users() without passing any argument.

Regarding Line 843 in file wp-includes/class-wp-user-query.php: cache_users( $this->results );

The said function cache_users() is located in wp-includes/pluggable.php at Line 125.

<?php
if ( ! function_exists( 'cache_users' ) ) :
        /**
         * Retrieves info for user lists to prevent multiple queries by get_userdata().
         *
         * @since 3.0.0
         *
         * @global wpdb $wpdb WordPress database abstraction object.
         *
         * @param int[] $user_ids User ID numbers list
         */
        function cache_users( $user_ids ) {
                global $wpdb;

                update_meta_cache( 'user', $user_ids );

                $clean = _get_non_cached_ids( $user_ids, 'users' );

                if ( empty( $clean ) ) {
                        return;
                }

                $list = implode( ',', $clean );

                $users = $wpdb->get_results( "SELECT * FROM $wpdb->users WHERE ID IN ($list)" );

                foreach ( $users as $user ) {
                        update_user_caches( $user );
                }
        }
endif;

I am not sure about you setup, but if possible, please deactivate all plugins and switch back to WordPress default theme. If the issue still persists, please feel free to reopen the ticket.

[carazo]

@subrataemfluence,

I was calling this function inside this plugin in version 0.5, here: ​https://plugins.trac.wordpress.org/browser/woo-products-restricted-users/tags/0.5/class-wpru-metabox.php

In line 95, the error appeared there.

[oakesjosh]

I am also experiencing cache_users not being defined when calling get_user on 6.1.

Under some conditions, my plugin calls get_users when initialized on the front end. In this context the plugin has loaded, but pluggable.php has not yet. Calling it without manually including wp-includes/pluggable.php results in a fatal error.

I believe the issue is with the context of the call and not the content

[TimothyBlynJacobs]

Moving to 6.1.1, I don't think this should be unsupported usage.

[oakesjosh]

Proposed patch

[oakesjosh]

I've attached a patch that would:

• Prevent fatal errors if get_users() is called before pluggable.php is loaded
• Not prematurely load pluggable.php so plugins can still overwrite functions

[TimothyBlynJacobs]

Thanks for the patch @oakesjosh!

The alternative would be to manually load pluggable.php instead of skipping the cache call. However, that would end up preventing any plugins loaded after the offending plugin from overriding any pluggable functions.

While not caching these requests isn't ideal, I think it's better than potentially breaking pluggable.php. We could consider adding a _doing_it_wrong here with an indication to wait until plugins_loaded before making any queries as well.

Cc: @peterwilsoncc, @spacedmonkey.

I don't think this is unit testable, since pluggable.php will have already been loaded by the test bootstrap. However, to test manually, create a plugin with the following code.

<?php
/*
 * Plugin Name: Test Plugin
 */

if ( isset( $_GET['test_query'] ) ) {
        $users = get_users();

        wp_die( sprintf( '%d users', count( $users ) ), 'Queried users', array( 'response' => 200 ) );
}

If you visit https://yoursite.com?test_query, you should see the number of users successfully queried on a WP Die screen. Otherwise, you'll see a fatal error.

Trac ticket: https://core.trac.wordpress.org/ticket/56952

[spacedmonkey]

@TimothyBlynJacobs Added a simple PR to fix this issue.

[spacedmonkey]

It is worth noting that cache_users has been called in ​this class as early as 3.1.

[obenland]

Introduced in #55594

Would this break if a plugin that defines cache_users() without a function_exists() check was loaded after the plugin that called get_users()?

Yes, though I think that is slightly less of a concern since that can always happen if another plugin overrides a pluggable function.

I don't think this approach works because it will prevent any other plugins that are loaded after the offending plugin from overriding any other pluggable functions.

[obenland]

#56988 was marked as a duplicate.

[TimothyBlynJacobs]

@dd32, @SergeyBiryukov I’d love your opinions about implementing a doing it wrong notice here. I think plugins ideally wouldn’t be doing user queries before plugins_loaded even prior to the change in 6.1. For instance, plugins also wouldn’t get an opportunity to register necessary filters.

However, I could see MU-Plugins needing to make early queries anyways. So as much as I think it’s “correct” for plugins to wait, I’m not sure we can get away with the notice here. It may just have to be a documentation note.

[SergeyBiryukov]

Replying to TimothyBlynJacobs:

I’d love your opinions about implementing a doing it wrong notice here. I think plugins ideally wouldn’t be doing user queries before plugins_loaded even prior to the change in 6.1. For instance, plugins also wouldn’t get an opportunity to register necessary filters.

However, I could see MU-Plugins needing to make early queries anyways.

Hmm, MU-plugins is a good point. Still, I strongly agree with not running anything before plugins_loaded.

Only a few things happen between muplugins_loaded and plugins_loaded:

• Cookie and SSL constants are defined.
• Initial post types and taxonomies are created.
• Default theme directory root is registered.
• Recovery mode is initialized (for single sites).
• Active plugins are loaded.
• Pluggable functions are loaded.

So I would say that even for a MU-plugin, it should be recommended to wait until plugins_loaded before runnning any queries.

Looks good to commit.

[spacedmonkey]

The PR ​#3557 is good to commit IMO.

Will commit shortly.

[TimothyBlynJacobs]

In 54766:

Query: Don't attempt caching if running a WP_User_Query before plugins_loaded.

In #55594 user meta caching was enabled by default when making a WP_User_Query. Previously, this was only enabled if a developer specifically queried for 'all_with_meta'
fields. User meta caching is implemented using a pluggable function, cache_users. If a plugin runs a WP_User_Query before pluggable functions have been defined, this
will now cause a fatal error.

In this commit, a function_exists check is introduced to avoid calling cache_users if it's not defined. Additionally, a _doing_it_wrong notice is issued if the
WP_User_Query::query method is called before the 'plugins_loaded' hook.

Props carazo, subrataemfluence, oakesjosh, spacedmonkey, obenland, SergeyBiryukov, peterwilsoncc.
Fixes #56952.

[TimothyBlynJacobs]

Reopening for backport.

[peterwilsoncc]

In 54773:

Query: Don't attempt caching if running a WP_User_Query before plugins_loaded.

In #55594 user meta caching was enabled by default when making a WP_User_Query. Previously, this was only enabled if a developer specifically queried for 'all_with_meta'
fields. User meta caching is implemented using a pluggable function, cache_users. If a plugin runs a WP_User_Query before pluggable functions have been defined, this
will now cause a fatal error.

In this commit, a function_exists check is introduced to avoid calling cache_users if it's not defined. Additionally, a _doing_it_wrong notice is issued if the
WP_User_Query::query method is called before the 'plugins_loaded' hook.

Props carazo, subrataemfluence, oakesjosh, spacedmonkey, obenland, SergeyBiryukov, peterwilsoncc, TimothyBlynJacobs.
Merges [54766] to the 6.1 branch.
Fixes #56952.

[azurseisme]

I have the bug but after applying the patch, I still have:

[Wed Nov 09 22:35:15.833980 2022] [php7:error] [pid 933177] [client 2a01:e0a:824:3ae0:ade:a428:3e00:8342:40936] PHP Fatal error:  Uncaught Error: Call to undefined function wp_cache_get() in /home/Web/Azurseisme/volcanspro/wp-includes/option.php:165\nStack trace:\n#0 /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php(63): get_option()\n#1 /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php(139): get_locale()\n#2 /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php(766): determine_locale()\n#3 /home/Web/Azurseisme/volcanspro/wp-includes/load.php(1395): load_textdomain()\n#4 /home/Web/Azurseisme/volcanspro/wp-includes/class-wpdb.php(1198): wp_load_translations_early()\n#5 /home/Web/Azurseisme/volcanspro/wp-includes/class-wpdb.php(1858): wpdb->select()\n#6 /home/Web/Azurseisme/volcanspro/wp-includes/class-wpdb.php(734): wpdb->db_connect()\n#7 /home/Web/Azurseisme/volcanspro/wp-includes/load.php(562): wpdb->__construct()\n#8 /home/Web/Azurseisme/volcanspro/wp-settings.php(124): require_wp_db()\n#9 /home/Web/Azurseisme/volcanspro/wp-config.php(89): require_once('/home/Web/Azurs...')\n#10 /home/Web/Azurseisme/volcanspro/wp-load.php(50): require_once(' in /home/Web/Azurseisme/volcanspro/wp-includes/option.php on line 165, referer: https://volcanspro.azurseisme.com/les-galapagos/
[Wed Nov 09 22:35:15.842924 2022] [php7:error] [pid 933177] [client 2a01:e0a:824:3ae0:ade:a428:3e00:8342:40936] PHP Fatal error:  Uncaught Error: Call to a member function set() on null in /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php:784\nStack trace:\n#0 /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php(872): load_textdomain()\n#1 /home/Web/Azurseisme/volcanspro/wp-includes/class-wp-fatal-error-handler.php(47): load_default_textdomain()\n#2 [internal function]: WP_Fatal_Error_Handler->handle()\n#3 {main}\n  thrown in /home/Web/Azurseisme/volcanspro/wp-includes/l10n.php on line 784, referer: https://volcanspro.azurseisme.com/les-galapagos/

[TimothyBlynJacobs]

Based on that error trace, that looks to be a separate issue @azurseisme. Would you mind opening a new ticket for it?

[azurseisme]

For sure, #57051
thanks,