Make WordPress Core

Opened 13 years ago

Last modified 11 months ago

#17491 reopened defect (bug)

Make is_email() compliant with RFC5322 (updated by RFC6854)

Reported by: arena's profile arena Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.1.2
Component: Formatting Keywords: has-patch dev-feedback needs-unit-tests is-email
Focuses: Cc:

Description (last modified by kawauso)

is_email('toto.@…') returns true

Attachments (8)

17491.diff (538 bytes) - added by sivel 13 years ago.
#17491.patch (5.4 KB) - added by arena 12 years ago.
new patch
is_email_new.patch (8.1 KB) - added by arena 5 years ago.
is_email_new.patch
#is_email_new_v2.patch (8.2 KB) - added by arena 5 years ago.
removed empty line introduced after function declarations and first line of code
#is_email_new_v3.patch (8.2 KB) - added by arena 5 years ago.
v3 => to adhere to the coding standards
is_email.PNG (51.6 KB) - added by garrett-eclipse 5 years ago.
Quoting @arena - "Here is the result of the current is_email() :"
is_email_new.PNG (55.5 KB) - added by garrett-eclipse 5 years ago.
Quoting @arena - "Here is the result applying the attached patch (is_email_new.patch) :"
46391.diff (1021 bytes) - added by achyuthajoy 5 years ago.
is_email local part Regex update to match RFC 5952

Download all attachments as: .zip

Change History (44)

#1 @arena
13 years ago

toto.@… does not comply with RFC 2822

#2 @scribu
13 years ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from critical to minor
  • Summary changed from is_email('toto.@toto.com') returns true !!!! to is_email('toto.@toto.com') returns true

Let's not get carried away.

@sivel
13 years ago

#3 @sivel
13 years ago

  • Keywords has-patch added; needs-patch removed

Patch adds in the rest of the regex from http://www.regular-expressions.info/email.html for the local part.

#4 @scribu
13 years ago

  • Milestone changed from Future Release to 3.2

#5 @ryan
13 years ago

  • Milestone changed from 3.2 to Future Release

#6 @arena
12 years ago

  • Keywords dev-feedback added

Here is a new fix (#17491.patch) with code from the class swiftmailer

http://swiftmailer.org

to comply with RFC2822

@arena
12 years ago

new patch

#7 follow-up: @arena
12 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

duplicate with #19759

#8 in reply to: ↑ 7 ; follow-up: @kawauso
12 years ago

Replying to arena:

duplicate with #19759

Are we closing an existing ticket with feedback and milestone in favour of a new one with a similar patch and issue or am I missing something?

#9 in reply to: ↑ 8 @helenyhou
12 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Replying to kawauso:

Are we closing an existing ticket with feedback and milestone in favour of a new one with a similar patch and issue or am I missing something?

I think arena may be trying to bump the ticket. Closing #19759 as duplicate - patch is exactly the same as #17491.patch.

#10 @arena
12 years ago

my purpose was not to bump but to clarify the situation with a more specific title : making is_email() compliant with rfc2822.

#11 @helenyhou
12 years ago

  • Summary changed from is_email('toto.@toto.com') returns true to Make is_email() compliant with RFC2822

#12 @arena
12 years ago

  • Summary changed from Make is_email() compliant with RFC2822 to Make is_email() compliant with RFC2822 !

#13 @kawauso
12 years ago

  • Description modified (diff)
  • Summary changed from Make is_email() compliant with RFC2822 ! to Make is_email() compliant with RFC2822

No need, really. Especially for those of us with email threads.

#14 follow-up: @miqrogroove
11 years ago

Have you considered simply adopting the Google solution? They offer a free PHP script for e-mail syntax validation.

#15 in reply to: ↑ 14 @arena
11 years ago

Replying to miqrogroove:

Have you considered simply adopting the Google solution? They offer a free PHP script for e-mail syntax validation.

any link to check that 'magic' google code , thanks

#17 @rmccue
11 years ago

(Worth noting that's not actually a Google-written piece of code, it's just hosted on Google Code.)

#19 @SergeyBiryukov
11 years ago

  • Component changed from General to Mail

Related: #25108

#20 @dd32
9 years ago

#17678 was marked as a duplicate.

#21 @dd32
9 years ago

#17678 proposed this package: http://code.google.com/p/isemail/ (BSD-licensed and unit-tested)

Personally I don't see a huge need to add a 100% RFC compliant heavy package to do email validation, I'd rather stick to the side of fast, small, simple - That being said, I admit email validation doesn't follow that.

We already Test for leading and trailing periods and whitespace in the $domain part of the email, if we were to extend that to the $local part of the email we'd cover the original issue here (and one that i've seen quite often).

