Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20460 closed defect (bug) (fixed)

Notice with clean_user_cache when deleting a user

Reported by: blueyed Owned by: duck_
Milestone: 3.4 Priority: normal
Severity: minor Version: 3.4
Component: Multisite Keywords: has-patch
Focuses: Cc:


I just got the following error when deleting a user in a network setup site / multisite setup:

Notice: Trying to get property of non-object in /srv/wordpress/wp-includes/user.php on line 1152
Call Stack:
    0.0000     342776   1. {main}() /srv/wordpress/wp-admin/network/users.php:0
    0.1137    3522036   2. wpmu_delete_user() /srv/wordpress/wp-admin/network/users.php:201
    0.1278    3528060   3. clean_user_cache() /srv/wordpress/wp-admin/includes/ms.php:165

This can happen when $user = WP_User::get_data_by( 'id', $id ); is "false".

The patch appears to be trivial, so I am attaching it inline:

diff --git i/wp-includes/user.php w/wp-includes/user.php
index 463f311..8d99dd5 100644
--- i/wp-includes/user.php
+++ w/wp-includes/user.php
@@ -1149,9 +1149,11 @@ function clean_user_cache($id) {
        $user = WP_User::get_data_by( 'id', $id );
        wp_cache_delete($id, 'users');
-       wp_cache_delete($user->user_login, 'userlogins');
-       wp_cache_delete($user->user_email, 'useremail');
-       wp_cache_delete($user->user_nicename, 'userslugs');
+       if ( is_object($user) ) {
+               wp_cache_delete($user->user_login, 'userlogins');
+               wp_cache_delete($user->user_email, 'useremail');
+               wp_cache_delete($user->user_nicename, 'userslugs');
+       }

Apart from that I suggest the following documentation fix:

diff --git i/wp-includes/capabilities.php w/wp-includes/capabilities.php
index 19568e5..481edd8 100644
--- i/wp-includes/capabilities.php
+++ w/wp-includes/capabilities.php
@@ -482,7 +482,7 @@ class WP_User {
         * @param string $field The field to query against: 'id', 'slug', 'email' or 'login'
         * @param string|int $value The field value
-        * @return object Raw user object
+        * @return object|boolean Raw user object, false on error.
        static function get_data_by( $field, $value ) {
                global $wpdb;

I am using WP trunk (via the Github mirror).

Attachments (2)

20460.diff (2.6 KB) - added by duck_ 3 years ago.
20460.2.diff (2.6 KB) - added by duck_ 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 @duck_3 years ago

The problem with the patch is that it could leave old data in the cache.

Previously noticed by jjj on #19500 with a patch provided. Though I'm tempted to leave this ticket open as it is a separate issue really.

Similar issue with post caches at the end of #19690.

@duck_3 years ago

@duck_3 years ago

comment:2 @ryan3 years ago

Looks good.

comment:3 @duck_3 years ago

  • Owner set to duck_
  • Resolution set to fixed
  • Status changed from new to closed

In [20522]:

Pass full user objects to clean_user_cache(). See #19500, fixes #20460.

Prevents notices when clean_user_cache() is called for a user that has been removed from the database.

comment:4 @duck_3 years ago

In [20523]:

clean_user_cache() after the DELETE query in wp_delete_user(). See #20460.

This is to prevent plugins managing to hook in between the cache cleaning and the actual deletion.

comment:5 @duck_3 years ago

In [20532]:

Don't deprecate cleaning the user cache by ID. See #20460.

comment:6 follow-up: @blueyed3 years ago

This code should take into account that WP_User->get_data_by might return false.
This would lead to a fatal error then with $user->exists() (where $user is a non-object).

comment:7 in reply to: ↑ 6 @duck_3 years ago

Replying to blueyed:

This code should take into account that WP_User->get_data_by might return false.
This would lead to a fatal error then with $user->exists() (where $user is a non-object).

The WP_User::get_data_by() method is not used directly anymore. It was replaced with "new WP_User()" which will always return a WP_User object.

comment:8 @ocean903 years ago

  • Milestone changed from Awaiting Review to 3.4
Note: See TracTickets for help on using tickets.