WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 15 months ago

#16788 new defect (bug)

Ampersands in e-mail address become invalid

Reported by: jfarthing84 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 3.0.5
Component: Users Keywords: dev-feedback has-patch
Focuses: Cc:

Description

When an e-mail address contains an ampersand, WordPress improperly escapes the ampersand invalidating the e-mail address.

Example: h&f@… becomes h&amp@…

First of all, the proper HTML entity for "&" is &. Where did the extra amp come from?

Also, an ampersand is a valid character in an e-mail address and should not be escaped. Escaping it could be a completely different e-mail address.

I have not dug into the code to find out where this is happening but I'd assume in sanitize_email().

Attachments (1)

16788.diff (1.3 KB) - added by dkotter 15 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 @nacin4 years ago

Where is this occurring?

comment:2 @jfarthing844 years ago

Upon registration.

comment:3 @ericmann4 years ago

I can verify this bug exists.

Testing on WordPress 3.2-bleeding.

  1. Attempted to register user with email t&est@eamann.com
  2. User was registered with no visible errors
  3. Registered user is listed with email t&ampest@eamann.com in WordPress admin
  4. Mailto mousover lists mailto:t&ampest@eamann.com as the link
  5. Editing the user lists t&est@eamann.com as the email address <- This is correct!
  6. Updating the user with t&est@eamann.com doesn't change any of the above behavior

comment:4 @ericmann4 years ago

Also, the new user email sent to the admin states the following:

New user registration on your site WordPress Test Site:

Username: thisisatest

E-mail: t&amp;est@eamann.com

comment:5 @garyc404 years ago

Network Users table don't have this issue. This is because in class-wp-users-list-table.php, we sanitize_user_object() before outputting user details, while in class-wp-ms-users-list-table.php, we don't. That being said, I still think it's appropriate to sanitize user object in Network Users table as well before printing out.

When user object is sanitized, user_email filter is applied on the user's email. As a result, the email address is passed through sanitize_email(), resulting in t&amp;est@eamann.com. Now if you're in the admin panel, wp_filter_kses() will further mutilate the email address, resulting in t&amp;ampest@eamann.com. See this code.

comment:6 @garyc404 years ago

Actually, upon further investigation, sanitize_email() seems to be innocent.

Here's a more accurate recap of the process:

  • User data is sanitized before being saved to the database. pre_user_email filter is applied to user_email, which passes the email to wp_filter_kses(). Here, the email address is mutilated the first time. (t&est@test.com becomes t&amp;est@test.com)
  • When the user info is displayed in wp-admin/users.php, sanitize_user_object() is called, which in turn applies user_email filter to user_email. This results in wp_kses being called when is_admin(), thus, double-escape the email address (t&amp;est@test.com becomes t&amp;ampest@test.com).

comment:7 @SergeyBiryukov4 years ago

  • Keywords needs-patch added

comment:9 @jkudish3 years ago

  • Cc joachim.kudish@… added

comment:10 @anmari17 months ago

Related: #11311 and #24354

@dkotter15 months ago

comment:11 @dkotter15 months ago

  • Keywords has-patch added; needs-patch removed

So that attached patch will stop ampersands from being converted in emails. In my testing, emails are then shown correctly on the front end, no encoding as mentioned above.

But I know there's lots of other suggestions we check for on emails #21537, as well as the other tickets mentioned above dealing with ampersands being encoded in other places when we might not want them to. So not sure if this patch is even worth it, but decided to throw it out there.

Note: See TracTickets for help on using tickets.