Opened 6 years ago
Last modified 3 weeks ago
#45714 new defect (bug)
Allow all valid email formats when editing accounts in the dashboard
Reported by: | chrisl27 | Owned by: | |
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Users | Keywords: | needs-patch |
Focuses: | administration | Cc: |
Description
When updating an email address in the dashboard wp-admin/includes/user.php:80 uses sanitize_text_field which strips out a substring like "%ed" even though that is valid in the local part of an email address. Eg, the valid email address "user%edition@…" is saved as "userition@…".
This ticket requests to using sanitize_email instead, similar to the REST API that checks using is_email.
(A related, fixed issue is in #18039 which also recommended using sanitize_email)
Attachments (1)
Change History (24)
#5
@
11 months ago
- Keywords needs-refresh added
I tried to apply the patch, it needs to be updated, as it does not apply at all. Error messages says:
error: wp-admin/includes/user.php: No such file or directory error: phpunit/tests/user.php: No such file or directory
This ticket was mentioned in PR #5708 on WordPress/wordpress-develop by @kebbet.
11 months ago
#6
- Keywords needs-refresh removed
Porting original patch to GitHub.
https://core.trac.wordpress.org/ticket/45714
assertInternalType
is used in the provided test which is an unidentified method
.
#8
@
11 months ago
- Keywords needs-refresh removed
PR is refreshed and assertIsInt
is used instead of assertInternalType
. Tests seems to pass at GitHub now. But please check that the tests are relevant and correct.
This ticket was mentioned in Slack in #core by rajinsharwar. View the logs.
8 months ago
#10
@
8 months ago
Thanks for your patch @kebbet. If we can get some test reports, and some eyes on this, hopefully, we can get this committed within the cycle. If not, we will have to punt this for 6.6 and check if we can get some early eyes. Keep this in the milestone for now.
#11
@
8 months ago
Test Report
Testing Environment
WordPress: 6.5-beta1
PHP: 8.1.23
Web Server: nginx
Database: MySQL 8.0.16
Browser: Safari Version 16.2 (18614.3.7.1.5)
OS: macOS Ventura 13.1
Theme: Twenty Twenty-Four 1.0
MU Plugins: None activated
Verdict
%ed is still being removed from user id as described in the original ticket.
The scenario was recreated for multiple special characters. Using "&" also behaves unusually, turning the email from "user&id@…" to "user&id@…".
See attachments for "&" issue
#12
follow-up:
↓ 13
@
8 months ago
- Keywords needs-refresh added
Test report indicates that the issue is not fixed, unless I'm missing something..?
Unit test can be improved to use a data provider.
#13
in reply to:
↑ 12
@
8 months ago
Replying to swissspidy:
Test report indicates that the issue is not fixed, unless I'm missing something..?
Unit test can be improved to use a data provider.
Yes. The issue is not resolved. If you'd suggest a data provider, I could redo the unit-test for improvement.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
8 months ago
#15
@
8 months ago
- Milestone changed from 6.5 to 6.6
As per today's bug scrub:
Since the PR still needs some work, let's move this ticket to milestone 6.6 to give it more time.
#17
@
5 months ago
Test Report
Testing Environment
WordPress: 6.5.3
PHP: 8.1.23
Web Server: 7.4.3-4
Database: MySQL 8.0.36
Browser: Google Chrome Version 124.0.6367.60 (Official Build) (64-bit)
OS: Linux 5.15.0-107-generic x86_64
Theme: Twenty Eleven 4.5
MU Plugins: None activated
The scenario was recreated for multiple special characters. Using "%$" also behaves unusually, turning the email from "sumit%$@gmail.com" to "sumit@…".
#18
@
4 months ago
Test Report
Description
I've Apply the both Patch 45714.patch and PR PR5708 anyone doesn't solve the issue check attached.
Environment
- WordPress: 6.6-alpha-57778-src
- PHP: 8.3.7
- Server: nginx/1.25.4
- Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.7)
- Browser: Firefox 126.0
- OS: macOS
- Theme: Twenty Fifteen 3.7
- MU Plugins: None activated
- Plugins:
- FakerPress 0.6.6
- Test Reports 1.1.0
Actual Results
- ✅ Issue resolved with patch.
Screenshots
Here is my view
This ticket was mentioned in Slack in #core by oglekler. View the logs.
4 months ago
#20
@
4 months ago
- Milestone changed from 6.6 to 6.7
This ticket was discussed during bug scrub. According to the test report, the issue is not resolved with existing patches, and additional work is needed. I am moving this ticket to the next milestone.
Let's try to solve it in the beginning of the 6.7 to have more time for testing.
Thanks!
add props to @hellofromTonya
This ticket was mentioned in PR #6956 on WordPress/wordpress-develop by @rajinsharwar.
3 months ago
#21
- Keywords needs-refresh removed
Removing the wp_filter_kses filter when inserting user.
Trac ticket: https://core.trac.wordpress.org/ticket/45714
#22
@
3 months ago
- Keywords changes-requested removed
I removed the wp_filter_kses, as that filter was converting the "&" to amp for the emails. As the sanitize_email filter is already present, it should remove the non-email elements from the email.
#23
@
3 weeks ago
- Keywords needs-patch added; has-patch has-unit-tests needs-testing removed
Test Report
Description
Issue has not been resold after patch in wp 6.7
Environment
- WordPress: 6.7-alpha-59082
- PHP: 8.2.23
- Server: Apache/2.4.62 (Debian)
- Database: mysqli (Server: 11.5.2-MariaDB-ubu2404 / Client: mysqlnd 8.2.23)
- Browser: Safari 17.6 (macOS)
- Theme: Twenty Twenty-Four 1.2
- MU-Plugins: None activated
- Plugins:
- WordPress Beta Tester 3.5.6
Screenshot
https://drive.google.com/file/d/1gHymyg-0xX_xN7uTdpS6j_VnZLSvpBXW/view?usp=sharing
Patch