refactor: [M3-7990] - Replace sanitize-html with DOM friendly alternative #10378
Conversation
abailly-akamai
changed the title
abailly-akamai
requested review from
dwiley-akamai and
jaalah-akamai
and removed request for
a team
| 'title', | ||
| 'align', | ||
| 'class', | ||
| 'rel', |
There was a problem hiding this comment.
We allow them, but then we strip them all unless it is for <span class="version" />
This will be all cleaned up once we get rid of out eventMessageGenerator transformers, which is part of this epic
| }, | ||
| sanitizingTier: 'strict', | ||
| text: localError || '', | ||
| }).toString(); |
There was a problem hiding this comment.
we need to satisfy the types here with toString()
would love to confirm those changes are good with people familiar with this particular Firewall files. Maybe @carrillo-erik ?
| KEEP_CONTENT: disallowedTagsMode === 'discard' ? false : true, | ||
| RETURN_DOM: false, | ||
| RETURN_DOM_FRAGMENT: false, | ||
| RETURN_TRUSTED_TYPE: false, |
There was a problem hiding this comment.
RETURN_TRUSTED_TYPE: false,
see https://github.com/cure53/DOMPurify?tab=readme-ov-file#what-about-dompurify-and-trusted-types
| node.removeAttribute('class'); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
legacy handlers which I hope to get rid of soon once we refactor event generators
|
Coverage Report: ✅ |
Description 📝
This PR replaces
sanitize-htmlwith the DOM friendly, well supporteddompurifypackage.https://github.com/cure53/DOMPurify
sanitize-htmlis meant to run with NodeJS - aka on the server. This substitution standardizes our codebase and avoid potential issues running nodeJS modules (path,FS,url,source-map-js) in the browser while improving the developer experience as we're removing a large amount of warnings in the console.This PR is meant as a 1/1 replacement, meaning all tests should be passing and current sanitization in CM should be unaltered.
The bundled compressed size difference between both packages is negligible.
ℹ️ Note: As a rule of thumb, sanitizing in the browser is only for the presentation layer. We don't want to get in the habit of sanitizing data we're sending to the API.
Changes 🔄
List any change relevant to the reviewer.
sanitize-htmlwithdompurify.Preview 📷
No visual change
How to test 🧪
Prerequisites
yarn && yarn upwith current branchVerification steps
In order to check some of the areas where we sanitize HTML, use the content matches in the table below. It sahould give you a pretty good idea what area to check. Namely, events (notification dropdown & event page), tickets,
sanitizeHTMLutilHighlightedMarkdownComponentAs an Author I have considered 🤔
Check all that apply