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 #1033 #1037

Merged
merged 4 commits into from
Nov 29, 2024
Merged

Fix #1033 #1037

merged 4 commits into from
Nov 29, 2024

Conversation

svdb99
Copy link
Contributor

@svdb99 svdb99 commented Nov 26, 2024

Summary

Small fix for issue #1033

Background & Context

const TMPLIT_EXPR = seal(/\${[\w\W]*}/gm);

Projects that use XSLT can inline 3rd party libs before pushing out the response to the browser. While doing so they can perform code resolution/substitutions on ${} espressions. Unfortunately the ${[\w\W]*} part can be misinterpreted as an expression even though it's in a regexp, and because it obviously doesn't resolve to anything during the transformation, it gets wiped out as a result and causes a syntax error when the browser gets it.
Result: const TMPLIT_EXPR = seal(/\/gm);

Proposed solution

The solution is extremely simple and doesn't change anything to how the regexp operates as it merely escapes the first curly brace.

@svdb99
Copy link
Contributor Author

svdb99 commented Nov 28, 2024

The Lint error wants to optimize the regexp by removing the escape character I added.
Well, no. This "optimization" is precisely what caused the issue in the first place.

@cure53
Copy link
Owner

cure53 commented Nov 28, 2024

Feel free to silence it with a eslint-disable :D

@cure53
Copy link
Owner

cure53 commented Nov 29, 2024

We are planning a release later today, are you planning to get this change in and amend the PR? :)

@svdb99
Copy link
Contributor Author

svdb99 commented Nov 29, 2024

Done.

@svdb99
Copy link
Contributor Author

svdb99 commented Nov 29, 2024

What's your time zone? I'm in CET (UTC+1)

@cure53
Copy link
Owner

cure53 commented Nov 29, 2024

What's your time zone? I'm in CET (UTC+1)

Same

@cure53
Copy link
Owner

cure53 commented Nov 29, 2024

This is again breaking the tests and we have already invested a lot off useless effort into adding one character, I would really recommend that you don't blindly commit stuff but actually run the linter before.

The comment you want to add is this: // eslint-disable-line unicorn/better-regex

@cure53
Copy link
Owner

cure53 commented Nov 29, 2024

Thanks, this looks better and tests don't die on init this time. In case all good, we will merge and then release 3.2.2

@cure53 cure53 merged commit 3b4b5e9 into cure53:main Nov 29, 2024
8 checks passed
@svdb99
Copy link
Contributor Author

svdb99 commented Nov 29, 2024

Sorry, I'm a security analyst not a software developer nor lint expert, and I'm not allowed to install any of your tools on my laptop, so I did indeed copy the same comment from the next line assuming it would be the same. I usually only test new library versions for security breaches and upgrade them in our products when needed (we use over 1800 different libraries). But for DomPurify it was impossible due to this single character.
Thank you for your understanding.

@cure53
Copy link
Owner

cure53 commented Nov 29, 2024

No worries, we managed after all 🙂

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.

2 participants