Users: Validate the whitespace-normalized username in add_user()#12630
Users: Validate the whitespace-normalized username in add_user()#12630irozum wants to merge 2 commits into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR updates user creation via add_user()/edit_user() in wp-admin to validate usernames against the same whitespace normalization that is applied when storing the username, avoiding false “illegal characters” rejections caused solely by leading/trailing or repeated internal whitespace.
Changes:
- Normalize whitespace (trim + collapse) before calling
validate_username()during new-user creation inedit_user(). - Add PHPUnit coverage to confirm usernames with normalized whitespace are accepted, while usernames with illegal characters are still rejected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/wp-admin/includes/user.php |
Normalizes whitespace in $_POST['user_login'] before validating, matching stored normalization behavior. |
tests/phpunit/tests/admin/includes/user/addUser.php |
Adds tests for whitespace-tolerant username validation and continued rejection of illegal characters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @group admin | ||
| * @group user | ||
| */ | ||
| class Tests_Admin_Includes_User_AddUser extends WP_UnitTestCase { |
| public function set_up() { | ||
| parent::set_up(); | ||
|
|
||
| $admin_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); | ||
| wp_set_current_user( $admin_id ); | ||
| } |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Description
edit_user()(used byadd_user()when creating a new user from wp-admin) stores the sanitized username:but validated the raw, un-unslashed
$_POST['user_login']value instead:validate_username()does a strict equality check againstsanitize_user(), andsanitize_user()trims leading/trailing whitespace and collapses repeated internal whitespace. Since the raw input still has that whitespace, the check fails even though the same value would be stored successfully once sanitized — a common false rejection when a username is pasted with a trailing space.I initially tried validating the fully-sanitized
$user->user_loginvalue directly, but that turned out to be too broad: sincewp_insert_user()re-sanitizes on insert regardless, the fully-sanitized value trivially equals itself, so it silently stopped rejecting any illegal characters too (e.g.bob@#&99would silently becomebob@99instead of erroring). That's a bigger behavior change than the ticket asks for.This PR instead normalizes only the whitespace (trim + collapse, matching what
sanitize_user()does) before validating, so:Trac ticket: https://core.trac.wordpress.org/ticket/65672
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5
Used for: Implementation, tests, and PR description. Reviewed by irozum.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.