fix: prevent linking text with directional change char #388
Conversation
|
Hmm, this isn't the worst idea, and is much better than stripping characters. It would mean however that people putting URLs in the middle of text in Arabic or Hebrew will not get linked URLs at all. (Not saying this isn't viable, but if this solution is accepted then at least a warning should be shown about this.) |
| @@ -110,6 +110,10 @@ export function isDomainLabelChar(char: string): boolean { | |||
| return char === '_' || isDomainLabelStartChar(char); | |||
| } | |||
|
|
|||
| export function hasDirectionalChar(char: string) { | |||
There was a problem hiding this comment.
Minor, but can we rename this to isDirectionOverrideChar, and add a jsdoc comment above it explaining why it's used?
Can add a link for #377 too inside the jsdoc to point developers to the original issue that this new function solves
| @@ -1032,6 +1032,13 @@ describe('Autolinker Url Matching >', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('unicode exploits', () => { | |||
| fit('text with directional change characters should not be linked', () => { | |||
| describe('unicode exploits', () => { | ||
| fit('text with directional change characters should not be linked', () => { | ||
| expect(autolinker.link('foo.com\u202Ebar.com')).toBe('foo.com\u202Ebar.com'); | ||
| expect(autolinker.link('foo.com\u202Emoc.rab')).toBe('foo.com\u202Emoc.rab'); |
There was a problem hiding this comment.
In addition to the domain name tests here, we should add tests for when the \u202E char appears in the path, query string, and hash sections of the URL.
Also, can we add tests for each of the direction override chars like you had in #386?
This PR would disable linking for urls with directional change characters
Fixes #377