Fix socket timeout value when HTTPConnection is reused
ADD
changelog/2645.bugfix.rst
+Fixed an issue where an ``HTTPConnection`` instance would erroneously reuse the socket
+read timeout value from reading the previous response instead of a newly configured connect timeout.
+Instead now if ``HTTPConnection.timeout`` is updated before sending the next request the new timeout value will be used.

MODIFY
src/urllib3/connection.py
src/urllib3/connection.py
@@ -330,6 +330,11 @@ class HTTPConnection(_HTTPConnection):
enforce_content_length: bool = True,
) -> None:
+ # Update the inner socket's timeout value to send the request.
+ # This only triggers if the connection is re-used.
+ if self.sock is not None:
+ self.sock.settimeout(self.timeout)
+
# Store these values to be fed into the HTTPResponse
# object later. TODO: Remove this in favor of a real
# HTTP lifecycle mechanism.

MODIFY
src/urllib3/connectionpool.py
src/urllib3/connectionpool.py
@@ -57,7 +57,7 @@ if TYPE_CHECKING:
log = logging.getLogger(__name__)
-_TYPE_TIMEOUT = Union[Timeout, float, _TYPE_DEFAULT]
+_TYPE_TIMEOUT = Union[Timeout, float, _TYPE_DEFAULT, None]
_SelfT = TypeVar("_SelfT")
@@ -460,7 +460,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
timeout_obj = self._get_timeout(timeout)
timeout_obj.start_connect()
- conn.timeout = timeout_obj.connect_timeout # type: ignore[assignment]
+ conn.timeout = Timeout.resolve_default_timeout(timeout_obj.connect_timeout)
try:
# Trigger any extra validation we need to do.

MODIFY
test/with_dummyserver/test_connectionpool.py
test/with_dummyserver/test_connectionpool.py
@@ -6,7 +6,7 @@ import time
import warnings
from test import LONG_TIMEOUT, SHORT_TIMEOUT
from threading import Event
-from typing import Dict, List, NoReturn, Optional, Tuple, Type, Union
+from typing import Dict, List, NoReturn, Optional, Sequence, Tuple, Type, Union
from unittest import mock
from urllib.parse import urlencode
@@ -30,7 +30,7 @@ from urllib3.exceptions import (
from urllib3.fields import _TYPE_FIELD_VALUE_TUPLE
from urllib3.util import SKIP_HEADER, SKIPPABLE_HEADERS
from urllib3.util.retry import RequestHistory, Retry
-from urllib3.util.timeout import Timeout
+from urllib3.util.timeout import _TYPE_TIMEOUT, Timeout
from .. import INVALID_SOURCE_ADDRESSES, TARPIT_HOST, VALID_SOURCE_ADDRESSES
from ..port_helpers import find_unused_port
@@ -386,6 +386,59 @@ class TestConnectionPool(HTTPDummyServerTestCase):
with HTTPConnectionPool(self.host, self.port, timeout=timeout) as pool:
pool.request("GET", "/")
+ socket_timeout_reuse_testdata = pytest.mark.parametrize(
+ ["timeout", "expect_settimeout_calls"],
+ [
+ (1, (1, 1)),
+ (None, (None, None)),
+ (Timeout(read=4), (None, 4)),
+ (Timeout(read=4, connect=5), (5, 4)),
+ (Timeout(connect=6), (6, None)),
+ ],
+ )
+
+ @socket_timeout_reuse_testdata
+ def test_socket_timeout_updated_on_reuse_constructor(
+ self, timeout: _TYPE_TIMEOUT, expect_settimeout_calls: Sequence[Optional[float]]
+ ) -> None:
+ with HTTPConnectionPool(self.host, self.port, timeout=timeout) as pool:
+ # Make a request to create a new connection.
+ pool.urlopen("GET", "/")
+
+ # Grab the connection and mock the inner socket.
+ assert pool.pool is not None
+ conn = pool.pool.get_nowait()
+ conn_sock = mock.Mock(wraps=conn.sock)
+ conn.sock = conn_sock
+ pool._put_conn(conn)
+
+ # Assert that sock.settimeout() is called with the new connect timeout, then the read timeout.
+ pool.urlopen("GET", "/", timeout=timeout)
+ conn_sock.settimeout.assert_has_calls(
+ [mock.call(x) for x in expect_settimeout_calls]
+ )
+
+ @socket_timeout_reuse_testdata
+ def test_socket_timeout_updated_on_reuse_parameter(
+ self, timeout: _TYPE_TIMEOUT, expect_settimeout_calls: Sequence[Optional[float]]
+ ) -> None:
+ with HTTPConnectionPool(self.host, self.port) as pool:
+ # Make a request to create a new connection.
+ pool.urlopen("GET", "/", timeout=LONG_TIMEOUT)
+
+ # Grab the connection and mock the inner socket.
+ assert pool.pool is not None
+ conn = pool.pool.get_nowait()
+ conn_sock = mock.Mock(wraps=conn.sock)
+ conn.sock = conn_sock
+ pool._put_conn(conn)
+
+ # Assert that sock.settimeout() is called with the new connect timeout, then the read timeout.
+ pool.urlopen("GET", "/", timeout=timeout)
+ conn_sock.settimeout.assert_has_calls(
+ [mock.call(x) for x in expect_settimeout_calls]
+ )
+
def test_tunnel(self) -> None:
# note the actual httplib.py has no tests for this functionality
timeout = Timeout(total=None)

Fix socket timeout value when HTTPConnection is reused