Escape some html characters for alert component #448
Conversation
y-kkamil
requested review from
antiheld,
dariadomagala-sap,
hardl,
jesusreal,
JohannesDoberer,
kwiatekus,
marynaKhromova,
maxmarkus,
parostatkiem-zz and
pekura
as code owners
|
Oooops.. I found another issue - will fix it in a minute |
There was a problem hiding this comment.
See my comments plus this one:
We add global onclick handler based on elemId that we get from a MF. This can install a click listener to any element in the DOM tree of luigi - not good! We should add a special prefix to our alert tag IDs or use a different mechanism.
| @@ -73,7 +72,8 @@ | |||
|
|
|||
| return Object.entries(links).reduce((acc, [key, content]) => { | |||
| const elemId = key; | |||
| const processedData = `<a id="${elemId}">${content.text}</a>`; | |||
| const escapedText = sanitizeHtml(content.text); | |||
| const processedData = `<a id="${elemId}">${escapedText}</a>`; | |||
There was a problem hiding this comment.
I would escape/sanitize the key/elemId as well - it won't unlikely be misused (because it will be escaped in the text before so it is not found when we want to replace it with tag - but it won't work then.
core/src/Alert.html
Outdated
| @@ -73,7 +72,8 @@ | |||
|
|
|||
| return Object.entries(links).reduce((acc, [key, content]) => { | |||
| const elemId = key; | |||
| const processedData = `<a id="${elemId}">${content.text}</a>`; | |||
| const escapedText = sanitizeHtml(content.text); | |||
| const processedData = `<a id="${elemId}">${escapedText}</a>`; | |||
| const pattern = new RegExp(`({${key}})`, 'g'); | |||
There was a problem hiding this comment.
The key can contain characters that regexp interpretes (like '.' for example). The key needs to be passed here regexp-escaped! (See here https://stackoverflow.com/questions/3561493/is-there-a-regexp-escape-function-in-javascript/3561711).
Description
Changes proposed in this pull request:
Related issue(s)
Fixes #447