Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(input): use Chromium's email validation regexp
Browse files Browse the repository at this point in the history 
This change uses the regexp from Chromium/Blink to validate emails, and corrects
an error in the validation engine, which previously considered an invalid email
to be valid. Additionally, the regexp was invalidating emails with capital
letters, however this is not the behaviour recomended in the spec, or implemented
in Chromium.

Closes #5899
Closes #5924
  • Loading branch information
@caitp
caitp committed Jan 22, 2014
1 parent 7cf5544 commit 79e519f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/ng/directive/input.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

var URL_REGEXP = /^(ftp|http|https):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?$/;
var EMAIL_REGEXP = /^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,6}$/;
var EMAIL_REGEXP = /^[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(\.[a-z0-9-]+)*$/i;
var NUMBER_REGEXP = /^\s*(\-|\+)?(\d+|(\d*(\.\d*)))\s*$/;

var inputType = {
Expand Down
3 changes: 2 additions & 1 deletion test/ng/directive/inputSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,8 @@ describe('input', function() {
it('should validate email', function() {
expect(EMAIL_REGEXP.test('[email protected]')).toBe(true);
expect(EMAIL_REGEXP.test('[email protected]')).toBe(true);
expect(EMAIL_REGEXP.test('[email protected]')).toBe(false);
expect(EMAIL_REGEXP.test('[email protected]')).toBe(true);
expect(EMAIL_REGEXP.test('[email protected]')).toBe(false);
});
});
});
Expand Down

6 comments on commit 79e519f

@litera
Copy link

@litera litera commented on 79e519f Feb 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's likely not ok to allow emails like a@b because these validate as ok. :S
Shouldn't this regular expression at least require two part after @:

/^[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(\.[a-z0-9-]+)+$/i

@caitp
Copy link
Contributor Author

@caitp caitp commented on 79e519f Feb 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A valid e-mail address is a string that matches the email production of the following ABNF, the character set for which is Unicode. This ABNF implements the extensions described in RFC 1123. [ABNF] [RFC5322] [RFC1034] [RFC1123]

email         = 1*( atext / "." ) "@" label *( "." label )
label         = let-dig [ [ ldh-str ] let-dig ]  ; limited to a length of 63 characters by RFC 1034 section 3.5
atext         = < as defined in RFC 5322 section 3.2.3 >
let-dig       = < as defined in RFC 1034 section 3.5 >
ldh-str       = < as defined in RFC 1034 section 3.5 >

This requirement is a willful violation of RFC 5322, which defines a syntax for e-mail addresses that is simultaneously too strict (before the "@" character), too vague (after the "@" character), and too lax (allowing comments, whitespace characters, and quoted strings in manners unfamiliar to most users) to be of practical use here.

The following JavaScript- and Perl-compatible regular expression is an implementation of the above definition.

/^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/

Based on that regexp, you can see that there is no requirement for a top-level domain, and even based on the RFC itself.

@vin
Copy link
Contributor

@vin vin commented on 79e519f Feb 28, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance this can be backported to v1.0.x ?

@quantizor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should just be able to go into your version of angular.js and replace the regex expression, vin. It doesn't look like anything else changed.

@vin
Copy link
Contributor

@vin vin commented on 79e519f Mar 2, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caitp
Copy link
Contributor Author

@caitp caitp commented on 79e519f Mar 2, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vin it's technically possible to do by creating a branch from a particular tag and backporting the fixes in, but to get that on the CDN someone internal to google would have to deal with that, and we'd have to discuss how we want to deal with support for older versions.

I think it's a good discussion to have, but we don't really have like an ESR programme at the moment, and most of the energy is not going to be going into maintaining older releases, unless we get a lot more help with it. (Keep in mind, this is just my opinion, but it's not an easy thing to do)

Please sign in to comment.