Last edited 9 years ago by dd32 (previous) (diff)

#22 @miqrogroove
9 years ago

  • Component changed from Mail to Formatting
  • Keywords is-email added

This ticket was mentioned in Slack in #core-privacy by pepe. View the logs.


5 years ago

#25 @garrett-eclipse
5 years ago

I'm re-opening this ticket as it seems the most appropriate location to discuss RFC email compliance as was raised in #46343 recently. In that ticket it was flagged that plugins in some cases support "RFC : RFC822 (year 1982), 2822 (year 2001), 532x (year 2010), 653x (year 2012)" but these wouldn't be erasable as they don't currently pass the is_email check. I closed #46343 indicating as these are custom from core and to support these in the privacy tools they'd need to customize is_email there. That being said I wanted to re-open this ticket to look into support for the RFC compliant emails that currently aren't covered in core.
Thank you

#26 @garrett-eclipse
5 years ago

Other related tickets that haven't previously been mentioned in this thread;
#4616
#9316
#15379

#27 @SergeyBiryukov
5 years ago

  • Milestone set to Future Release

@arena
5 years ago

is_email_new.patch

#28 @arena
5 years ago

  • Keywords 2nd-opinion added

Here is a table with 3 columns : expected result, email, result.
The last column is in red (background color) when the result is not equal to what is expected.

Here is the result of the current is_email() :

https://drive.google.com/open

Here is the result applying the attached patch (is_email_new.patch) :

https://drive.google.com/open

hope this will help !

@arena
5 years ago

removed empty line introduced after function declarations and first line of code

#29 @arena
5 years ago

  • Keywords dev-feedback 2nd-opinion removed

@arena
5 years ago

v3 => to adhere to the coding standards

@garrett-eclipse
5 years ago

Quoting @arena - "Here is the result of the current is_email() :"

@garrett-eclipse
5 years ago

Quoting @arena - "Here is the result applying the attached patch (is_email_new.patch) :"

#30 @garrett-eclipse
5 years ago

  • Keywords dev-feedback 2nd-opinion added; is-email removed

Thanks @arena appreciate the patches. I'll be honest this is a bit out of my wheelhouse so am going to let others feedback. Cheers
P.S. I uploaded your screens to Trac for safekeeping.

#31 @ocean90
5 years ago

#44191 was marked as a duplicate.

#32 @ocean90
5 years ago

#46391 was marked as a duplicate.

#33 @ocean90
5 years ago

  • Keywords needs-unit-tests is-email added; 2nd-opinion removed

@achyuthajoy
5 years ago

is_email local part Regex update to match RFC 5952

#34 @arena
5 years ago

@achyuthajoy,

nice move, but out of scope of ticket subject !

WordPress is told to power more or less 10% of the web.
The wordpress open source dev community is obviously focused to fully support web standards.
In WP, you can use left to right and right to left languages but cannot use internationalized emails such as 用户@例子.广告 or θσερ@εχαμπλε.ψομ or अजय@डाटा.भारत

If you read #46343 (closed invalid) there is still a lot of work to do ...
RFC822 (year 1982), 2822 (year 2001), 532x (year 2010), 653x (year 2012)
The last one (internationalized emails, RFC6531) requires smtp mail server to support SMTPUTF8 extension (gmail as a smtp server does it very well). May be there are some external constraints that make this part of the wordpress code so stable (almost a decade) ! (maybe wordpress.com mail servers ?)

I expect to know more whenever the V2 privacy roadmap will be formalized : kick off, objectives, milestones, deadline ... (i googled it and all i found for now is this)

Regards

#35 @garrett-eclipse
5 years ago

Hi @arena as alot of tickets are being closed as duplicate in reference to this one and is_email and RFC compliance we might want to expand the title/scope here to be a general compliance ticket and tackle as many of them as can be done. Just my thought.

As to the Privacy Roadmap it can be found here - https://make.wordpress.org/core/roadmap/privacy/
Work has already started on the roadmap and you can see some tickets with strikethrough that have been addressed already. The objectives are outlined in the document. There are no milestones or deadlines just general direction.

  • Not sure what you're looking to know from there but if something needs external resources then we'd want to avoid that from a privacy perspective and at worst use a wp.org server over a wp.com.

#36 @arena
5 years ago

just for your information,

Drupal Api is using egulias/EmailValidator

#37 @arena
11 months ago

  • Summary changed from Make is_email() compliant with RFC2822 to Make is_email() compliant with RFC5322 (updated by RFC6854)
Note: See TracTickets for help on using tickets.