Make valid_email_address() support IDNs

Very neat use of the new PHP 5.2 requirement! Patch applies cleanly with a little offset. Testing is working great so far! Using this patch it actually caught the invalid e-mail of 'test@e' that core did not catch before. It even catches the two commented-out invalid e-mails from the contact.test that apparently needed to be commented out since they weren't caught by the old valid_email_address function!
$invalid_recipients = array('invalid', 'invalid@', /*'invalid@site', 'invalid@site.',*/ '@site.', '@site.com');

Maybe a couple more reviews and RTBC!

I'd have to look and see if there was more we could replace with filter_var. One likely suspect is valid_url. Not sure how it would work with local drupal paths however.

function valid_url($url, $absolute = FALSE) {
  $allowed_characters = '[a-z0-9\/:_\-_\.\?\$,;~=#&%\+]';
  if ($absolute) {
    return (bool)filter_var($url, FILTER_VALIDATE_URL);
  }
  else {
    return (bool)preg_match("/^" . $allowed_characters . "+$/i", $url);
  }
}

Looks great since this will narrow down invalid e-mail addresses! Patch applied cleanly and ran the contact & user tests with 100% pass. I believe it is RTBC!

Looks like an improvement to me, and the tests pass. Committed. Thanks.

I think this is a bit of a step backwards. Previously you could use 'drewish@localhost' as a valid email this is not longer allowed.

I really don't think that should be a valid e-mail. Suppose you send a user an e-mail through their personal contact form. When that user replies to your e-mail, it's going to send it to 'drewish@localhost' from their computer or web mail, which would be invalid. If someone wants to use localhost as an e-mail domain, they should be using somekind of externall- accessable domain like 'mylocalcomputer.dyndns.org'.

looks like there are problems with the function: #43402 FILTER_VALIDATE_EMAIL is not RFC2822 compliant

dave reid, the function is called valid_email_address(), and 'drewish@localhost' is a valid email address. you could make an argument that we should have a function valid_globally_email() or some such but as the function is currently named it's a step backwards that could invalidate existing user data.

mfer, i think the relevant part of RFC 2822 is page 17 (emphasis is mine):

addr-spec = local-part "@" domain

local-part = dot-atom / quoted-string / obs-local-part

domain = dot-atom / domain-literal / obs-domain

domain-literal = [CFWS] "[" *([FWS] dcontent) [FWS] "]" [CFWS]

dcontent = dtext / quoted-pair

dtext = NO-WS-CTL / ; Non white space controls

%d33-90 / ; The rest of the US-ASCII
%d94-126 ; characters not including "[",
; "]", or "\"

The domain portion identifies the point to which the mail is
delivered. In the dot-atom form, this is interpreted as an Internet
domain name (either a host name or a mail exchanger name) as
described in [STD3, STD13, STD14]. In the domain-literal form, the
domain is interpreted as the literal Internet address of the
particular host. In both cases, how addressing is used and how
messages are transported to a particular host is covered in the mail
transport document [RFC2821]. These mechanisms are outside of the
scope of this document.

That leads me to believe that user@hostname is a valid email address.

Update: I forgot to mention that I'm in favor of rolling back to the regex.

Is there a case where using @localhost.localdomain wouldn't work? It passes valid_email_address and looks to be a generally more accepted form over "localhost". I just think there is no real use case for @localhost besides testing. I cannot e-mail user@localhost so it should not be a "valid" email.

Dave Reid, .localdomain isn't a reserved TLD and Apple uses .local and Microsoft suggests using it. if you just want something that'll validate you can use foo@127.0.0.1.

mfer, we need to be reading RFC 2822 rather than regular-expressions.info's commentary on it (on that page he discusses using happily using a regex that blocks .museum addresses).

i already quoted the part of the RFC with the grammar for an address token and you can see there that one of the productions for the domain token is the dot-atom token. this is in turn defined as:

atext = ALPHA / DIGIT / ; Any character except controls,
"!" / "#" / ; SP, and specials.
"$" / "%" / ; Used for atoms
"&" / "'" /
"*" / "+" /
"-" / "/" /
"=" / "?" /
"^" / "_" /
"`" / "{" /
"|" / "}" /
"~"

atom = [CFWS] 1*atext [CFWS]

dot-atom = [CFWS] dot-atom-text [CFWS]

dot-atom-text = 1*atext *("." 1*atext)

after brushing up on ABNF by reading RFC 2234, i read the 1*atext production to be one or more atext tokens then optionally followed by any number of period-one-or-more-atext tokens. please correct me if i'm reading that wrong but it sure looks like we're not following the standard the current implementation.

my big problem with this change is that it invalidates existing data. i could have foo@localhost on my site and next time i go to submit that form it'll fail.

Good points, though I think you meant section 4.1.3 Address Literals.

i've no objection to using the rfc 2821 so long as we document that that's what valid_email_address is using. there should probably be a changelog message too.

Using filter_var() could possibly introduce a security issue on PHP 5.2.0 and 5.2.1:
PHP ext/filter Email Validation Vulnerability, http://www.php-security.org/MOPB/PMOPB-45-2007.html

