-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(brotli) remove re2 dependency #1813
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Hi @titanism - can we merge it? It will help to keep support for Node 14 as well |
It looks like this causes problems on some architectures, so we may need to look at an alternative. At a glance, it seems like the regular expressions here are actually safe: Lines 121 to 139 in 134cdd7
> require('safe-regex2')(/^\s*(?:deflate|gzip)\s*$/)
true
> require('safe-regex2')(/^\s*(?:br)\s*$/)
true So we may not need RE2 after all. Though this might need tested. |
@titanism - I removed re2. we have unit tests around that area |
superagent v10.0.1 is released (we removed https://github.com/ladjs/superagent/releases/tag/v10.0.1 |
Use a fixed version of re2 to maintain support for Node14 (now need 14.21.3)
Checklist