tomayac / local-reverse-geocoder

Local reverse geocoder for Node.js based on GeoNames data
Apache License 2.0
190 stars 60 forks source link

Replace `request` with `got-cjs` #73

Closed yukha-dw closed 1 year ago

yukha-dw commented 1 year ago

Hello, is it possible to replace request with maintained HTTP Request Library? request has vulnerability and already on EOL. The easiest lib to become an alternative that I can think of is got. It has same stream method but no longer support CommonJS.

Vulnerability on request lib: https://github.com/request/request/issues/3442

Edit: Nevermind, got also has this vulnerability :/ I think the solution is to add SSRF Filter to the client

socket-security[bot] commented 1 year ago

New dependency changes detected. Learn more about Socket for GitHub ā†—ļøŽ


šŸ‘ No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts āœ… 0 issues
Native code āœ… 0 issues
Bin script shell injection āœ… 0 issues
Unresolved require āœ… 0 issues
Invalid package.json āœ… 0 issues
HTTP dependency āœ… 0 issues
Git dependency āœ… 0 issues
Potential typo squat āœ… 0 issues
Known Malware āœ… 0 issues
Telemetry āœ… 0 issues
Protestware/Troll package āœ… 0 issues

šŸ“Š Modified Dependency Overview:

āž• Added Package Capability Access +/- Transitive Count Publisher
got-cjs@12.5.4 network, filesystem +25 mnmkng

šŸš® Removed packages: request@2.88.2

tomayac commented 1 year ago

Hi there. Thanks for the PR. Iā€™m all for removing no longer maintained dependencies. Rather than introduce a new one that might end up unmaintained tomorrow, why not make the switch to the native fetch(), which during a transition period we could back by node-fetch()? What do you think?

yukha-dw commented 1 year ago

I think so, node-fetch would be a solid option too.

tomayac commented 1 year ago

Feel free to reopen this once you have the code ready. Thanks for your help!

yukha-dw commented 1 year ago

Feel free to reopen this once you have the code ready. Thanks for your help!

got it. i'll try when i have time!