Make WordPress Core

Opened 13 years ago

Closed 10 years ago

Last modified 8 years ago

#18039 closed defect (bug) (fixed)

Allow apostrophes in email addresses when accounts are added via Dashboard

Reported by: boonebgorges's profile boonebgorges Owned by: boonebgorges's profile 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 13 years ago.
18039.02.patch (4.9 KB) - added by boonebgorges 11 years ago.
18039.03.patch (5.4 KB) - added by boonebgorges 11 years ago.
18039.04.patch (4.0 KB) - added by boonebgorges 11 years ago.
18039.04.2.patch (4.0 KB) - added by boonebgorges 10 years ago.
18039.05.patch (4.0 KB) - added by boonebgorges 10 years ago.

Download all attachments as: .zip

Change History (26)

@boonebgorges
13 years ago

#2 @holizz
13 years ago

  • Cc tom@… added

#3 @beaulebens
12 years ago

  • Cc beau@… added

#4 @harrym
12 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
11 years ago

  • Cc pm@… added

#6 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.7

#7 @wonderboymusic
11 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
11 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
11 years ago

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

#10 @nacin
11 years ago

  • Milestone changed from 3.7 to 3.8

#11 @nacin
11 years ago

#18658 was marked as a duplicate.

#12 follow-up: @nacin
11 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
11 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
10 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
10 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
10 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
10 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.


10 years ago

#19 @boonebgorges
10 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
8 years 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.