478446 - Bespin registration does not accept valid e-mail addresses with '+'

Status: RESOLVED FIXED

(Skywriter Graveyard :: General, defect)

Description

[Marcus P S]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6) Gecko/2009011912 Firefox/3.0.6 Ubiquity/0.1.5
Build Identifier: 

When registering for a Bespin account, e-mails with a '+' are not accepted even though they are valid e-mail addresses.

The + can be used as a way to categorize e-mails, as the text between the + and the @ is ignored for routing purposes, although it remains in the e-mail as it is transmitted. It is widely used by Gmail account holders, for example.

Reproducible: Always

Steps to Reproduce:
1. Follow the Bespin registration link in https://bespin.mozilla.com/index.html
2. Enter an email address of the form [e-mail]

Actual Results:  
The e-mail address will be flagged as invalid

Expected Results:  
The e-mail address should be accepted as it is well-formed.

See this IETF RFC for (too many) details, if you don't believe the + character is acceptable.

http://tools.ietf.org/html/rfc5322

Comment 1

[boushley]

Well, I guess its a question of how many you want to include there's some pretty big discussions on email validation.  I definitely see the use in the plus sign.  But to be totally official, I believe that this is the regex to do it
(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])

And that's just insane... but I think something like this
/^([a-zA-Z0-9_\.\-\+])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/

would probably suffice. 

That will allow for the plus sign.  

(Sorry Dion... not really familiar with submitting patches, otherwise I would... this needs to go in frontend/js/registration.js line 95 in the validateEmail function...)

Comment 2

[Reed Loden [:reed]]

(In reply to comment #1)
> /^([a-zA-Z0-9_\.\-\+])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/

There are gTLDs longer than four characters, mind you... .museum for example.

Comment 3

[Marcus P S]

I just think it may be a good idea to do proper validation.  If the e-mail address is well-formed, then there is at least some reasonable chance that someone might use it. It would be a shame to not allow these people to use the service (either here or wherever it may be installed) just because the proper way to validate the e-mail address was too complex.

Since the validation is done so rarely, what is the problem with complexity? Is it the difficulty of testing (ensuring code coverage, for example)?

Comment 4

[boushley]

One problem that I can see as being a problem with having the full validation is that it allows some characters such as ' " and the like.  And some people will run emails through this regex and then assume that it is safe and has no sql injection or something like that.  Just so you know.  

But I definitely agree that proper validation would be a good idea.  And some of those email addresses are ridiculous.  Anyways, whatever we want to go with  I think the point is that the regex we currently use is insufficient.  I copied both of those regex from other sites actually... I've used the shorter one or a derivative of it quite a bit.

So which one we want to go I don't really know... depends on how crazy you want to let people get.

Comment 5

[Gordon P. Hemsley [:GPHemsley]]

Confirming this bug. Strangely, though, the registration will complete even when the e-mail address is flagged as invalid (at least, with the plus sign as the cause).

Comment 6

[Gordon P. Hemsley [:GPHemsley]]

Attachment: Allows local part of e-mail address with plus sign (+) and underscore (_)

I think this patch should be good enough. People don't generally have really complicated e-mail addresses nowadays, with symbols and all that, so I don't think it's necessarily important to have the complete validation implemented.

However, this patch does allow the use of plus signs and underscores in the local part (username) of the e-mail address. It's a very minor change, in terms of the regexp.

Comment 7

[Dion Almaer]

Thanks Gordon! Fixed in changeset cd9532496882

Comment 8

[Gordon P. Hemsley [:GPHemsley]]

This is a mass migration from Mozilla Labs :: Bespin to Bespin :: General.

This bug likely still needs to be triaged and categorized.