WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 4 months ago

#18039 new defect (bug)

Allow apostrophes in email addresses when accounts are added via Dashboard

Reported by: boonebgorges Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2
Component: Users Keywords: has-patch needs-testing 3.9-early
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 (4)

18039.diff (3.9 KB) - added by boonebgorges 3 years ago.
18039.02.patch (4.9 KB) - added by boonebgorges 7 months ago.
18039.03.patch (5.4 KB) - added by boonebgorges 7 months ago.
18039.04.patch (4.0 KB) - added by boonebgorges 4 months ago.

Download all attachments as: .zip

Change History (17)

boonebgorges3 years ago

comment:2 holizz3 years ago

  • Cc tom@… added

comment:3 beaulebens15 months ago

  • Cc beau@… added

comment:4 harrym13 months 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.

comment:5 pmaiorana8 months ago

  • Cc pm@… added

comment:6 SergeyBiryukov8 months ago

  • Milestone changed from Awaiting Review to 3.7

comment:7 wonderboymusic7 months 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.

comment:8 boonebgorges7 months 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.

boonebgorges7 months ago

boonebgorges7 months ago

comment:9 boonebgorges7 months ago

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

comment:10 nacin7 months ago

  • Milestone changed from 3.7 to 3.8

comment:11 nacin7 months ago

#18658 was marked as a duplicate.

comment:12 follow-up: nacin5 months 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?

comment:13 in reply to: ↑ 12 boonebgorges4 months 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().

boonebgorges4 months ago

Note: See TracTickets for help on using tickets.