Skip to content

Conversation

yigaldviri
Copy link
Contributor

Use a fixed version of re2 to maintain support for Node14 (now need 14.21.3)

Checklist

  • [V] I have ensured my pull request is not behind the main or master branch of the original repository.
  • [V] I have rebased all commits where necessary so that reviewing this pull request can be done without having to merge it first.
  • [V] I have written a commit message that passes commitlint linting.
  • [V] I have ensured that my code changes pass linting tests.
  • [V] I have ensured that my code changes pass unit tests.
  • [V] I have described my pull request and the reasons for code changes along with context if necessary.

Copy link

socket-security bot commented Aug 6, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

🚮 Removed packages: npm/[email protected]

View full report↗︎

Copy link

socket-security bot commented Aug 6, 2024

👍 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.

View full report↗︎

@yigaldviri
Copy link
Contributor Author

Hi @titanism - can we merge it? It will help to keep support for Node 14 as well

@titanism
Copy link
Collaborator

It looks like this causes problems on some architectures, so we may need to look at an alternative.

#1814

At a glance, it seems like the regular expressions here are actually safe:

https://github.com/ladjs/superagent/blob/134cdd7c4fd57868c80102826507047da23c34aa/src/utils.js#L121-L139

> 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.

@yigaldviri yigaldviri changed the title fix(brotli) use a fixed version of re2 to support Node 14 fix(brotli) remove re2 dependency Aug 11, 2024
@yigaldviri
Copy link
Contributor Author

@titanism - I removed re2. we have unit tests around that area

@titanism
Copy link
Collaborator

superagent v10.0.1 is released (we removed re2 dependency)

https://github.com/ladjs/superagent/releases/tag/v10.0.1

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