-
Notifications
You must be signed in to change notification settings - Fork 605
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
ProxyAgent: support socks5 protocol #2224
Comments
This would be amazing, but it'd also be somewhat feasible to create it outside of this module. I recommend that route at this point as we are stretched really thin. |
It would be best if it could be integrated in this module. |
PRs are welcomed. |
have you tried https://github.com/Kaciras/fetch-socks? |
https://github.com/Kaciras/fetch-socks/blob/master/package.json#L34 This package depends on an undici. So I don't think it's ideal. |
the caret means it supports undici versions of at least 5.22.1 |
When I use undici, it installs one more on its side, which will now will exist two undici. |
you can probably use dedupe |
Wouldn't it be simpler to make a buildConnector for each type of proxy and use it as a connector?
Or you can do it the same way as done by aiohttp in python (https://github.com/Slzdude/aiohttp-proxy) |
Also after implementation SOCKS5 - we have got CRITICAL Vulnerability ❌ CVE-2023-38545 which affected on the security. |
If no one is working on it I am interested in picking it up, as I am working on nodejs/node#57165 I think it would be a great addition for Node.js built fetch to work more seamlessly with socks 5 proxies. The lack of support of it was one of the reasons I added the local README config in node-core-utils as a workaround (because if you try to fetch github raw content within Chinese GFW you get connection issues). It would also help closing nodejs/node-core-utils#653 and overall making Node.js easier to use for developers in China (socks5 is very popular in China). |
This would solve...
Using the socks5 proxy.
The implementation should look like...
I have also considered...
Additional context
The text was updated successfully, but these errors were encountered: