-
Notifications
You must be signed in to change notification settings - Fork 27.5k
email validation regexp update with tests #6026
Conversation
Previously, domain parts which began with a dash or a number, or ended with a dash, would be accepted as valid.
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
CLA is now signed. On Tue, Jan 28, 2014 at 3:15 PM, Mary Poppins [email protected]:
|
The fix was merged into chrome, https://src.chromium.org/viewvc/blink?revision=165978&view=revision, probably won't get released until like v34 or something. But, it might not be the worst thing in the world to merge a similar fix into angular. I'll mark this for 1.3 |
@@ -9,7 +9,7 @@ | |||
*/ | |||
|
|||
var URL_REGEXP = /^(ftp|http|https):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?$/; | |||
var EMAIL_REGEXP = /^[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z0-9-]+(\.[a-z0-9-]+)*$/i; | |||
var EMAIL_REGEXP = /^[a-z0-9!#$%&'*+/=?^_`{|}~.-]+@[a-z]([a-z0-9-]*[a-z0-9])?(\.[a-z]([a-z0-9-]*[a-z0-9])?)*$/i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[a-z]([a-z0-9-]*[a-z0-9])
The first character of a domain label can be a digit ([email protected], for example).
In Chrome, we're using a maximum of 63 digits for each domain label, because the RFC states that the domain can only be so long. It might be good to use that strategy here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would appear that the rfc I referenced was outdated.
From rfc 1123.
Limiting domain labels to 63 characters would also appear to not be correct.
2.1 Host Names and Numbers
The syntax of a legal Internet host name was specified in
RFC-952 http://www.faqs.org/rfcs/rfc952.html
[DNS:4]. One aspect of host name syntax is hereby changed: the
restriction on the first character is relaxed to allow either a
letter or a digit. Host software MUST support this more liberal
syntax.
Host software MUST handle host names of up to 63 characters and
SHOULD handle host names of up to 255 characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
domain label
and host name
are different things. A domain label is the section of text between .
periods (and after the @
). An email address is technically not supposed to exceed 256 characters, but the domain label in Chromium is limited to 63 characters each, which is way more than anybody really uses.
For instance, [email protected]
, both 4chan
and org
are domain labels. 4chan.org
is the full domain name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a character limit necessary though? The consequences of not implementing
a character limit are the same as a typo or spelling mistake, are they not?
On Wed, Feb 19, 2014 at 1:53 PM, Caitlin Potter [email protected]:
In src/ng/directive/input.js:
@@ -9,7 +9,7 @@
*/var URL_REGEXP = /^(ftp|http|https)://(\w+:{0,1}\w_@)?(\S+)(:[0-9]+)?(/|/([\w#!:.?+=&%@!-/]))?$/;
-var EMAIL_REGEXP = /^[a-z0-9!#$%&'+/=?^{|}~.-]+@[a-z0-9-]+(.[a-z0-9-]+)_$/i; +var EMAIL_REGEXP = /^[a-z0-9!#$%&'_+/=?^_
{|}~.-]+@a-z?(.a-z?)*$/i;domain label and host name are different things. A domain label is the
section of text between . periods (and after the @). An email address is
technically not supposed to exceed 256 characters, but the domain label in
Chromium is limited to 63 characters each, which is way more than anybody
really uses.Reply to this email directly or view it on GitHubhttps://github.com//pull/6026/files#r9885266
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to use the same character limit rules, but I'm not super worried about these, I would like to hear the opinion of another developer for that.
changed the regular expression again as domain validation was modified by rfc1123 to allow numbers in the first character of a domain label.
modified test for domain labels beginning with numebrs to match rfc 1123
So, Chrome stable has the correct email validation now, I think we can merge this, it's a bit later than it should have been =) |
agh oh god didn't commit message it >_< |
Previously, domain parts which began with or ended with a dash, would be accepted as valid. This CL matches Angular's email validation with that of Chromium and Firefox. Closes #6026
Previously, domain parts which began with or ended with a dash, would be accepted as valid. This CL matches Angular's email validation with that of Chromium and Firefox. Closes #6026
Previously, domain parts which began with or ended with a dash, would be accepted as valid. This CL matches Angular's email validation with that of Chromium and Firefox. Closes angular#6026
Added some improved testing and regexp matching to be a closer match to chromium and to rfc1035