WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 2 years ago

Last modified 4 months ago

#18039 closed defect (bug) (fixed)

Allow apostrophes in email addresses when accounts are added via Dashboard

Reported by: boonebgorges Owned by: boonebgorges
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.2
Component: Users Keywords: has-patch needs-testing
Focuses: Cc:

Description

See #4616.

Currently it's not possible to add a user with an apostrophe in his email address in any of the following ways:

  • Dashboard > Add User (non-MS)
  • Dashboard > Add User > Add Existing User (MS)
  • Dashboard > Add User > Add New User (MS)

With existing users, you get a 'user not found' error. Otherwise you get an error about invalid email addresses.

This is inconsistent with WP's basic email address behavior, which allows apostrophes in email addresses.

For the most part, the problem is simply that the value of 'email' in the $_POST data must be stripslashed. In a few cases, adjustments had to be made to the way that email addresses are escaped, to allow for the ' character (see esc_email()).

See attached patch.

Attachments (6)

18039.diff (3.9 KB) - added by boonebgorges 6 years ago.
18039.02.patch (4.9 KB) - added by boonebgorges 3 years ago.
18039.03.patch (5.4 KB) - added by boonebgorges 3 years ago.
18039.04.patch (4.0 KB) - added by boonebgorges 3 years ago.
18039.04.2.patch (4.0 KB) - added by boonebgorges 2 years ago.
18039.05.patch (4.0 KB) - added by boonebgorges 2 years ago.

Download all attachments as: .zip

Change History (26)

@boonebgorges
6 years ago

#2 @holizz
5 years ago

  • Cc tom@… added

#3 @beaulebens
4 years ago

  • Cc beau@… added

#4 @harrym
4 years ago

  • Cc mail@… added
  • Keywords 2nd-opinion added
  • Severity changed from minor to normal

So - this has been open for 21 months. It's a real bug. It's not too hard to fix. There's a patch here.

Please can this be reviewed? We run into this issue often, and it's become a real irritation.

#5 @pmaiorana
4 years ago

  • Cc pm@… added

#6 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 3.7

#7 @wonderboymusic
4 years ago

  • Keywords needs-unit-tests added; dev-feedback 2nd-opinion removed

Write a unit test that fails with current behavior, apply the patch, confirm it works - then this can probably go in.

#8 @boonebgorges
3 years ago

  • Keywords needs-testing added; needs-unit-tests removed

18039.02 is a refresh that does the following:

  • refreshes up to trunk@25631. Much of the logic has been centralized since the original patch, so there's less to do. Mostly stripslashes().
  • refreshes for develop.svn.wordpress.org repo format
  • adds a test for the new esc_email()

wonderboymusic - The real fixes in this patch come from the way that $_POST data is handled in the procedural logic of user-new.php etc. It's not easy to write tests for this sort of thing.

#9 @boonebgorges
3 years ago

Oops, forgot the new tests file. See 18039.03.patch

#10 @nacin
3 years ago

  • Milestone changed from 3.7 to 3.8

#11 @nacin
3 years ago

#18658 was marked as a duplicate.

#12 follow-up: @nacin
3 years ago

  • Keywords 3.9-early added
  • Milestone changed from 3.8 to Future Release

An esc_* function should be used for output. (esc_url_raw() is an exception; it was done to as not to deviate much from esc_url()). We already have sanitize_email() — would that work here?

#13 in reply to: ↑ 12 @boonebgorges
3 years ago

Replying to nacin:

An esc_* function should be used for output. (esc_url_raw() is an exception; it was done to as not to deviate much from esc_url()). We already have sanitize_email() — would that work here?

Yup, good call. I think I'd introduced an esc_* function because esc_html() was already in use here. Bonus: sanitize_email() already supports apostrophes.

18039.04.patch adds the stripslashes(), swaps the esc_html() for sanitize_email(), and adds a passing apostrophe unit test for sanitize_email().

#14 follow-up: @tomdxw
3 years ago

What needs to be done apart from merge boonebgorges' patch? Is there anything somebody without commit rights like me can do to help?

#15 in reply to: ↑ 14 @rmccue
3 years ago

Observation: this should use wp_unslash instead of stripslashes/stripslashes_deep.

Apart from that, I like.

(As an indication of how old this ticket is, "This is only boonebgorges’s third ticket!" :) )

#16 @lancewillett
3 years ago

Wanted to add that we're seeing a few related bug reports for this on WordPress.com. Would love to get this in so people with single quotes in their email addresses can register with that email address.

#17 @boonebgorges
2 years ago

  • Keywords 3.9-early removed
  • Milestone changed from Future Release to 4.1
  • Owner set to boonebgorges
  • Status changed from new to accepted

18039.05.patch uses wp_unslash() instead of stripslashes(_deep)?, and contains lots of love and kisses, from my heart to the WordPress Community.

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov. View the logs.


2 years ago

#19 @boonebgorges
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 29966:

Allow apostrophes in email addresses when adding users via the Dashboard.

Email addresses entered in a number of interfaces were not being stripslashed
properly, with the result that the emails were not being recognized as valid.

Fixes #18039.

#20 @boonebgorges
4 months ago

In 39544:

Allow apostrophes in email address during wp-login.php registration.

See #18039 for a related fix when creating users via the Dashboard.

Props tomdxw.
Fixes #34483.

Note: See TracTickets for help on using tickets.