I'm with RFC 2821 as will the majority of admins that deploy Drupal I am sure. For this, I would rather see us validate against user@localhost type of emails.

I hope the powers above will use common sense when deciding this. Drupal is largely used on the internet rather than intranet.

RFC 2821 should absolutely be the default validation, since it is only a few cases where it is not preferred.

An option I've been brainstorming recently: allowing e-mails like user@machinename should be something fit for an intranet install profile that will somehow work to override email address verification to only allow e-mails of a certain domain, etc. The trouble would be how exactly to implement 'other' validation in valid_email_address. Maybe some kind of hook_validate?

RFC 2821 is probaly soon obsolete! RFC 5321 is here: http://tools.ietf.org/html/rfc5321
RFC 2822 is also probaly soon obsolete! RFC 5322 is here: http://tools.ietf.org/html/rfc5322

alexanderpas, the 532? standards are still drafts, I don't think it'd be wise to adopt those just yet. In any case it doesn't answer the question of which spec do we follow.

is the e-mail address user@[127.0.0.1] valid? afaik yes.

in my opinion, we should only accept localhost e-mail addresses after user confirmation.

Notice: you have used a localhost e-mail address, mails send to this address will never be send to another computer, and you can only recieve mail send to this address from this computer.

RFC 5321 and RFC 5322 have been released on October 2, 2008, and supersede the old rfcs.
Here is a commented comparison of the (few and subtle) changes.

This here is a crazy long regex for validating email addresses - never seen such a regex before. But I don't think we need to go that far. We need to do a good job, not a perfect one.

However, I also noticed that neither valid_url() nor valid_email_address() accept IDNs like www.übercart.com, which are perfectly valid. This is a real showstopper, especially for languages with a non-latin alphabet. IPv6 is not accepted either in the domain part.

So we definitely need to study the RFCs and rewrite the regexes correctly. Or we can search for the best premade one we can find and do a lot of testing. I suppose there will be acceptable ones in some other GPLed projects.

The more I think about this I'm not sure I like the idea of visitors coming to a site, filling out the contact form with an address like user@example and having it pass validation.

A quick reminder to all participants in the discussion; the email address validation should only check for compliance to the relevant RFC, not whether the address is an existing and / or reachable address.

In the case of the quote, there's not much difference to user@example and user@a-non-existing-domain.com or even user@example.com.

Please take notice of #209672: Use site name in From: header for system e-mails and #214114: Modify valid email core function to support RFC2822 display name. as well.

don't forget punycode ;) http://www.xn--bercart-m2a.com/

marked #250022: User was able to register with email address without ".com" on the end as a duplicate.

FWIW, FILTER_VALIDATE_EMAIL accepts "foo@localhost" as of PHP 5.2.9 that was released today:

http://bugs.php.net/bug.php?id=47282
http://cvs.php.net/viewvc.cgi/php-src/ext/filter/logical_filters.c?r1=1....
http://cvs.php.net/viewvc.cgi/php-src/ext/filter/tests/016.phpt?r1=1.4.2...

okay... that seems to make this issue completely fixable...

now we just need the following:
IDN to ASCII encoded domain. (using Nameprep (RFC 3491) and Punycode (RFC 3492) encoding)
ASCII encoded domain to IDN. (using Punycode (RFC 3492) decoding)

I'm thinking that this encoding only should happen if:
1) FILTER_VALIDATE_EMAIL determines it is not a valid e-mail address (i'm using the Pareto principle here)
2) the domain contains non-ASCII characters.
it just should just redo FILTER_VALIDATE_EMAIL after encoding.

postponed until #389278: Create IDN encoding and decoding functions is in.

Moving this to D8 because #389278: Create IDN encoding and decoding functions has been moved to D8 as well.

#984568: valid email not passing validation (someuser@localhost) marked as duplicate.

It maybe that an email address without a domain-ending is a valid email adress.

But for the normal practice i think it is not the right way to use this function for user registration.

On all big platforms like amazon or ebay an email address without a domain-ending will not accepted.

This is a feature, not a bug.

This is most certainly a bug which should urgently be fixed in the stable versions.

Anybody interested in fixing this beyond using it for issue ping-pong games?

8.x now requires PHP 5.3, this means that foo@localhost e-mail addresses are supported. For Drupal 7 there is an easy workaround of upgrading to PHP 5.2.9+

Some people clearly don't like this - but that's not a major bug, so since that discussion has barely anything to do with the original patch that was committed, I'm marking this as 'closed (fixed)' as it should have been over a year ago. If you want to revert the use of FILTER_VALIDATE_EMAIL then please open a new issue to discuss that for 8.x.

It looks like killes has found a valid issue in 6.x that was fixed by this patch - we should probably have a separate 6.x issue for that since it can't use a fix requiring PHP 5.2.x anyway.

Strange, I have a D8 installation running on Ubuntu 10.10, and it doesn't want to accept mike@localhost as a valid email address. Is there something special I need to do here, or should I re-open the bug?