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

add socks5h proxy support #3178

Merged
merged 3 commits into from
Oct 29, 2024
Merged

Conversation

liubin
Copy link
Contributor

@liubin liubin commented Apr 25, 2024

In some case socks5h is needed to resolve the hostname.

Summary

add socks5h proxy support.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

In some case socks5h is needed to resolve the hostname.

Signed-off-by: bin liu <[email protected]>
@T-256
Copy link
Contributor

T-256 commented Apr 25, 2024

socks5 are resolving hostnames in remote side by default (socks5h).
Also, see httpx-socks where you can explicitly set rdns=False to locally resolving.

@liubin
Copy link
Contributor Author

liubin commented Apr 26, 2024

@T-256 Indeed I run into a complexity case that socks5 not work:

12:39:33-bin~/tmppp\ -> export https_proxy=socks5h://127.0.0.1:15666
12:39:49-bin~/tmppp\ -> curl -I https://twitter.com
HTTP/2 403 
date: Fri, 26 Apr 2024 04:39:51 GMT
server: tsa_p
x-powered-by: Express
... ...

12:39:51-bin~/tmppp\ -> export https_proxy=socks5://127.0.0.1:15666
12:39:56-bin~/tmppp\ -> curl -I https://twitter.com
curl: (35) LibreSSL SSL_connect: SSL_ERROR_SYSCALL in connection to twitter.com:443 

And my code with requests only work with socks5h too, if set to socks5, it will get an error:

  File "/Users/bin/Library/Python/3.9/lib/python/site-packages/requests/adapters.py", line 517, in send
    raise SSLError(e, request=request)
requests.exceptions.SSLError: (MaxRetryError("SOCKSHTTPSConnectionPool(host='huggingface.co', port=443): Max retries exceeded with url: /BAAI/bge-small-en-v1.5/resolve/main/config.json (Caused by SSLError(SSLEOFError(8, 'EOF occurred in violation of protocol (_ssl.c:1129)')))"), '(Request ID: a0fdd8ed-dd99-4b38-bf53-47f133004e22)')

My code using both requests and httpx, it is not controlled by me, but be the packages that I'm using directly.

So let httpx to support socks5h is the simplest way for my case.

@tomchristie
Copy link
Member

Thanks, yep we probably should have socks5h proxy support.

Have you tested the above? I don't think it'll work, although you'll be able to pass socks5h:// through, support for it isn't actually implemented in httpcore yet.

Here's where you'll actually need to start on this...

@mateuszlitwin
Copy link

mateuszlitwin commented Sep 13, 2024

Hello @tomchristie! I am also running into this issue, i.e. lack of socks5h:// support. I cannot use socks5:// because it is interpreted differently by other tools (e.g. curl). I did test this PR + 1 line change in https://github.com/encode/httpcore/blob/master/httpcore/_models.py#L294 to add default port for sock5h and it works as expected - traffic is sent to the proxy without resolving hostname.

@longouyang
Copy link

Hi @tomchristie - firstly, thanks so much for all the work you've done on this project. We sure use the library a lot at $DAYJOB.

Also - I'd be happy to pitch in $250 support to the project if we could get socks5h support merged.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, thanks! Does also need an httpcore release in order to work all the way through.

@tomchristie tomchristie merged commit 12be5c4 into encode:master Oct 29, 2024
5 checks passed
@hawktang
Copy link

hawktang commented Nov 1, 2024

link from #3386

@hawktang
Copy link

hawktang commented Nov 1, 2024

When will the change for httpx and http core release to pip?

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.

6 participants