Make WordPress Core

Opened 4 years ago

Closed 15 months ago

Last modified 15 months ago

#57635 closed defect (bug) (fixed)

Check if empty user_nicename in the function wp_insert_user

Reported by: missveronicatv Owned by: peterwilsoncc
Priority: normal Milestone: 6.9
Component: Users Version: 6.1.1
Severity: normal Keywords: good-first-bug has-patch
Cc: Focuses:

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 4 years ago.
Patch for empty nicename check fix
57635.2.patch (678 bytes ) - added by rinkalpagdar 3 years ago.
Solution for username validation

Download all attachments as: .zip

Change History (21)

#1 @SergeyBiryukov
4 years ago

  • Keywords needs-patch good-first-bug added; changes-requested removed
  • Milestone Awaiting ReviewFuture 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.


4 years ago

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

With $user_login empty you will get $user_nicename empty too

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

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

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

#8 @kalpeshh
4 years ago

Got it. That's a good catch.

@kalpeshh
4 years ago

Patch for empty nicename check fix

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


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

Solution for username validation

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


2 years 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:


2 years ago
#13

Hey @mukeshpanchal27, Can you review this PR.

#14 @thehercules
2 years 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
2 years ago

@thehercules This patch looks OK

#16 @rayhatron
15 months ago

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

Last edited 15 months ago by rayhatron (previous) (diff)

#17 @peterwilsoncc
15 months ago

  • Milestone Future Release6.9

#18 @peterwilsoncc
15 months ago

  • Owner set to peterwilsoncc
  • Resolutionfixed
  • Status newclosed

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:


15 months ago
#19

If possible add unit test coverage for changes.

Note: See TracTickets for help on using tickets.