Add safety check for falsy sanitizer input. #29639
Conversation
When the input of sanitizer evaluates to false: - do not rely on assumed string length - default to empty string on falsy input With that change the falsy input like null, undefined or empty string should return the same results: empty string. Thanks!
| @@ -92,8 +92,8 @@ export const DefaultWhitelist = { | |||
| } | |||
|
|
|||
| export function sanitizeHtml(unsafeHtml, whiteList, sanitizeFn) { | |||
| if (!unsafeHtml.length) { | |||
| return unsafeHtml | |||
| if (!unsafeHtml || !unsafeHtml.length) { | |||
There was a problem hiding this comment.
How about we just do !Boolean(unsafeHtml)?
There was a problem hiding this comment.
Good point, done the reading and now I knew Boolean(...) is alrady used here in the source base. To be amended
There was a problem hiding this comment.
So that way:
if (!Boolean(unsafeHtml) || !unsafeHtml.length) {cannot be written, as there is a rule in ESLint config Redundant Boolean call.eslint(no-extra-boolean-cast), which fixes to expression I've used. Not sure what to do with this to not over engineer simple statement
There was a problem hiding this comment.
I guess we can just do !unsafeHtml? @Johann-S what do you prefer?
| if (!unsafeHtml.length) { | ||
| return unsafeHtml | ||
| if (!unsafeHtml || !unsafeHtml.length) { | ||
| return unsafeHtml || '' |
There was a problem hiding this comment.
Any specific reason to return an empty string instead of false?
There was a problem hiding this comment.
As there is no JSDoc, I tried to read intent from:
- empty string (String.lenght) returns empty string
sanitizeFnfrom docs recommends DOMPurify, which returns empty string always (we are using DOMPurify in one product, pretty sure here)- the return from the function itself, here is the string (
innerHtml)
|
That issue is for v3 BTW. @Johann-S we should edit to mention if it happens in v4 and v5 (remove 3 since it's EOL) and mark this for backport |
XhmikosR
changed the title
|
It seems it's impossible to pass |
|
Closing due to lack of response. You can comment here and we can re-open the PR, if you decide to resume working on the requested changes. This is an automated reply |
When the input of sanitizer evaluates to false:
With that change the falsy input like null, undefined or empty string
should return the same results: empty string.
Thanks!
Fixes #29474