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

fix(linky): handle the double quote character in email addresses #8964

Closed
wants to merge 1 commit into from
Closed

fix(linky): handle the double quote character in email addresses #8964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ngSanitize/filter/linky.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ angular.module('ngSanitize').filter('linky', ['$sanitize', function($sanitize) {
html.push('" ');
}
html.push('href="');
html.push(url);
html.push(url.replace('"', '"'));
Copy link
Contributor

Choose a reason for hiding this comment

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

@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 ".

Copy link
Contributor

Choose a reason for hiding this comment

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

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

html.push('">');
addText(text);
html.push('</a>');
Expand Down
4 changes: 4 additions & 0 deletions test/ngSanitize/filter/linkySpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ describe('linky', function() {
toEqual('my email is &#34;<a href="mailto:me@example.com">me@example.com</a>&#34;');
});

it('should handle quotes in the email', function() {
expect(linky('foo@"bar.com')).toEqual('<a href="mailto:foo@&#34;bar.com">foo@&#34;bar.com</a>');
});

it('should handle target:', function() {
expect(linky("http://example.com", "_blank")).
toEqual('<a target="_blank" href="http://example.com">http://example.com</a>');
Expand Down