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

fix: escape unsafe patterns in elements, comment nodes and processing instructions #18

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

AndrewKushnir
Copy link

@AndrewKushnir AndrewKushnir commented Sep 28, 2023

This commit updates the logic to espace unsafe patterns:

  • for elements that can have raw text: all matching closing tags are escaped
  • for comment nodes: all closing comment tags are escaped
  • for processing instructions: all > symbols are escaped

@AndrewKushnir AndrewKushnir force-pushed the escape_raw_content branch 2 times, most recently from 113c586 to ad21ca6 Compare September 28, 2023 04:49
lib/NodeUtils.js Outdated Show resolved Hide resolved
@AndrewKushnir AndrewKushnir force-pushed the escape_raw_content branch 2 times, most recently from c3bcd83 to 1c8f504 Compare September 29, 2023 18:33
@AndrewKushnir AndrewKushnir force-pushed the escape_raw_content branch 2 times, most recently from 17d7f15 to e4e1c95 Compare September 29, 2023 18:52
@AndrewKushnir AndrewKushnir changed the title fix: escape matching closing tag in raw text fix: escape unsafe patterns in elements, comment nodes and processing instructions Sep 29, 2023
@AndrewKushnir AndrewKushnir force-pushed the escape_raw_content branch 2 times, most recently from aa16603 to 9bf2b98 Compare October 1, 2023 18:13
Copy link

@securityMB securityMB left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for putting this up!

@AndrewKushnir
Copy link
Author

AndrewKushnir commented Oct 1, 2023

@securityMB thanks for the review 👍

FYI I've used your recommendation and also added tests for escaping util functions (in ab8ff69), so it's easier to extend the coverage if needed.

Copy link

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, just some performance related observations. (Totally optional)

lib/NodeUtils.js Outdated Show resolved Hide resolved
lib/NodeUtils.js Outdated Show resolved Hide resolved
lib/NodeUtils.js Outdated Show resolved Hide resolved
… instructions

This commit updates the logic to espace unsafe patterns:

- for elements that can have raw text: all matching closing tags are escaped
- for comment nodes: all closing comment tags are escaped
- for processing instructions: all `>` symbols are excaped
@AndrewKushnir AndrewKushnir removed the request for review from jessicajaniuk October 2, 2023 21:38
@AndrewKushnir
Copy link
Author

@alan-agius4 thanks for reviewing, I've applied the necessary changes.

@AndrewKushnir AndrewKushnir merged commit f2435fe into main Oct 2, 2023
@alan-agius4 alan-agius4 deleted the escape_raw_content branch October 3, 2023 05:01
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.

3 participants