Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#20372 closed enhancement (fixed)

WP_User::exists()

Reported by: markjaquith's profile markjaquith Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch commit
Focuses: Cc:

Description

Would be good to have a canonical way of testing that a WP_User object refers to a real user.

Attachments (6)

20372.diff (586 bytes) - added by ryan 13 years ago.
20372.2.diff (5.3 KB) - added by ryan 13 years ago.
Use it in a few places
20372.3.diff (2.0 KB) - added by ryan 13 years ago.
Sanity check current_user global in get_currentuserinfo()
20372-unit.diff (1.7 KB) - added by ryan 13 years ago.
Unit tests for get and set current
20372.4.diff (1.9 KB) - added by ryan 13 years ago.
Tweaked comment
20372.xmlrpc-caps.diff (866 bytes) - added by duck_ 13 years ago.

Download all attachments as: .zip

Change History (21)

@ryan
13 years ago

#1 @nacin
13 years ago

  • Component changed from General to Users
  • Keywords has-patch commit added
  • Milestone changed from Awaiting Review to 3.4

WP_Theme has the same method.

@ryan
13 years ago

Use it in a few places

#2 @ryan
13 years ago

In [20378]:

Introduce WP_User::exists(). see #20372

#3 follow-up: @nacin
13 years ago

Not sure what's going on, but seeing this on WP.org:

Call to a member function exists() on a non-object
File: wp-includes/capabilities.php
Line: 1371
Source: http://cn.wordpress.org/xmlrpc.php

#4 in reply to: ↑ 3 @duck_
13 years ago

Replying to nacin:

Not sure what's going on, but seeing this on WP.org:

Call to a member function exists() on a non-object
File: wp-includes/capabilities.php
Line: 1371
Source: http://cn.wordpress.org/xmlrpc.php

This happens on Multisite installations. ms_site_check() is called in wp-settings.php and it tries to use is_super_admin() which gets NULL from wp_get_current_user() since this is an XML-RPC request.

#5 @soulseekah
13 years ago

Fatal error: Call to a member function exists() on a non-object in /home/www/wordpress.lo/trunk/wp-includes/capabilities.php on line 1371

Call Stack:
    0.0004     140172   1. {main}() /home/www/wordpress.lo/trunk/xmlrpc.php:0
    0.0008     148492   2. include('/home/www/wordpress.lo/trunk/wp-load.php') /home/www/wordpress.lo/trunk/xmlrpc.php:29
    0.0012     155948   3. require_once('/home/www/wordpress.lo/trunk/wp-config.php') /home/www/wordpress.lo/trunk/wp-load.php:29
    0.0024     199076   4. require_once('/home/www/wordpress.lo/trunk/wp-settings.php') /home/www/wordpress.lo/trunk/wp-config.php:101
    0.3641    8247928   5. ms_site_check() /home/www/wordpress.lo/trunk/wp-settings.php:308
    0.3641    8248084   6. is_super_admin() /home/www/wordpress.lo/trunk/wp-includes/ms-load.php:80
Last edited 13 years ago by soulseekah (previous) (diff)

#6 follow-up: @nacin
13 years ago

I looked at [3430] and wp_xmlrpc_server::login(). I feel like get_currentuserinfo() should call wp_set_current_user(0) in the XMLRPC_REQUEST branch. That would mean that wp_get_current_user() would always return a WP_User object, which it does in literally every other situation.

If wp_get_current_user() returns a non-existent user before login() runs in XML-RPC, I think that's just fine. $current_user should never be null after calling get_currentuserinfo().

I am going to commit this to avoid seeing the fatal error in trunk, but open for discussion for what the best fix would be.

#7 follow-up: @nacin
13 years ago

In [20402]:

wp_set_current_user(0) for XMLRPC_REQUEST in get_currentuserinfo(). Ensures that wp_get_current_user() always returns a WP_User object. see #20372.

#8 in reply to: ↑ 6 @duck_
13 years ago

Replying to nacin:

I looked at [3430] and wp_xmlrpc_server::login(). I feel like get_currentuserinfo() should call wp_set_current_user(0) in the XMLRPC_REQUEST branch. That would mean that wp_get_current_user() would always return a WP_User object, which it does in literally every other situation.

If wp_get_current_user() returns a non-existent user before login() runs in XML-RPC, I think that's just fine. $current_user should never be null after calling get_currentuserinfo().

I was thinking along similar lines. Maybe even explicit wp_set_current_user(0) in WP::init() so we can be sure that the $current_user global is always a WP_User object.

Looking at the code it would require that get_currentuserinto() switches to using the exists() method too.

@ryan
13 years ago

Sanity check current_user global in get_currentuserinfo()

@ryan
13 years ago

Unit tests for get and set current

@ryan
13 years ago

Tweaked comment

#10 @nacin
13 years ago

20372.4.diff looks good. Should prevent a lot of issues. I like it, a lot.

#11 @ryan
13 years ago

In [20410]:

When fetching the user in get_currentuserinfo(), make sure it is a valid WP_User object. If it is stdClass, upgrade it to WP_User. If it is WP_Error, an int, or anything else, set the current user to ID 0.

In wp_set_current_user(), return the current user global only if it is a WP_User object. If it is not, fall through and go about setting it up properly.

Formatting cleanups for both functions.

see #20372

#12 @scribu
13 years ago

  • Cc scribu added

#13 in reply to: ↑ 7 @duck_
13 years ago

Replying to nacin:

In [20402]:

wp_set_current_user(0) for XMLRPC_REQUEST in get_currentuserinfo(). Ensures that wp_get_current_user() always returns a WP_User object. see #20372.

As reported by koke in IRC this breaks caps checks during XML-RPC requests. This is because the call to wp_get_current_user() in current_user_can() makes get_currentuserinfo() override the user set by wp_xmlrpc_server::login().

A solution would be to move the XMLRPC_REQUEST block to come after the check for a non-empty $current_user global, see attached patch.

#14 @duck_
13 years ago

In [20424]:

Don't override the $current_user global in get_currentuserinfo() on an XML-RPC request
if it's non-empty. Fixes capabilities checks for XML-RPC requests. See #20372.

#15 @nacin
13 years ago

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

Think we're good here?

Note: See TracTickets for help on using tickets.