Propagate timeout through SOCKS5 handshake#1009
Conversation
During SOCKS5 connection setup, all reads and writes to the proxy socket were called without a timeout. A non-responsive proxy would block the calling thread/task indefinitely regardless of any `timeout` configured on the request. Thread the `connect` timeout (already used for the initial TCP connect) through `_init_socks5_connection()` to every `stream.read()` and `stream.write()` call. Ported from encode/httpcore#1055 (drshvik), via codeberg.org/httpxyz/httpcorexyz@0387930. Co-Authored-By: drshvik <ai23btech11004@iith.ac.in>
Merging this PR will not alter performance
Comparing Footnotes
|
Kludex
left a comment
There was a problem hiding this comment.
This doesn't seem correct, is it? We are using the connect timeout for everything.
I think before addressing this we need to make httpcore2 a bit more type safe in the Extensions type - maybe starting by splitting it in RequestExtensions and ResponseExtensions.
That's reasonable! Do you pick that up or would you like me to create a PR? |
|
You can do it. You'll probably need extra_items and total=False, so we probably need to include typing extensions. Change httpcore2 only if possible. |
Summary
During SOCKS5 connection setup, all reads and writes to the proxy socket were called without a timeout. A non-responsive SOCKS5 proxy (one that accepts the TCP connection but never sends a handshake reply) would block the calling thread/task indefinitely, regardless of any
timeoutconfigured on the request.This PR threads the
connecttimeout (already used for the initial TCP connect) through_init_socks5_connection()to everystream.read()/stream.write()call during auth method negotiation, optional username/password auth, and the SOCKS5 CONNECT request.Ports encode/httpcore#1055 (drshvik), via codeberg.org/httpxyz/httpcorexyz@0387930.
Notes
_async/socks_proxy.pywas edited by hand;_sync/socks_proxy.pywas regenerated byscripts/unasync.py.MockBackend-based SOCKS5 tests already exercise the new code path withtimeout=None. 100% coverage is preserved.Note: this change was prepared with AI assistance (Claude Code).