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

[bobbingwide]

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.

[bobbingwide]

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" );

[nacin]

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).

[bobbingwide]

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.

[barrykooij]

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

[markoheijnen]

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

[barrykooij]

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

[bobbingwide]

Refreshed version of the patch

[bobbingwide]

Refreshed version with a unit test

[bobbingwide]

I've added a new file which contains a test setting the user by name with ID=null.
and a test demonstrating that name is trumped by ID. e.g. wp_set_current_user( "admin", "bobbingwide" ); will return the "admin" user.

There seems little point in adding tests for setting it by a numeric ID, since this is already implicit in many other tests.

[bobbingwide]

Added the ID trumps name test

[boonebgorges]

In 34945:

Add tests for current behavior of wp_set_current_user().

See #20845.

[boonebgorges]

Thanks for the patch, bobbingwide. The strict check you've suggested will force a user switch in the following case:

wp_set_current_user( 3 );
wp_set_current_user( '3' );

I don't think we want that. So we'll be a bit more precise and check for $id === null, which is how the $id bypass is supposed to work anyway.

[boonebgorges]

In 34947:

Allow a null id to do a name lookup in wp_set_current_user().

Previously, the name fallback was failing in the case where the current user
was 0, due to a loose comparison between 0 (the current user) and null (the
value that is used to trigger the name fallback).

Props bobbingwide.
Fixes #20845.

[bobbingwide]

Replying to boonebgorges:

Thanks for the patch, bobbingwide. The strict check you've suggested will force a user switch in the following case:

wp_set_current_user( 3 );
wp_set_current_user( '3' );

I don't think we want that. So we'll be a bit more precise and check for $id === null, which is how the $id bypass is supposed to work anyway.

Good spot. Will there need to be a test where?

ID=3 user_login="three"
ID=4 user_login="3"

[boonebgorges]

Replying to bobbingwide:

Good spot. Will there need to be a test where?

ID=3 user_login="three"
ID=4 user_login="3"

I don't think we need a separate case for that, since $id and $name are separate parameters. The only way they can be mixed up is by developer error, not by internal type mismatches.

[barrykooij]

@boonebgorges is it correct I didn't get props here?

[boonebgorges]

barrykooij - Dang it, you are correct. Many apologies. You'll be in the 4.4 credits for #22212.

[bobbingwide]

Replying to boonebgorges:

Replying to bobbingwide:

Good spot. Will there need to be a test where?

ID=3 user_login="three"
ID=4 user_login="3"

I don't think we need a separate case for that, since $id and $name are separate parameters. The only way they can be mixed up is by developer error, not by internal type mismatches.

For this function it's OK. It's WP_User::__construct() that could do with the test.