Make WordPress Core

Opened 16 months ago

Last modified 16 months ago

#57635 new defect (bug)

Check if empty user_nicename in the function wp_insert_user

Reported by: missveronicatv's profile missveronicatv Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 6.1.1
Component: Users Keywords: good-first-bug has-patch
Focuses: Cc:

Description

Check if empty 'user_nicename' in the function 'wp_insert_user' after the 'sanitize_title' and filter 'pre_user_nicename'. Only 'user_nicename' length > 50 is being reported as an error.

With an empty 'user_nicename' in the users WP table the function 'update_user_caches' will fail with an empty key when calling 'wp_cache_add'.

This empty field will give a PHP Warning: "Function WP_Object_Cache::add was called <strong>incorrectly</strong>. Cache key must not be an empty string. Please see ..."

Stack trace when the plugin Ultimate Member is calling 'get_users':

wp-includes/functions.php:5835
    WP_Object_Cache->is_valid_key()
    wp-includes/class-wp-object-cache.php:204
    WP_Object_Cache->add()
    wp-includes/cache.php:44
    wp_cache_add()
    wp-includes/user.php:1864
    update_user_caches()
    wp-includes/pluggable.php:141
    cache_users()
    wp-includes/class-wp-user-query.php:856
    WP_User_Query->query()
    wp-includes/class-wp-user-query.php:79
    WP_User_Query->__construct()
    wp-includes/user.php:763
    get_users()
    wp-content/plugins/ultimate-member/includes/core/um-actions-form.php:874

    um_submit_form_errors_hook_()
    wp-includes/class-wp-hook.php:308
    do_action('um_submit_form_errors_hook_')
    wp-content/plugins/ultimate-member/includes/core/um-actions-form.php:293
    um_submit_form_errors_hook()
    wp-includes/class-wp-hook.php:308
    do_action('um_submit_form_errors_hook')
    wp-content/plugins/ultimate-member/includes/core/class-form.php:569
    um\core\Form->form_init()
    wp-includes/class-wp-hook.php:308
    do_action('template_redirect')
    wp-includes/template-loader.php:13

Stack trace when displaying the backend page WP All Users

    wp-includes/functions.php:5835
    WP_Object_Cache->is_valid_key()
    wp-includes/class-wp-object-cache.php:204
    WP_Object_Cache->add()
    wp-includes/cache.php:44
    wp_cache_add()
    wp-includes/user.php:1864
    update_user_caches()
    wp-includes/pluggable.php:141
    cache_users()
    wp-includes/class-wp-user-query.php:856
    WP_User_Query->query()
    wp-includes/class-wp-user-query.php:79
    WP_User_Query->__construct()
    wp-admin/includes/class-wp-users-list-table.php:140
    WP_Users_List_Table->prepare_items()
    wp-admin/users.php:521

Attachments (2)

57635.patch (1.0 KB) - added by kalpeshh 16 months ago.
Patch for empty nicename check fix
57635.2.patch (678 bytes) - added by rinkalpagdar 8 months ago.
Solution for username validation

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
16 months ago

  • Keywords needs-patch good-first-bug added; changes-requested removed
  • Milestone changed from Awaiting Review to Future Release

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Good catch, looks like we should apply the same logic for user_nicename as we do for user_login:

// user_login must be between 0 and 60 characters.
if ( empty( $user_login ) ) {
	return new WP_Error( 'empty_user_login', __( 'Cannot create a user with an empty login name.' ) );
} elseif ( mb_strlen( $user_login ) > 60 ) {
	return new WP_Error( 'user_login_too_long', __( 'Username may not be longer than 60 characters.' ) );
}

This ticket was mentioned in Slack in #core by mramosmanhaes. View the logs.


16 months ago

#3 @kalpeshh
16 months ago

  • Keywords needs-testing-info added

I could not reproduce this and I can see that empty 'user_nicename' is already handled in wp_insert_user function

	 /*
	 * If a nicename is provided, remove unsafe user characters before using it.
	 * Otherwise build a nicename from the user_login.
	 */
	if ( ! empty( $userdata['user_nicename'] ) ) {
		$user_nicename = sanitize_user( $userdata['user_nicename'], true );
	} else {
		$user_nicename = mb_substr( $user_login, 0, 50 );
	}

To reproduce exception you mentioned, I had to change value of user_nicename to empty manually from database.

@missveronicatv can you please provide steps to reproduce this? I couldn't reproduce with below.

$userdata = array(
    'user_login' =>  'login_name',
    'user_url'   =>  'https://example.com',
    'user_pass'  =>  'password',
    'user_nicename' => ''
);

$user_id = wp_insert_user( $userdata );

#4 @missveronicatv
16 months ago

With $user_login empty you will get $user_nicename empty too

$user_nicename = mb_substr( $user_login, 0, 50 );

#5 @kalpeshh
16 months ago

  • Keywords 2nd-opinion added

@missveronicatv I tried that and still no luck. I got an expected error "Cannot create a user with an empty login name."

Also, if you look at the code, empty user_login case is handled. Can you please share sample code or API call to reproduce this?

        // user_login must be between 0 and 60 characters.
	if ( empty( $user_login ) ) {
		return new WP_Error( 'empty_user_login', __( 'Cannot create a user with an empty login name.' ) );
	} elseif ( mb_strlen( $user_login ) > 60 ) {
		return new WP_Error( 'user_login_too_long', __( 'Username may not be longer than 60 characters.' ) );
	}

#6 @missveronicatv
16 months ago

You are right.
If $user_login is a "dot" after

$sanitized_user_login = sanitize_user( $userdata['user_login'], true );

This "dot" will be removed in

$user_nicename = sanitize_title( $user_nicename );

#7 @missveronicatv
16 months ago

When displaying the WP users table in phpMyAdmin user_login is a "dot" and user_nicename is empty

#8 @kalpeshh
16 months ago

Got it. That's a good catch.

@kalpeshh
16 months ago

Patch for empty nicename check fix

#10 @kalpeshh
16 months ago

  • Keywords has-patch added; needs-patch needs-testing-info 2nd-opinion removed

@SergeyBiryukov do you suggest unit test for this?

This ticket was mentioned in PR #4037 on WordPress/wordpress-develop by kalpeshhiran.


16 months ago
#11

Improvement in nicename empty check and adding sanitized value in empty check to avoid creating user with empty value.

Trac ticket: https://core.trac.wordpress.org/ticket/57635

@rinkalpagdar
8 months ago

Solution for username validation

Note: See TracTickets for help on using tickets.