Make WordPress Core

Opened 17 years ago

Closed 16 years ago

#4616 closed defect (bug) (fixed)

is_email isn't rfc2822 conformant

Reported by: hendry's profile hendry Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

I think that regex needs tweaking for my friend Martin and his a=b@… style email address:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=432547

Attachments (2)

4616.diff (608 bytes) - added by Nazgul 17 years ago.
4616b.diff (2.5 KB) - added by Nazgul 17 years ago.

Download all attachments as: .zip

Change History (22)

@Nazgul
17 years ago

#1 @Nazgul
17 years ago

  • Component changed from Administration to General
  • Keywords has-patch added
  • Milestone set to 2.3 (trunk)

Patch with new regex, based on one found here:
http://www.regular-expressions.info/email.html

#2 follow-up: @hendry
17 years ago

Sam Hocevar pointed out a better regex:
http://iamcal.com/publish/articles/php/parsing_email/

#3 in reply to: ↑ 2 @Nazgul
17 years ago

Replying to hendry:

Sam Hocevar pointed out a better regex:
http://iamcal.com/publish/articles/php/parsing_email/

That URL appears to be dead?

#4 @Nazgul
17 years ago

Thanks to Google Cache I was able to get that page.

New patch attached, based on the regex suggested by Sam Hocevar.

@Nazgul
17 years ago

#5 @jhodgdon
17 years ago

It would be handy if WordPress allowed email addresses like jhodgdon@localhost, for purposes of setting up test installations on a local box. Do any of the above regex suggestions allow "localhost" as the domain?

#6 follow-up: @Bobcat
17 years ago

  • Cc Bobcat added

Can we use PEAR? It has a function that does exactly what we need:

http://pear.php.net/manual/en/package.mail.mail-rfc822.parseaddresslist.php

#7 in reply to: ↑ 6 @foolswisdom
17 years ago

Replying to Bobcat:

Can we use PEAR? It has a function that does exactly what we need:

http://pear.php.net/reference/Mail-latest/__filesource/fsource_Mail__Mail-1.1.14MailRFC822.php.html
As the code is BSD licensed, which is GPL-compatible, you are welcome to distribute that code from there (with license) with WordPress.

#8 follow-up: @westi
17 years ago

I'm not convinced that we should include complete perfect rfc822 email validation support in core.

I would be more in favour of providing a simple validation scheme which matched the most common use cases and provided a filter for plugins to override the validation scheme.

Perfect 822 support is probably in the realms of a plugin.

I think we should concentrate on the more common email address types.

#9 @foolswisdom
17 years ago

This seems like a lot to do about a common problem with common solutions. I wasn't suggesting that mail-rfc822.parseaddresslist.php should be included only that either the specific solution to the problem by copied from there.

What does it cost us by being rfc822 compliant?

#10 in reply to: ↑ 8 @Bobcat
17 years ago

Replying to westi:

I think we should concentrate on the more common email address types.

I guess it depends on how you define "common". Besides alphanumerics, we currently allow - . and _ . We already have tickets for = and ' . Wouldn't it just be easiest to allow all the other special characters ! # $ % * / ? | ^ { } ` ~ & + ?

I can see prohibiting quoted strings in the address. That allows any characters at all, and I can see that as a potential security problem, despite assurances to the contrary (so I'm paranoid).

Regardless, the PEAR class parses out the entire "From:" line, which makes life a lot easier even if we don't allow the entire set of addresses.

#11 @foolswisdom
17 years ago

If it really takes that much code to be rfc822 complicant then I think now understand where westi is coming from, and agree that it is better to deal with all common (and uncomplex to implement) email address support, and leave compliance as extension territory. This will match the expectation created

Failing that, Nazgul's patch is a single function, based on a complete and highly regarded solution.

#12 @Bobcat
17 years ago

Nazgul's patch doesn't parse the "From:" line (for the Blog by Email feature). It only works once you have isolated the email address, which is a lot more complicated than it sounds.

#13 @Bobcat
17 years ago

OK, here's a proposal of what we should accept:

  1. Allow local@…, where local can contain alphanumeric characters plus ! # $ % * / ? | ^ { } ` ~ & ' + - = _ .
  2. As a design goal, check for the restrictions on the placement of the . character.
  3. Do not allow the form of email address where the local part is a quoted string. i.e., double quotes would not be allowed.
  4. Blog by Email will accept two forms for the From or Reply-To lines: either a plain email address all by itself as defined above, or a name in quotes (which is ignored) followed by an email address enclosed in angle brackets.
  5. Only one address in the From or Reply-To lines will be used.

This should handle almost all of the address formats in use.

#14 @jhodgdon
17 years ago

Please also consider that the domain part of the address could legally be "localhost", rather than something.com. This is very useful for test box set-ups, but is not allowed currently by WordPress.

#15 @Bobcat
17 years ago

Both of the proposed implementations above allow '@localhost'.

#16 @Bobcat
17 years ago

I'm working on a patch for this. I see that email addresses are run through wp_specialchars(). What is the purpose of this function? I don't see why email addresses should be run through it.

#17 @ryan
17 years ago

The PEAR module also offers these cheap checks:

$regex = $strict ? '/^([.0-9a-z_+-]+)@(([0-9a-z-]+\.)+[0-9a-z]{2,})$/i' : '/^([*+!.&#$|\'\\%\/0-9a-z^_`{}=?~:-]+)@(([0-9a-z-]+\.)+[0-9a-z]{2,})$/i';

#19 @ryan
16 years ago

  • Milestone changed from 2.9 to 2.8

#20 @ryan
16 years ago

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

(In [10769]) Improved is_email() and sanitize_email(). Props sambauers. fixes #9316 #4616

Note: See TracTickets for help on using tickets.