Skip to content

Apply the connect timeout to the SOCKS5 handshake#1097

Open
wwmanidumaneesha wants to merge 1 commit into
encode:masterfrom
wwmanidumaneesha:fix/socks5-handshake-timeout
Open

Apply the connect timeout to the SOCKS5 handshake#1097
wwmanidumaneesha wants to merge 1 commit into
encode:masterfrom
wwmanidumaneesha:fix/socks5-handshake-timeout

Conversation

@wwmanidumaneesha

Copy link
Copy Markdown

Apply the connect timeout to the SOCKS5 handshake

Closes #1015. Closes #1089.

The problem

The connect timeout is resolved and passed to connect_tcp(), but then dropped for the SOCKS5 handshake that immediately follows:

timeout = timeouts.get("connect", None)
...
kwargs = {"host": ..., "port": ..., "timeout": timeout}   # applied here
stream = await self._network_backend.connect_tcp(**kwargs)

kwargs = {"stream": stream, "host": ..., "port": ..., "auth": ...}   # and not here
await _init_socks5_connection(**kwargs)

_init_socks5_connection() took no timeout parameter at all, so all three of its stream.read(max_bytes=4096) calls were unbounded.

A proxy that completes the TCP handshake and then stops responding therefore blocks forever, whatever timeouts were configured.

The fix

Thread the connect timeout through the handshake, so it covers the reads and writes that talk to the proxy as well as the connection to it.

Tests

test_socks5_connect_timeout_applies_to_handshake records the timeout applied to each read and asserts that the two handshake reads — the auth method reply and the connect reply — are both bounded by the configured connect timeout.

On master it fails with [None, None] == [5.0, 5.0].

The connect timeout was resolved and passed to `connect_tcp()`, but then
omitted from the SOCKS5 handshake that follows. `_init_socks5_connection()`
took no timeout at all, so its reads and writes were unbounded.

A proxy that accepts the TCP connection and then stops responding therefore
blocked forever, whatever timeouts had been configured.

Closes encode#1015, closes encode#1089.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant