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

Replace the request package by something else (DEPRECATED) #535

Open
mahmoud-triki opened this issue Mar 14, 2023 · 13 comments
Open

Replace the request package by something else (DEPRECATED) #535

mahmoud-triki opened this issue Mar 14, 2023 · 13 comments

Comments

@mahmoud-triki
Copy link

Description/Steps to reproduce

Here you find the deprecation warning, it has been deprecated for more than 3 years.

you can do npm install and you can see the deprecation warning.

Link to reproduction sandbox

https://www.npmjs.com/package/request

Expected result

Not using a deprecated package
This can be replaced by axios.

Additional information

@villelahdenvuo
Copy link

villelahdenvuo commented Mar 17, 2023

Related: GHSA-p8p7-x288-28g6

I think replacement should be node-fetch and for Node 18+ it should be optional since fetch is built in Node 18+.

@mahmoud-triki
Copy link
Author

That's a good callout

@villelahdenvuo
Copy link

@dhmlau Hey, as this is a Moderate severity security issue that shows up on npm audit I wanted to make sure you see it. Seems like changing the http client to use fetch wouldn't require that much work.

@dhmlau
Copy link
Member

dhmlau commented Mar 22, 2023

UPDATED to clarify to suggest fetch that is built-in for Node.js 18.

@villelahdenvuo, I agree to use node-fetch fetch API in Node.js. Would you like to submit a PR?

When we're making this change, we'd also need to update the minimum Node.js version required in this module and it will be a breaking change.

@villelahdenvuo
Copy link

villelahdenvuo commented Mar 27, 2023

@dhmlau We could keep backwards compatibility until Node 12 if we use https://github.com/nodejs/undici

Edit: Actually it says "Only supported on Node 16.8+." https://github.com/nodejs/undici#undicifetchinput-init-promise

@villelahdenvuo
Copy link

@dhmlau For us this is not a direct security threat as we only use SOAP to integrate with an internal system, so we don't expect our other team to hack us. 😄 If I have some free time I can take a look at it.

@mahmoud-triki Would you have some time to look at replacing the library with fetch?

@s100
Copy link
Contributor

s100 commented May 15, 2023

Please note that strong-soap also makes use of request indirectly, via httpntlm-maa. I have raised an issue against httpntlm-maa, but the package has not been maintained for over three years, and I do not expect it to be fixed. So, please could you also consider stopping using httpntlm-maa or migrating away from it to something else.

@Timpan4
Copy link

Timpan4 commented Jun 13, 2023

Any updates on this matter?

@sseide
Copy link
Contributor

sseide commented Jun 23, 2023

Please note that strong-soap also makes use of request indirectly, via httpntlm-maa. I have raised an issue against httpntlm-maa, but the package has not been maintained for over three years, and I do not expect it to be fixed. So, please could you also consider stopping using httpntlm-maa or migrating away from it to something else.

@s100 Are you sure NPM releases are the version from maa105 and not the original one from Sam Decrock? https://github.com/SamDecrock/node-http-ntlm Maybe you created the ticket at the wrong repo...
But this repo non the less has no releases for a long time too - BUT Readme was updated "recently" with a new donation.

@s100
Copy link
Contributor

s100 commented Jun 23, 2023

@s100 Are you sure NPM releases are the version from maa105 and not the original one from Sam Decrock?

Yes, I'm sure. Here's strong-soap's dependency on httpntlm-maa:

https://github.com/loopbackio/strong-soap/blob/4a5a125e26ec70f10f9084e95d634ae1e4401de4/package.json#L11C6-L11C18

Here's the npm page for the httpntlm-maa package. Here's the repo link on that page:

image

And that link goes here.

@sseide
Copy link
Contributor

sseide commented Jun 23, 2023

@s100 Are you sure NPM releases are the version from maa105 and not the original one from Sam Decrock?

Yes, I'm sure. Here's strong-soap's dependency on httpntlm-maa:

https://github.com/loopbackio/strong-soap/blob/4a5a125e26ec70f10f9084e95d634ae1e4401de4/package.json#L11C6-L11C18

Here's the npm page for the httpntlm-maa package. Here's the repo link on that page:

image

And that link goes here.

oh - thanks. Need more coffee probably. I missed the small difference in the package name and only seen it is a fork of the other one with very similar readme file for booth packages...

Maybe the original one from Sam Decrock might be a possible replacement as development started again this year and it does not depend on "request" library. But contrary to the library from mma105 it does not support promisses, only callback style.

@ktj
Copy link

ktj commented Nov 16, 2023

Consider using undici instead of fetch.
Fetch api is very limited since it's the same api that browsers implement.

@s100
Copy link
Contributor

s100 commented Sep 20, 2024

This issue is fixed and can now be closed.

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

No branches or pull requests

7 participants