Skip to content

CB-4743 adds html sanitizer#2420

Merged
Wroud merged 5 commits intodevelfrom
CB-4743-remove-all-dangerously-set-inner-html-properties
Mar 5, 2024
Merged

CB-4743 adds html sanitizer#2420
Wroud merged 5 commits intodevelfrom
CB-4743-remove-all-dangerously-set-inner-html-properties

Conversation

@sergeyteleshev
Copy link
Copy Markdown
Contributor

No description provided.

@sergeyteleshev sergeyteleshev self-assigned this Feb 27, 2024
Comment thread webapp/packages/core-utils/src/sanitizeHtml.test.tsx Outdated
@sergeyteleshev
Copy link
Copy Markdown
Contributor Author

tests are okay. I just changed the file name and job here went crazy cause it's left in dist library

* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
export function toSafeHtmlString(dirty: string): string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably better to use lib like dompurify to sanitize html

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this was my first solution. @Wroud suggested to generate a html safe string
in this case we kill 2 rabbits:

  • the code is not executable by browser
  • the user will see that something illegal happened in his JSON tab editor and start investigation what happened

image
image

@Wroud Wroud merged commit 9934ed9 into devel Mar 5, 2024
@Wroud Wroud deleted the CB-4743-remove-all-dangerously-set-inner-html-properties branch March 5, 2024 19:14
sergeyteleshev added a commit that referenced this pull request Mar 6, 2024
* CB-4743 adds html sanitizer

* CB-4743 adds license to sanitizeHtml

* CB-4743 do not use sanitize to purify the json line data

* CB-4743 fix: toSafeHtmlString test correct cases

---------

Co-authored-by: s.teleshev <s.teleshev@mu.se>
Co-authored-by: mr-anton-t <42037741+mr-anton-t@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants