Make WordPress Core

Opened 3 years ago

Closed 7 months ago

Last modified 7 months ago

#57635 closed defect (bug) (fixed)

Check if empty user_nicename in the function wp_insert_user

Reported by: missveronicatv's profile missveronicatv Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.9 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 3 years ago.
Patch for empty nicename check fix
57635.2.patch (678 bytes) - added by rinkalpagdar 2 years ago.
Solution for username validation

Download all attachments as: .zip

Change History (21)

#1 @SergeyBiryukov
3 years 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.


3 years ago

#3 @kalpeshh
3 years 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
3 years ago

With $user_login empty you will get $user_nicename empty too

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

#5 @kalpeshh
3 years 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
3 years 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
3 years ago

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

#8 @kalpeshh
3 years ago

Got it. That's a good catch.

@kalpeshh
3 years ago

Patch for empty nicename check fix

#10 @kalpeshh
3 years 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.


3 years 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
2 years ago

Solution for username validation

This ticket was mentioned in PR #6775 on WordPress/wordpress-develop by @thehercules.


19 months ago
#12

Fixes Ticket #57635

Check if 'user_nicename' is empty in the function 'wp_insert_user' after the 'sanitize_title' and filter 'pre_user_nicename'.

@thehercules commented on PR #6775:


19 months ago
#13

Hey @mukeshpanchal27, Can you review this PR.

#14 @thehercules
19 months ago

@missveronicatv I have created a patch for this in this PR https://github.com/WordPress/wordpress-develop/pull/6775 could you check. And suggest if there are any changes to be made.

#15 @missveronicatv
19 months ago

@thehercules This patch looks OK

#16 @rayhatron
7 months ago

Test this at WordCamp Europe 2025 using WordPress playground and recorded 2 videos of the tests.

Before the fix we can observe that a user is successfully created with an empty user_nicename which results in the author page link leading to a 404. https://www.loom.com/share/f23eabfc48974f04919c9ed827ad5126?sid=9f58298e-ae09-4e7a-bba4-3aa901726347

After the fix we can observe that if a user's user_nicename would be empty then no user is created and an appropriate error is returned. https://www.loom.com/share/1d1d9881694e453b870a056bed43ab6c?sid=1c7f0090-8dab-48e8-9a46-0772e096ea83

Based on these results and a review of the code in the pull request, I've approved the pull request.

Thanks @thehercules for the patch !!

cc: @peterwilsoncc as discussed at contributor day, please review and continue with next steps if all is well.

Version 0, edited 7 months ago by rayhatron (next)

#17 @peterwilsoncc
7 months ago

  • Milestone changed from Future Release to 6.9

#18 @peterwilsoncc
7 months ago

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

In 60288:

Users: Prevent creating of empty usernames after sanitization.

Introduces a check in wp_insert_user() to ensure the username doesn't have a length of zero after sanitization removes invalid characters.

Props kalpeshh, missveronicatv, rayhatron, rinkalpagdar, sergeybiryukov, thehercules.
Fixes #57635.

@mukesh27 commented on PR #6775:


7 months ago
#19

If possible add unit test coverage for changes.

Note: See TracTickets for help on using tickets.