Skip to content

Users: Make user email comparison case-insensitive in wp_insert_user()/wp_update_user() - #11697

Open
wppoland wants to merge 3 commits into
WordPress:trunkfrom
wppoland:trac/52976
Open

wppoland wants to merge 3 commits into
WordPress:trunkfrom
wppoland:trac/52976

Conversation

@wppoland

@wppoland wppoland commented May 3, 2026 •

Copy link
Copy Markdown

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

Email addresses are case-insensitive in their domain part (per RFC 5321) and treated as such by virtually all mail providers in the local part too. Previously, comparing user_email with !== could:

  • Reset user_activation_key even when only the case of an email changed
  • Trigger the email-change notification for a case-only "change"

This PR replaces those strict comparisons with strcasecmp().

Tests cover:

  • Case-only email change does not fire send_email_change_email
  • Genuinely different email change still fires the notification
  • Case-only email change does not clear user_activation_key

Complements GH-9196 with proper PHPUnit coverage. Narrower scope than GH-9196 (focused on user.php only); happy to expand to admin/multisite call sites in a follow-up if reviewers prefer one bundled change.

Use of AI Tools

AI assistance: Yes
Tool(s): Antigravity IDE
Model(s): Gemini 2.5 Pro
Used for: Code analysis, conflict resolution, test suite updates, and patch verification. All changes were manually reviewed and tested locally.


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.

@github-actions

github-actions Bot commented May 3, 2026 •

Copy link
Copy Markdown

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props motylanogha, orionaselite.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@GeorgeWebDevCy

Copy link
Copy Markdown

Tested PR #11697 locally against current trunk.

Environment:

  • WordPress develop at 7.1-alpha-src
  • PHP 8.3.31 in the Docker local environment
  • MySQL 9.7 Docker service
  • PHPUnit 9.6.34

Results:

  • Targeted regression tests added in this PR pass:
    • test_case_only_email_change_does_not_trigger_email_change_notification
    • test_real_email_change_triggers_email_change_notification
    • test_case_only_email_change_does_not_clear_user_activation_key
  • Full user test file passes: npm run test:php -- tests/phpunit/tests/user.php
    • 125 tests, 419 assertions, 1 skipped
  • PHPCS on src/wp-includes/user.php and tests/phpunit/tests/user.php reports no errors. It does report existing prepared-SQL warnings in unrelated parts of src/wp-includes/user.php.

The PR behavior looks correct to me: case-only email changes do not trigger the email-change notification or clear the activation key, while genuinely different email changes still trigger the notification.

@wppoland

Copy link
Copy Markdown
Author

@GeorgeWebDevCy thank you for the test report, and sorry it sat unanswered for so long — that's on me, not on the quality of the report. Reproducing the three regression tests, running the full user.php file and checking PHPCS is exactly what this needed, and it's more than most patches at this stage get.

For anyone arriving later, the substance of what was verified on 2026-06-01, against 7.1-alpha-src on PHP 8.3 / MySQL 9.7:

  • test_case_only_email_change_does_not_trigger_email_change_notification
  • test_real_email_change_triggers_email_change_notification
  • test_case_only_email_change_does_not_clear_user_activation_key
  • full tests/phpunit/tests/user.php: 125 tests, 419 assertions, 1 skipped
  • PHPCS clean on src/wp-includes/

I'll carry this over to #52976 so the testing is recorded on the ticket rather than only here, since that's where a committer will look.

@GeorgeWebDevCy

GeorgeWebDevCy commented Jul 29, 2026 •

Copy link
Copy Markdown

@GeorgeWebDevCy thank you for the test report, and sorry it sat unanswered for so long — that's on me, not on the quality of the report. Reproducing the three regression tests, running the full user.php file and checking PHPCS is exactly what this needed, and it's more than most patches at this stage get.

For anyone arriving later, the substance of what was verified on 2026-06-01, against 7.1-alpha-src on PHP 8.3 / MySQL 9.7:

  • test_case_only_email_change_does_not_trigger_email_change_notification
  • test_real_email_change_triggers_email_change_notification
  • test_case_only_email_change_does_not_clear_user_activation_key
  • full tests/phpunit/tests/user.php: 125 tests, 419 assertions, 1 skipped
  • PHPCS clean on src/wp-includes/

I'll carry this over to #52976 so the testing is recorded on the ticket rather than only here, since that's where a committer will look.

@wppoland don't worry about it I understand. I have only recently started contributing after being at WCEU 2026 contributor day. I am teaching myself how to contribute. Not sure if I am following SOP. I am busy with freelancing but when I have some free time I just jump on and look for something to work on so don't worry about it :)

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The 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

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

…()` and `wp_update_user()`.

Email addresses are case-insensitive in their domain part (per RFC 5321) and treated as such by virtually all mail providers in the local part too. Previously, comparing `user_email` with `!==` could:

* Reset `user_activation_key` even when only the case of an email changed.
* Trigger the email-change notification for a case-only "change".

This replaces those strict comparisons with `strcasecmp()` and adds tests for both code paths, including a regression test that case-only changes do not invalidate an existing password reset key.

Complements WordPressGH-9196 with proper PHPUnit coverage.

See #52976.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants