Opened 11 years ago
Last modified 6 years ago
#26805 reopened defect (bug)
Email with Apostrophe May Not Update in Multisite
Reported by: | contrid | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Users | Keywords: | has-patch |
Focuses: | multisite | Cc: |
Description
From what I understand, an apostrophe is allowed in an email address before the @ sign.
The is_email function does not validate this and fails, returning false if there is an apostrophe
Attachments (3)
Change History (18)
#3
@
10 years ago
We are running into this problem as well. The issue here is that the email is getting passed with a backslash to escape the single quote. The email getting passed is j.d.o\'farrell@… which is essentially
is_email('j.d.o\\\'farrell@example.com');
which fails.
#4
follow-up:
↓ 5
@
10 years ago
Related: #18039.
The issue here is that the email is getting passed with a backslash
Why? Where is it getting passed from? Part of WP's core interface, or through a custom routine?
#5
in reply to:
↑ 4
@
10 years ago
Replying to boonebgorges:
Related: #18039.
The issue here is that the email is getting passed with a backslash
Why? Where is it getting passed from? Part of WP's core interface, or through a custom routine?
I'm not that familiar with core but to me it looks like wp_magic_quotes() in wp-includes\load.php so when you call is_email( $_POST[ 'email' ] ) on an email with an apostrophe/single quote it will also contain a backslash.
#6
@
10 years ago
I'm not that familiar with core but to me it looks like wp_magic_quotes() in wp-includes\load.php so when you call is_email( $_POST[ 'email' ] ) on an email with an apostrophe/single quote it will also contain a backslash.
Yes, but how are you getting to this point? Where in the interface are you entering the problematic email address? If you could provide specific details to reproduce, it'd be helpful.
#7
follow-up:
↓ 11
@
10 years ago
- Keywords close added
I suspect this is the reason why we still do that in core. People who don't understand the superglobal variables use them in security-sensitive areas.
JoshRoy, just use is_email( stripslashes( $_POST[ 'email' ] ) )
or as boonebgorges suggests, try to figure out which is the more appropriate variable.
#8
@
10 years ago
I apologize for my lack of details. I'm experiencing this issue when trying to edit a user using the standard WordPress interface, not through any custom code. We use a custom interface for adding users so I don't know if the error occurs there as well. is_email( $_POST[ 'email' ] ) is called on line 154 of wp-admin\user-edit.php
#9
@
10 years ago
I'm experiencing this issue when trying to edit a user using the standard WordPress interface, not through any custom code. We use a custom interface for adding users so I don't know if the error occurs there as well. is_email( $_POST[ 'email' ] ) is called on line 154 of wp-admin\user-edit.php
Aha. So the problem here is probably that we should be stripping slashes from the $_POST data before running through is_email()
, *not* that we should be accepting slashed data in is_email()
. Let's leave the ticket open for further testing.
#10
@
10 years ago
- Component changed from Formatting to Networks and Sites
- Focuses multisite added
- Keywords reporter-feedback close removed
- Summary changed from is_email not allowing apostrophe to Email with Apostrophe May Not Update in Multisite
Looks like a multisite bug then. Correcting summary.
#11
in reply to:
↑ 7
@
10 years ago
Replying to miqrogroove:
JoshRoy, just use
is_email( stripslashes( $_POST[ 'email' ] ) )
or as boonebgorges suggests, try to figure out which is the more appropriate variable.
Quick note: use wp_unslash
instead of stripslashes
here; if the superglobal slashing ever changes, wp_unslash
will ensure that doesn't break.
#12
@
10 years ago
- Component changed from Networks and Sites to Users
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#13
@
9 years ago
- Keywords has-patch added; needs-patch removed
First attempt at helping out here. Please let me know if I did something wrong. Thanks!
#15
@
6 years ago
- Status changed from new to reopened
Reopening as this is still an issue and can be picked up with the last patch.
#16
@
6 years ago
- Milestone set to Awaiting Review
We can try to fix this, but there's a deeper problem here.
Backing up for a moment, the line in question https://core.trac.wordpress.org/browser/tags/5.1/src/wp-admin/user-edit.php#L154 is intended to do the following: When editing a user's email address, look for rows in wp_signups
that match the login of the edited user, and update the email address there as well. As far as I can see, this signups
update has not worked since [12842]. Prior to that changeset, the $user_login
variable was defined (see https://core.trac.wordpress.org/browser/trunk/wp-admin/includes/ms.php?annotate=blame&rev=12841&marks=1166,1168#L1164). After that changeset, $user_login
was no longer supplied, causing the update to fail. For reference, this functionality was introduced in https://mu.trac.wordpress.org/changeset/1854.
I'm attaching a patch that fixes both issues - the apostrophe issue and the signups
issue - but I'm somewhat concerned that this bug is so baked into the behavior of WordPress Multisite that it may now be considered a feature. Plugins etc may now consider user_email
in signups
to be an immutable field, and it could conceivably cause problems to start changing it now. For this reason, I lean toward removing this block altogether. Cc @jeremyfelt in case he has some deep wisdom here.
I didn't see any other uses of is_email()
that take slashed data, but I may have missed something.
Apostrophe is allowed per RFC5322 and is properly accepted by
is_email()
in version 3.8. For example, this returns true (valid):is_email('j.d.o\'farrell@example.com');
Maybe you are using a UTF-8 character that looks like an apostrophe, such as HTML entity
‘
?