Make WordPress Core

Opened 3 years ago

Last modified 22 months ago

#20845 new defect (bug)

wp_set_current_user( null, "name") does not work when $current_user is already 0

Reported by: bobbingwide Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.3.2
Component: Users Keywords: has-patch
Focuses: Cc:

Description (last modified by scribu)

According to the documentation for wp_set_current_user() you can

Set $id to null and specify a name if you do not know a user's ID.

This does not work when the current user has already been set to anonymous ( ID = 0 ) since the following test returns true.

if ( isset( $current_user ) && ( $current_user instanceof WP_User ) && ( $id == $current_user->ID ) ) {

The last part of the test should be corrected to
&& ($id === $current_user->ID) )

which will ensure that the test fails when the $id parameter is null, which will allow the rest of the pluggable function to continue and set the current user to the specified $name.

This is similar to #19769.
Note: the change in 3.4 has not fixed this particular problem.

Attachments (1)

20845.patch (579 bytes) - added by barrykooij 23 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 @scribu3 years ago

  • Description modified (diff)

comment:2 @bobbingwide3 years ago

Note: The change in 3.4 was Revision [20410] for #20372

I tried a simple workaround; set to "admin" first.

wp_set_current_user( 1 );
wp_set_current_user( null, "workaround-for-20845" );

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

comment:3 @scribu3 years ago

  • Milestone changed from Awaiting Review to 3.4

comment:4 @nacin3 years ago

  • Milestone changed from 3.4 to Awaiting Review

I believe bobbingwide was simply pointing out that the change in 3.4, [20410], did not fix this problem, and that it is not a regression (it was reported against 3.3.2).

comment:5 @bobbingwide3 years ago

TRUE. I found the problem in 3.3.2 and confirmed that the latest change for 3.4 doesn't address this problem. I suspect that the problem has potentially existed for a long time but was exacerbated by more recent code that sets the current user to 0.

@barrykooij23 months ago

comment:6 @barrykooij23 months ago

I agree on the importance of the difference between 0 and null, I've added a patch with the strict check.

comment:7 @barrykooij23 months ago

  • Keywords has-patch added

comment:8 @markoheijnen23 months ago

Is this something that can be added to 3.7? or should the milestone be set to 3.8?

comment:9 @barrykooij22 months ago

Just talked this over with Nacin, I will write some unit test for this patch ASAP.

Note: See TracTickets for help on using tickets.