Skip to content

Conversation

@drshvik
Copy link

@drshvik drshvik commented Jan 30, 2026

Summary

This PR fixes a critical issue where SOCKSProxy ignores the configured connect timeout during the SOCKS5 handshake.

Previously, the timeout was used to establish the initial TCP connection, but it was not passed to the _init_socks5_connection function. As a result, if a proxy server accepted the TCP connection but failed to send data (e.g., a "blackhole" server), the client would hang indefinitely (or until the OS socket timeout) regardless of the user's configured timeout.

The Fix

  • Updated _init_socks5_connection in both _sync/socks_proxy.py and _async/socks_proxy.py to accept a timeout argument.
  • Passed this timeout explicitly to all stream.read() and stream.write() calls during the authentication and connection negotiation phases.
  • Updated Socks5Connection.handle_request (and its async counterpart) to extract the connect timeout from request.extensions and pass it to the initialization function.

Reproduction

I verified this fix using a local reproduction script that simulates a hanging SOCKS5 proxy.

  • Before: The client hung for >20s (ignoring the 2s timeout).
  • After: The client correctly raises TimeoutException after 2s.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion.
  • I've verified the fix with reproduction scripts (see above).
  • I've updated the documentation accordingly. (Not applicable for this bug fix)

@drshvik
Copy link
Author

drshvik commented Jan 30, 2026

All checks are passing and I'm happy to make any changes if needed. Thanks!

@drshvik
Copy link
Author

drshvik commented Jan 30, 2026

I noticed this appears to address the same issue as #1017. I've submitted this fresh PR because I verified the bug still exists on the current main branch, and I ensured this version passes all current linting and test checks. Happy to supersede the older PR if that one is inactive.

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.

1 participant