Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-4743 adds html sanitizer #2420

Merged

Conversation

sergeyteleshev
Copy link
Contributor

No description provided.

@sergeyteleshev sergeyteleshev self-assigned this Feb 27, 2024
@sergeyteleshev
Copy link
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
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
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
1 of 3 checks passed
@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 <[email protected]>
Co-authored-by: mr-anton-t <[email protected]>
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