From a402038637408453e2aadb6fcd5d97dfbeb6b52a Mon Sep 17 00:00:00 2001 From: wwmanidumaneesha Date: Sat, 25 Jul 2026 17:58:48 +0530 Subject: [PATCH] Apply the connect timeout to the SOCKS5 handshake 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 #1015, closes #1089. --- CHANGELOG.md | 1 + httpcore/_async/socks_proxy.py | 14 ++--- httpcore/_sync/socks_proxy.py | 14 ++--- tests/_async/test_socks_proxy.py | 91 ++++++++++++++++++++++++++++++++ tests/_sync/test_socks_proxy.py | 91 ++++++++++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5761a3cd..434fdc5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased - Fix `max_keepalive_connections` not being properly handled. (#1000) +- Apply the connect timeout to the SOCKS5 handshake, not only to the TCP connection to the proxy. (#1015, #1089) ## Version 1.0.9 (April 24th, 2025) diff --git a/httpcore/_async/socks_proxy.py b/httpcore/_async/socks_proxy.py index b363f55a..ce9b00ec 100644 --- a/httpcore/_async/socks_proxy.py +++ b/httpcore/_async/socks_proxy.py @@ -45,6 +45,7 @@ async def _init_socks5_connection( host: bytes, port: int, auth: tuple[bytes, bytes] | None = None, + timeout: float | None = None, ) -> None: conn = socksio.socks5.SOCKS5Connection() @@ -56,10 +57,10 @@ async def _init_socks5_connection( ) conn.send(socksio.socks5.SOCKS5AuthMethodsRequest([auth_method])) outgoing_bytes = conn.data_to_send() - await stream.write(outgoing_bytes) + await stream.write(outgoing_bytes, timeout=timeout) # Auth method response - incoming_bytes = await stream.read(max_bytes=4096) + incoming_bytes = await stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5AuthReply) if response.method != auth_method: @@ -75,10 +76,10 @@ async def _init_socks5_connection( username, password = auth conn.send(socksio.socks5.SOCKS5UsernamePasswordRequest(username, password)) outgoing_bytes = conn.data_to_send() - await stream.write(outgoing_bytes) + await stream.write(outgoing_bytes, timeout=timeout) # Username/password response - incoming_bytes = await stream.read(max_bytes=4096) + incoming_bytes = await stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5UsernamePasswordReply) if not response.success: @@ -91,10 +92,10 @@ async def _init_socks5_connection( ) ) outgoing_bytes = conn.data_to_send() - await stream.write(outgoing_bytes) + await stream.write(outgoing_bytes, timeout=timeout) # Connect response - incoming_bytes = await stream.read(max_bytes=4096) + incoming_bytes = await stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5Reply) if response.reply_code != socksio.socks5.SOCKS5ReplyCode.SUCCEEDED: @@ -237,6 +238,7 @@ async def handle_async_request(self, request: Request) -> Response: "host": self._remote_origin.host.decode("ascii"), "port": self._remote_origin.port, "auth": self._proxy_auth, + "timeout": timeout, } async with Trace( "setup_socks5_connection", logger, request, kwargs diff --git a/httpcore/_sync/socks_proxy.py b/httpcore/_sync/socks_proxy.py index 0ca96ddf..391eef78 100644 --- a/httpcore/_sync/socks_proxy.py +++ b/httpcore/_sync/socks_proxy.py @@ -45,6 +45,7 @@ def _init_socks5_connection( host: bytes, port: int, auth: tuple[bytes, bytes] | None = None, + timeout: float | None = None, ) -> None: conn = socksio.socks5.SOCKS5Connection() @@ -56,10 +57,10 @@ def _init_socks5_connection( ) conn.send(socksio.socks5.SOCKS5AuthMethodsRequest([auth_method])) outgoing_bytes = conn.data_to_send() - stream.write(outgoing_bytes) + stream.write(outgoing_bytes, timeout=timeout) # Auth method response - incoming_bytes = stream.read(max_bytes=4096) + incoming_bytes = stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5AuthReply) if response.method != auth_method: @@ -75,10 +76,10 @@ def _init_socks5_connection( username, password = auth conn.send(socksio.socks5.SOCKS5UsernamePasswordRequest(username, password)) outgoing_bytes = conn.data_to_send() - stream.write(outgoing_bytes) + stream.write(outgoing_bytes, timeout=timeout) # Username/password response - incoming_bytes = stream.read(max_bytes=4096) + incoming_bytes = stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5UsernamePasswordReply) if not response.success: @@ -91,10 +92,10 @@ def _init_socks5_connection( ) ) outgoing_bytes = conn.data_to_send() - stream.write(outgoing_bytes) + stream.write(outgoing_bytes, timeout=timeout) # Connect response - incoming_bytes = stream.read(max_bytes=4096) + incoming_bytes = stream.read(max_bytes=4096, timeout=timeout) response = conn.receive_data(incoming_bytes) assert isinstance(response, socksio.socks5.SOCKS5Reply) if response.reply_code != socksio.socks5.SOCKS5ReplyCode.SUCCEEDED: @@ -237,6 +238,7 @@ def handle_request(self, request: Request) -> Response: "host": self._remote_origin.host.decode("ascii"), "port": self._remote_origin.port, "auth": self._proxy_auth, + "timeout": timeout, } with Trace( "setup_socks5_connection", logger, request, kwargs diff --git a/tests/_async/test_socks_proxy.py b/tests/_async/test_socks_proxy.py index 907594a4..1a3aea2c 100644 --- a/tests/_async/test_socks_proxy.py +++ b/tests/_async/test_socks_proxy.py @@ -1,3 +1,5 @@ +import typing + import pytest import httpcore @@ -195,3 +197,92 @@ async def test_socks5_request_incorrect_auth(): assert str(exc_info.value) == "Invalid username/password" assert not proxy.connections + + +class TimeoutRecordingStream(httpcore.AsyncMockStream): + """ + A mock stream which records the timeout applied to each read. + """ + + def __init__( + self, + buffer: typing.List[bytes], + read_timeouts: typing.List[typing.Optional[float]], + ) -> None: + super().__init__(buffer) + self._read_timeouts = read_timeouts + + async def read( + self, max_bytes: int, timeout: typing.Optional[float] = None + ) -> bytes: + self._read_timeouts.append(timeout) + return await super().read(max_bytes, timeout) + + +class TimeoutRecordingBackend(httpcore.AsyncMockBackend): + """ + A mock backend handing out streams which record their read timeouts. + """ + + def __init__( + self, + buffer: typing.List[bytes], + read_timeouts: typing.List[typing.Optional[float]], + ) -> None: + super().__init__(buffer) + self._read_timeouts = read_timeouts + + async def connect_tcp( + self, + host: str, + port: int, + timeout: typing.Optional[float] = None, + local_address: typing.Optional[str] = None, + socket_options: typing.Optional[typing.Iterable[httpcore.SOCKET_OPTION]] = None, + ) -> httpcore.AsyncNetworkStream: + return TimeoutRecordingStream(list(self._buffer), self._read_timeouts) + + +@pytest.mark.anyio +async def test_socks5_connect_timeout_applies_to_handshake(): + """ + The connect timeout must cover the SOCKS5 handshake, and not only the + initial TCP connection to the proxy itself. + + A proxy which accepts the TCP connection but then never replies would + otherwise block the read forever, whatever timeouts were configured. + + See https://github.com/encode/httpcore/issues/1015 + """ + read_timeouts: typing.List[typing.Optional[float]] = [] + network_backend = TimeoutRecordingBackend( + [ + # The initial socks CONNECT + # v5 NOAUTH + b"\x05\x00", + # v5 SUC RSV IP4 127 .0 .0 .1 :80 + b"\x05\x00\x00\x01\xff\x00\x00\x01\x00\x50", + # The actual response from the remote server + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: plain/text\r\n", + b"Content-Length: 13\r\n", + b"\r\n", + b"Hello, world!", + ], + read_timeouts, + ) + + async with httpcore.AsyncConnectionPool( + proxy=httpcore.Proxy("socks5://localhost:8080/"), + network_backend=network_backend, + ) as proxy: + response = await proxy.request( + "GET", + "https://example.com/", + extensions={"timeout": {"connect": 5.0}}, + ) + assert response.status == 200 + + # The two handshake reads, being the auth method reply and the connect + # reply, must both be bounded by the configured connect timeout. + assert read_timeouts[:2] == [5.0, 5.0] diff --git a/tests/_sync/test_socks_proxy.py b/tests/_sync/test_socks_proxy.py index 89ec9fae..ccc0972a 100644 --- a/tests/_sync/test_socks_proxy.py +++ b/tests/_sync/test_socks_proxy.py @@ -1,3 +1,5 @@ +import typing + import pytest import httpcore @@ -195,3 +197,92 @@ def test_socks5_request_incorrect_auth(): assert str(exc_info.value) == "Invalid username/password" assert not proxy.connections + + +class TimeoutRecordingStream(httpcore.MockStream): + """ + A mock stream which records the timeout applied to each read. + """ + + def __init__( + self, + buffer: typing.List[bytes], + read_timeouts: typing.List[typing.Optional[float]], + ) -> None: + super().__init__(buffer) + self._read_timeouts = read_timeouts + + def read( + self, max_bytes: int, timeout: typing.Optional[float] = None + ) -> bytes: + self._read_timeouts.append(timeout) + return super().read(max_bytes, timeout) + + +class TimeoutRecordingBackend(httpcore.MockBackend): + """ + A mock backend handing out streams which record their read timeouts. + """ + + def __init__( + self, + buffer: typing.List[bytes], + read_timeouts: typing.List[typing.Optional[float]], + ) -> None: + super().__init__(buffer) + self._read_timeouts = read_timeouts + + def connect_tcp( + self, + host: str, + port: int, + timeout: typing.Optional[float] = None, + local_address: typing.Optional[str] = None, + socket_options: typing.Optional[typing.Iterable[httpcore.SOCKET_OPTION]] = None, + ) -> httpcore.NetworkStream: + return TimeoutRecordingStream(list(self._buffer), self._read_timeouts) + + + +def test_socks5_connect_timeout_applies_to_handshake(): + """ + The connect timeout must cover the SOCKS5 handshake, and not only the + initial TCP connection to the proxy itself. + + A proxy which accepts the TCP connection but then never replies would + otherwise block the read forever, whatever timeouts were configured. + + See https://github.com/encode/httpcore/issues/1015 + """ + read_timeouts: typing.List[typing.Optional[float]] = [] + network_backend = TimeoutRecordingBackend( + [ + # The initial socks CONNECT + # v5 NOAUTH + b"\x05\x00", + # v5 SUC RSV IP4 127 .0 .0 .1 :80 + b"\x05\x00\x00\x01\xff\x00\x00\x01\x00\x50", + # The actual response from the remote server + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: plain/text\r\n", + b"Content-Length: 13\r\n", + b"\r\n", + b"Hello, world!", + ], + read_timeouts, + ) + + with httpcore.ConnectionPool( + proxy=httpcore.Proxy("socks5://localhost:8080/"), + network_backend=network_backend, + ) as proxy: + response = proxy.request( + "GET", + "https://example.com/", + extensions={"timeout": {"connect": 5.0}}, + ) + assert response.status == 200 + + # The two handshake reads, being the auth method reply and the connect + # reply, must both be bounded by the configured connect timeout. + assert read_timeouts[:2] == [5.0, 5.0]