fix(linky): handle the double quote character in email addresses #8964
Conversation
Handle the double quote character as part of the domain in email addresses Closes angular#8945
petebacondarwin
force-pushed
the
master
branch
from
02dc2aa to
fd2d6c0
Compare
|
I can confirm that this fixes #9256 in that no exception is raised, but the linkification isn't optimal |
|
@lorenzhs I agree that the linkification is not optimal. This patch is not about fixing the regex that is used for emails or URLs, just about not generating an exception when quotes are part of what will end up being linkified |
|
@lgalfaso Yes, absolutely. Linkification can be optimised later on, but raising an exception on a valid input is a lot worse. |
|
I don't think that this fixes #8945. I think the right thing to do in the example provided in that issue is not to translate that snippet of text into a link. |
|
@IgorMinar this fixes a potential security issue. If tomorrow the regex is changed so it accepts spaces and quotes (for whatever reason) then Valid case This is some text, and we enhanced the email regex so it is able to accept comments in the email
foo@(comment goes here)bar.comwould get transformed to This is some text, and we enhanced the email regex so it is able to accept comments in the email
<a href="mailto:foo@(comment goes here)bar.com">foo@(comment goes here)bar.com</a>but foo@(comment" onclick="alert(1);" id="goes here)bar.comwould get transformed to <a href="foo@(comment" onclick="alert(1);" id="goes here)bar.com">foo@(comment" onclick="alert(1);" id="goes here)bar.com</a>
foo@(comment" onclick="alert(1);" id="goes here)bar.comFixing the regex is another issue (the same can be said by URLs) |
jeffbcross
force-pushed
the
master
branch
3 times, most recently
from
8292c21 to
7b9fddf
Compare
petebacondarwin
force-pushed
the
master
branch
from
4dd5a20 to
998c61c
Compare
| @@ -142,7 +142,7 @@ angular.module('ngSanitize').filter('linky', ['$sanitize', function($sanitize) { | |||
| html.push('" '); | |||
| } | |||
| html.push('href="'); | |||
| html.push(url); | |||
| html.push(url.replace('"', '"')); | |||
There was a problem hiding this comment.
@lgalfaso - from what I read here - https://url.spec.whatwg.org/#url-parsing - we should be percent-encoding characters, rather than HTML encoding them. So the double quotes would be encoded as %22. Here is a link to the spec: http://tools.ietf.org/html/rfc3696#section-4.1
I was wondering if we should go a step further and percent encode all the non-URL code points, which includes ".
There was a problem hiding this comment.
But otherwise, I think this is a good compromise before we try improve our email parsing, which I now see if rather complicated if you allow comments and quoted sections...
There was a problem hiding this comment.
@petebacondarwin it is true that quotes need to be percent encoded, but if something reaches this point then we do not care what it is. We just have to put it inside an html attribute, where we need to encode the double quotes as this is a double-quotes attribute value http://www.w3.org/TR/html-markup/syntax.html#syntax-attr-double-quoted
There was a problem hiding this comment.
I agree that the regex that we use to detect emails and urls need some help, I am not trying to fix that, I am just trying to make linky generate valid html so angular does not break for people that use linky with user content
There was a problem hiding this comment.
So I think what you are saying is that since "Pete Bacon Darwin"@example.com is a valid email (even with unencoded double quotes) then this ought to be allowed as a value to be put into an anchor element's href attribute, and that when serializing this attribute we should only be HTML encoding it.
I am good with that. Let's go with this and then put out a PR Plz issue for better improving the email/URL identification in linky - perhaps looking at something like https://github.com/FogCreek/email-addresses
I'll merge.
Email addresses can (under certain restrictions) include double quote characters. See http://tools.ietf.org/html/rfc3696#section-3. For example, `"Jo Bloggs"@abc.com` is a valid email address. When serializing emails to the `href` attribute of an anchor element, we must HTML encode these double quote characters. See http://www.w3.org/TR/html-markup/syntax.html#syntax-attr-double-quoted This commit does not attempt to improve the functionality (i.e. regex) that attempts to identify email addresses in a general string. Closes #8945 Closes #8964 Closes #5946 Closes #10090 Closes #9256
Handle the double quote character as part of the domain in email addresses
Closes #8945