register_new_user() can't handle WP_Error result from wp_create_user()

[coffee2code]

register_new_user() in wp-login.php does not properly handle a WP_Error object, which is the potential return value from its call to wp_create_user().

wp_create_user() is basically a pass-through to wp_insert_user(). As a result of [12778], the commit of a patch that was part of #11644 (the MU-merge ticket), new checks were added to wp_insert_user() which changed the potential behavior of the function to return a WP_Error object. register_new_user() can thus fail with a FATAL error:

Notice: Object of class WP_Error could not be converted to int in /Users/scott/Sites/wp30.dev/wp-includes/functions.php on line 3150

Catchable fatal error: Object of class WP_Error could not be converted to string in /Users/scott/Sites/wp30.dev/wp-includes/formatting.php on line 2782

Currently it is not likely the above situation would be tripped. But this is only because register_new_user() and wp_insert_user() perform almost identical error checks (the former does more, actually). However, the errors generated in register_new_user() can all be suppressed by plugins via the 'registration_errors' hook. However, some those same error checks will then be performed in wp_insert_user() and thus return a WP_Error (without a way to suppress those).

Related issues aside, the fact of the matter is that wp_create_user() can return a WP_Error object and register_new_user() can't handle it. The fix is simple and attached.

[coffee2code]

Patch

[coffee2code]

Also see the related #14308 (and my first comment there). Any consensus made there might relate to whether this particular issue becomes moot or not.

[coffee2code]

Related #15001.

[mdawaffe]

Looks like wp_insert_user() could in theory still return 0, so the check should be for !$user_id || is_wp_error( $user_id ).

[coffee2code]

@mdawaffe is correct, though it looks to be pretty rare that a 0 would be returned by wp_insert_user(). Updated patch has been attached (14290.2.diff), which refreshes original patch against latest trunk and accounts for $user_id potentially being 0 or a WP_Error. But to be clear, wp_insert_user() is meant to return a WP_Error if there is an error and register_new_user() can't handle it without this patch.

[coffee2code]

Updated patch that accounts for potential 0 return value

[coffee2code]

Refreshed patch to apply cleanly as of r25172.

[coffee2code]

Simple and obvious fix for 3.7? Especially given #15001.

[nacin]

In 25174:

Check for a WP_Error return from wp_create_user() in register_new_user().

props coffee2code.
fixes #14290.

[nacin]

This seems like it could be unit testable?