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

fix: reqwest can not use SOCKS proxy #311

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nihao
Copy link

@nihao nihao commented Feb 19, 2025

#310

  • enable feature of reqwest to support socks proxy

0xPlaygrounds#310

* enable feature of reqwest to support socks proxy
@0xMochan
Copy link
Contributor

Hey, thanks for this PR! Is there more details you can describe about your use case with SOCK proxies? I could see this being used for an optional feature or a default feature based on the use case!

@nihao
Copy link
Author

nihao commented Feb 20, 2025

Inside our datacenter, the servers cannot access the internet directly. Instead, we provide a SOCKS proxy by default because it offers better speed and stability.

Maybe you can set it as an optional feature, but it will require a convenient configuration method

@joshua-mo-143
Copy link
Contributor

I think the most ideal way to do this would be to add it as a feature since otherwise, it'll be a breaking change and it would be ideal to keep core dependencies low.

@nihao
Copy link
Author

nihao commented Mar 4, 2025

hi @0xMochan , when will you merge this contribute to the project

@joshua-mo-143
Copy link
Contributor

joshua-mo-143 commented Mar 4, 2025

Hi, Mochan's currently OOO at the moment.

If you can set up the socks feature as a separate feature for rig-core rather than including it in the library by default, I'd be happy to merge it. In the interest of ensuring the core library is as lean as possible, it'd be preferred to enable users to opt into features rather than opting out.

add socks as an option feature
@nihao nihao force-pushed the fix/reqwest-socks-support branch from cf2446c to 21f01ed Compare March 5, 2025 01:37
@nihao
Copy link
Author

nihao commented Mar 5, 2025

@joshua-mo-143 pls check now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants