email validator is wrong #5899

Closed
ofirdagan opened this Issue Jan 20, 2014 · 25 comments

Projects

None yet
@ofirdagan

If you enter the email address:
me@.example.com

It says the email is valid..

Check this in this URL (from the docs):

http://docs.angularjs.org/api/ng.directive:input.email

@caitp
Contributor
caitp commented Jan 22, 2014

Yeah you're right, it looks like EMAIL_REGEX is wrong.

According to http://dev.w3.org/html5/markup/input.email.html, we can use

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

and that looks like it should work better due to not including the . following the @

I'll try submitting a quick patch for this.

... actually, this is a bit more complicated than I thought it would be, this might take a bit more work. It looks like it will also accept regexps which begin with a period, too. So there's a bit of work to do here I guess.

If you can solve this before me, feel free to submit a pull request

@caitp caitp added a commit to caitp/angular.js that referenced this issue Jan 22, 2014
@caitp caitp fix(input): use Chromium's email validation regexp
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
a48c0cb
@caitp caitp added a commit that closed this issue Jan 22, 2014
@caitp caitp fix(input): use Chromium's email validation regexp
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
79e519f
@caitp caitp closed this in 79e519f Jan 22, 2014
@caitp
Contributor
caitp commented Jan 22, 2014

Thanks for the bug report. On the docs demo, you won't notice this fix until 1.2.10 drops, because it loads the 1.2.9 angular script all the time. But in a few days you should see this addressed on the doc demo (unless the commit gets reverted, which could happen O_O)

@ofirdagan

Great, 10x!

On Wed, Jan 22, 2014 at 5:32 AM, Caitlin Potter notifications@github.comwrote:

Thanks for the bug report. On the docs demo, you won't notice this fix
until 1.2.10 drops, because it loads the 1.2.9 angular script all the time.
But in a few days you should see this addressed on the doc demo


Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/5899#issuecomment-32990587
.

@mica16
mica16 commented Jan 29, 2014

What was my facial expression when I noticed that "dummy@ferferfe" (without any domain) is considered as a valid mail ;)

@fbenz fbenz added a commit to fbenz/angular.js that referenced this issue Feb 4, 2014
@caitp @fbenz caitp + fbenz fix(input): use Chromium's email validation regexp
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
b2cd278
@Zequez
Zequez commented Feb 13, 2014

@mica16 but dummy@ferferfe is a valid email address. Domains don't need to have an extension, just think about localhost.

@mbeckenbach

@Zequez correct, but who the hell uses a localhost address for a websites registration in example?

@DracoThuban

@mbeckenbach Use
ng-pattern="/^[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+.[a-z0-9-]/"
Not ideal but works.

@Zequez
Zequez commented Feb 17, 2014

@mbeckenbach yeah, true. But consider that a domain name without dots is a valid domain. That ICANN won't let you register a domain with THEM, doesn't mean that the domain name is invalid.

@jeffmcmahan

6@6 is not an email address. The fact that it could be a proper email address in a special circumstance shouldn't, in my opinion, lead us to permit it. The odds of a mistake getting through via this permissive pattern are orders of magnitude greater than the odds that we'll exclude someone with such an email address.

@mica16
mica16 commented Feb 22, 2014

+1 for Jeff

@caitp
Contributor
caitp commented Feb 23, 2014

@jeffmcmahan I'm sorry but it sounds like you should file an issue with the IETF or the WHATWG. Once the definition of what an email address looks like changes, then it should be changed in Angular, but we really need to follow what is described in the RFCs and WHATWG recommendations.

Now, if you do want to be a bit more strict, you could also add a pattern validator, that would be fine.

@jeffmcmahan

@caitp You're the boss.

@rjokelai
rjokelai commented May 8, 2014

How about making the EMAIL_REGEXP configurable? Our server side validation is stricter than the angularjs one. Specifically, it requires, e.g., that the part before the @ sign is not longer than 63 characters. Now we have to create a custom directive for all email inputs or remember to add the ng-pattern to each email input.

@caitp
Contributor
caitp commented May 8, 2014

you can use ng-pattern in addition to type=email, @rjokelai -- also, @matsko's validation pipeline change will make it easier to add custom email validation

@omasback

"Now, if you do want to be a bit more strict, you could also add a pattern validator, that would be fine."

how about the default validator only accepts normal emails with a dot in them, and if someone in the tiny minority wants to be less strict, they can make their own validator. Why should the vast majority of devs need to make their own pattern just so the tiny minority doesnt have to? This is silly standards purism

@caitp
Contributor
caitp commented May 24, 2014

because this is basically a polyfill for html5 constraint validation, that's why. Next week we'll hopefully be shipping a feature that will make customizing this much easier

@revolunet revolunet added a commit to revolunet/angular.js that referenced this issue Jun 12, 2014
@caitp @revolunet caitp + revolunet fix(input): use Chromium's email validation regexp
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
30315f9
@artuska
artuska commented Oct 22, 2014

Are emails without domain zone valid? I'm testing simple input field with type="email" and Angular validates foo@bar email succesfully. (Angular v1.3.0)

Oh, there is a closed issue already — #9463

@caitp
Contributor
caitp commented Oct 22, 2014

Are emails without domain zone valid? I'm testing simple input field with type="email" and Angular validates foo@bar email succesfully. (Angular v1.3.0)

They are valid emails, per spec. This question has been answered so many times on this issue tracker, there is nothing invalid about an email without a TLD.

Use a custom validator to require the specific TLDs you want

@jeffmcmahan

@caitp, I realize we've been hassling you about this, but I think it could be helpful if you explained why keeping to the spec is imperative. We all recognize that a reasonable and competent developer is not going to assume that the email validator allows 0@0. That is to say, it's going to be used for the standard form registration situation, and it's going to cause problems in many such situations. And we're told that the importance of keeping to the spec outweighs those problems. Okay---cool. But why?

@caitp
Contributor
caitp commented Oct 22, 2014

Because it's going to be really weird when your browser says it's valid and angular says it isn't

@caitp
Contributor
caitp commented Oct 22, 2014

You're not looking for email validation, you're looking for pattern validation on top of email validation --- you want valid email addresses (handled by angular) which match a specific pattern (contains a TLD)

@jeffmcmahan
  1. Would it be weird when you add pattern validation and then "your browser says it's valid and [A]ngular says it isn't"? No. So why would it be weird if that pattern validation were built in?
  2. We're all aware that what passes for a valid email in the use case we're interested in is narrower than the spec.

I just want to know why Angular shouldn't serve the use case I'm interested in, since it is the general case.

@caitp
Contributor
caitp commented Oct 22, 2014

So why would it be weird if that pattern validation were built in?

Actually, you get the same behaviour from the browser. http://jsfiddle.net/t6rdmngo/ If you run this in a modern browser, you won't be able to submit the top form because of a pattern mismatch if you don't include a TLD.

In the angular version, you get the same behaviour. This is good. This is what we want. Identical behaviour between the native behaviour and the framework behaviour. The framework shouldn't have disagreements with the platform

@jeffmcmahan

Now I get it. Thanks for putting up with me!

@kumarharsh kumarharsh referenced this issue in playlyfe/themis Jul 31, 2015
Merged

Email Regex #20

@steelx
steelx commented Nov 16, 2015

You can quickly fix this my using ng-pattern

<input type="email" ng-pattern="/^[_a-z0-9]+(\.[_a-z0-9]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,4})$/">

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment