Repository: urllib3/urllib3

Issue:
Investigate removing `object` from the `Union[...]` of many `timeout` types
`object` is only there because of the `socket` module "default" sentinel value which is kind-of an unfortunate hack if you ask me.

It's causing a lot of problems type-wise when going from urllib3 timeout to http.client timeouts, let's take a peek if we can instead remove `object` and add `type: ignore` only where the socket default timeout sentinel is used.

Discussion:
hramezani: Thanks @bluetech for your great review. I've addressed your comments.

@sethmlarson I think now it's ready for final review.

Patch:
diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py
index ba4afcca..daf9e444 100644
--- a/src/urllib3/connection.py
+++ b/src/urllib3/connection.py
@@ -26,6 +26,7 @@ if TYPE_CHECKING:
     from typing_extensions import Literal, NoReturn
 
 from .util.proxy import create_proxy_ssl_context
+from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT, Timeout
 from .util.util import to_bytes, to_str
 
 try:  # Compiled with SSL?
@@ -127,7 +128,7 @@ class HTTPConnection(_HTTPConnection):
         self,
         host: str,
         port: Optional[int] = None,
-        timeout: Optional[float] = connection.SOCKET_GLOBAL_DEFAULT_TIMEOUT,
+        timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
         source_address: Optional[Tuple[str, int]] = None,
         blocksize: int = 8192,
         socket_options: Optional[
@@ -148,7 +149,7 @@ class HTTPConnection(_HTTPConnection):
         super().__init__(
             host=host,
             port=port,
-            timeout=timeout,
+            timeout=Timeout.resolve_default_timeout(timeout),
             source_address=source_address,
             blocksize=blocksize,
         )
@@ -361,7 +362,7 @@ class HTTPSConnection(HTTPConnection):
         key_file: Optional[str] = None,
         cert_file: Optional[str] = None,
         key_password: Optional[str] = None,
-        timeout: Optional[float] = connection.SOCKET_GLOBAL_DEFAULT_TIMEOUT,
+        timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
         ssl_context: Optional["ssl.SSLContext"] = None,
         server_hostname: Optional[str] = None,
         source_address: Optional[Tuple[str, int]] = None,
diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py
index 68015357..3167e6a8 100644
--- a/src/urllib3/connectionpool.py
+++ b/src/urllib3/connectionpool.py
@@ -1,7 +1,6 @@
 import errno
 import logging
 import queue
-import socket
 import sys
 import warnings
 from http.client import HTTPResponse as _HttplibHTTPResponse
@@ -58,7 +57,7 @@ from .util.request import set_file_position
 from .util.response import assert_header_parsing
 from .util.retry import Retry
 from .util.ssl_match_hostname import CertificateError
-from .util.timeout import Timeout
+from .util.timeout import _DEFAULT_TIMEOUT, _TYPE_DEFAULT, Timeout
 from .util.url import Url, _encode_target
 from .util.url import _normalize_host as normalize_host
 from .util.url import parse_url
@@ -71,10 +70,7 @@ if TYPE_CHECKING:
 
 log = logging.getLogger(__name__)
 
-_Default = object()
-
-
-_TYPE_TIMEOUT = Union[Timeout, int, float, object]
+_TYPE_TIMEOUT = Union[Timeout, float, _TYPE_DEFAULT]
 
 _SelfT = TypeVar("_SelfT")
 
@@ -189,7 +185,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         self,
         host: str,
         port: Optional[int] = None,
-        timeout: Optional[Union[Timeout, float, int, object]] = Timeout.DEFAULT_TIMEOUT,
+        timeout: Optional[_TYPE_TIMEOUT] = _DEFAULT_TIMEOUT,
         maxsize: int = 1,
         block: bool = False,
         headers: Optional[Mapping[str, str]] = None,
@@ -251,7 +247,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         conn = self.ConnectionCls(
             host=self.host,
             port=self.port,
-            timeout=self.timeout.connect_timeout,  # type: ignore[arg-type]
+            timeout=self.timeout.connect_timeout,
             **self.conn_kw,
         )
         return conn
@@ -353,7 +349,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
 
     def _get_timeout(self, timeout: _TYPE_TIMEOUT) -> Timeout:
         """Helper that always returns a :class:`urllib3.util.Timeout`"""
-        if timeout is _Default:
+        if timeout is _DEFAULT_TIMEOUT:
             return self.timeout.clone()
 
         if isinstance(timeout, Timeout):
@@ -367,7 +363,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         self,
         err: Union[BaseSSLError, OSError, SocketTimeout],
         url: str,
-        timeout_value: _TYPE_TIMEOUT,
+        timeout_value: Optional[_TYPE_TIMEOUT],
     ) -> None:
         """Is the error actually a timeout? Will raise a ReadTimeout or pass"""
 
@@ -387,7 +383,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         conn: HTTPConnection,
         method: str,
         url: str,
-        timeout: _TYPE_TIMEOUT = _Default,
+        timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
         chunked: bool = False,
         **httplib_request_kw: Any,
     ) -> _HttplibHTTPResponse:
@@ -451,10 +447,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
                 raise ReadTimeoutError(
                     self, url, f"Read timed out. (read timeout={read_timeout})"
                 )
-            if read_timeout is Timeout.DEFAULT_TIMEOUT:
-                conn.sock.settimeout(socket.getdefaulttimeout())
-            else:  # None or a value
-                conn.sock.settimeout(read_timeout)
+            conn.sock.settimeout(read_timeout)
 
         # Receive the response from the server
         try:
@@ -540,7 +533,7 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
         retries: Optional[Union[Retry, bool, int]] = None,
         redirect: bool = True,
         assert_same_host: bool = True,
-        timeout: _TYPE_TIMEOUT = _Default,
+        timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
         pool_timeout: Optional[int] = None,
         release_conn: Optional[bool] = None,
         chunked: bool = False,
@@ -903,7 +896,7 @@ class HTTPSConnectionPool(HTTPConnectionPool):
         self,
         host: str,
         port: Optional[int] = None,
-        timeout: _TYPE_TIMEOUT = Timeout.DEFAULT_TIMEOUT,
+        timeout: Optional[_TYPE_TIMEOUT] = _DEFAULT_TIMEOUT,
         maxsize: int = 1,
         block: bool = False,
         headers: Optional[Mapping[str, str]] = None,
@@ -1013,7 +1006,7 @@ class HTTPSConnectionPool(HTTPConnectionPool):
         conn = self.ConnectionCls(
             host=actual_host,
             port=actual_port,
-            timeout=self.timeout.connect_timeout,  # type: ignore[arg-type]
+            timeout=self.timeout.connect_timeout,
             cert_file=self.cert_file,
             key_file=self.key_file,
             key_password=self.key_password,
diff --git a/src/urllib3/util/connection.py b/src/urllib3/util/connection.py
index c12cecd2..2c5bce75 100644
--- a/src/urllib3/util/connection.py
+++ b/src/urllib3/util/connection.py
@@ -2,9 +2,9 @@ import socket
 from typing import Optional, Sequence, Tuple, Union
 
 from ..exceptions import LocationParseError
+from .timeout import _DEFAULT_TIMEOUT, _TYPE_TIMEOUT
 from .wait import wait_for_read
 
-SOCKET_GLOBAL_DEFAULT_TIMEOUT = socket._GLOBAL_DEFAULT_TIMEOUT  # type: ignore[attr-defined]
 _TYPE_SOCKET_OPTIONS = Sequence[Tuple[int, int, Union[int, bytes]]]
 
 
@@ -28,7 +28,7 @@ def is_connection_dropped(conn: socket.socket) -> bool:  # Platform-specific
 # discovered in DNS if the system doesn't have IPv6 functionality.
 def create_connection(
     address: Tuple[str, int],
-    timeout: Optional[float] = SOCKET_GLOBAL_DEFAULT_TIMEOUT,
+    timeout: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
     source_address: Optional[Tuple[str, int]] = None,
     socket_options: Optional[_TYPE_SOCKET_OPTIONS] = None,
 ) -> socket.socket:
@@ -68,7 +68,7 @@ def create_connection(
             # If provided, set socket level options before connecting.
             _set_socket_options(sock, socket_options)
 
-            if timeout is not SOCKET_GLOBAL_DEFAULT_TIMEOUT:
+            if timeout is not _DEFAULT_TIMEOUT:
                 sock.settimeout(timeout)
             if source_address:
                 sock.bind(source_address)
diff --git a/src/urllib3/util/timeout.py b/src/urllib3/util/timeout.py
index d861dff2..bd364d78 100644
--- a/src/urllib3/util/timeout.py
+++ b/src/urllib3/util/timeout.py
@@ -1,18 +1,21 @@
 import time
-
-# The default socket timeout, used by httplib to indicate that no timeout was
-# specified by the user
-from socket import _GLOBAL_DEFAULT_TIMEOUT  # type: ignore[attr-defined]
-from typing import Optional, Union
+from enum import Enum
+from socket import getdefaulttimeout
+from typing import TYPE_CHECKING, Optional, Union
 
 from ..exceptions import TimeoutStateError
 
-# A sentinel value to indicate that no timeout was specified by the user in
-# urllib3
-_Default = object()
+if TYPE_CHECKING:
+    from typing_extensions import Final
+
+
+class _TYPE_DEFAULT(Enum):
+    token = 0
+
 
+_DEFAULT_TIMEOUT: "Final[_TYPE_DEFAULT]" = _TYPE_DEFAULT.token
 
-_TYPE_TIMEOUT = Optional[Union[float, int, object]]
+_TYPE_TIMEOUT = Optional[Union[float, _TYPE_DEFAULT]]
 
 
 class Timeout:
@@ -101,13 +104,13 @@ class Timeout:
     """
 
     #: A sentinel object representing the default timeout value
-    DEFAULT_TIMEOUT: object = _GLOBAL_DEFAULT_TIMEOUT
+    DEFAULT_TIMEOUT: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT
 
     def __init__(
         self,
         total: _TYPE_TIMEOUT = None,
-        connect: _TYPE_TIMEOUT = _Default,
-        read: _TYPE_TIMEOUT = _Default,
+        connect: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
+        read: _TYPE_TIMEOUT = _DEFAULT_TIMEOUT,
     ) -> None:
         self._connect = self._validate_timeout(connect, "connect")
         self._read = self._validate_timeout(read, "read")
@@ -120,10 +123,12 @@ class Timeout:
     # __str__ provided for backwards compatibility
     __str__ = __repr__
 
+    @staticmethod
+    def resolve_default_timeout(timeout: _TYPE_TIMEOUT) -> Optional[float]:
+        return getdefaulttimeout() if timeout is _DEFAULT_TIMEOUT else timeout
+
     @classmethod
-    def _validate_timeout(
-        cls, value: _TYPE_TIMEOUT, name: str
-    ) -> Optional[Union[float, object]]:
+    def _validate_timeout(cls, value: _TYPE_TIMEOUT, name: str) -> _TYPE_TIMEOUT:
         """Check that a timeout attribute is valid.
 
         :param value: The timeout value to validate
@@ -133,10 +138,7 @@ class Timeout:
         :raises ValueError: If it is a numeric value less than or equal to
             zero, or the type is not an integer, float, or None.
         """
-        if value is _Default:
-            return cls.DEFAULT_TIMEOUT
-
-        if value is None or value is cls.DEFAULT_TIMEOUT:
+        if value is None or value is _DEFAULT_TIMEOUT:
             return value
 
         if isinstance(value, bool):
@@ -145,7 +147,7 @@ class Timeout:
                 "be an int, float or None."
             )
         try:
-            float(value)  # type: ignore[arg-type]
+            float(value)
         except (TypeError, ValueError):
             raise ValueError(
                 "Timeout value %s was %s, but it must be an "
@@ -153,7 +155,7 @@ class Timeout:
             ) from None
 
         try:
-            if value <= 0:  # type: ignore[operator]
+            if value <= 0:
                 raise ValueError(
                     "Attempted to set %s timeout to %s, but the "
                     "timeout cannot be set to a value less "
@@ -169,7 +171,7 @@ class Timeout:
         return value
 
     @classmethod
-    def from_float(cls, timeout: Optional[Union[int, float, object]]) -> "Timeout":
+    def from_float(cls, timeout: _TYPE_TIMEOUT) -> "Timeout":
         """Create a new Timeout from a legacy timeout value.
 
         The timeout value used by httplib.py sets the same timeout on the
@@ -236,13 +238,13 @@ class Timeout:
         if self.total is None:
             return self._connect
 
-        if self._connect is None or self._connect is self.DEFAULT_TIMEOUT:
+        if self._connect is None or self._connect is _DEFAULT_TIMEOUT:
             return self.total
 
-        return min(self._connect, self.total)  # type: ignore[no-any-return, call-overload]
+        return min(self._connect, self.total)  # type: ignore[type-var]
 
     @property
-    def read_timeout(self) -> _TYPE_TIMEOUT:
+    def read_timeout(self) -> Optional[float]:
         """Get the value for the read timeout.
 
         This assumes some time has elapsed in the connection timeout and
@@ -254,21 +256,21 @@ class Timeout:
         raised.
 
         :return: Value to use for the read timeout.
-        :rtype: int, float, :attr:`Timeout.DEFAULT_TIMEOUT` or None
+        :rtype: int, float or None
         :raises urllib3.exceptions.TimeoutStateError: If :meth:`start_connect`
             has not yet been called on this object.
         """
         if (
             self.total is not None
-            and self.total is not self.DEFAULT_TIMEOUT
+            and self.total is not _DEFAULT_TIMEOUT
             and self._read is not None
-            and self._read is not self.DEFAULT_TIMEOUT
+            and self._read is not _DEFAULT_TIMEOUT
         ):
             # In case the connect timeout has not yet been established.
             if self._start_connect is None:
                 return self._read
-            return max(0, min(self.total - self.get_connect_duration(), self._read))  # type: ignore[no-any-return, call-overload, operator]
-        elif self.total is not None and self.total is not self.DEFAULT_TIMEOUT:
-            return max(0, self.total - self.get_connect_duration())  # type: ignore[operator]
+            return max(0, min(self.total - self.get_connect_duration(), self._read))
+        elif self.total is not None and self.total is not _DEFAULT_TIMEOUT:
+            return max(0, self.total - self.get_connect_duration())
         else:
-            return self._read
+            return self.resolve_default_timeout(self._read)


Test patch:
diff --git a/test/test_connectionpool.py b/test/test_connectionpool.py
index aad0c794..d033e7f9 100644
--- a/test/test_connectionpool.py
+++ b/test/test_connectionpool.py
@@ -31,7 +31,7 @@ from urllib3.exceptions import (
 )
 from urllib3.response import HTTPResponse
 from urllib3.util.ssl_match_hostname import CertificateError
-from urllib3.util.timeout import Timeout
+from urllib3.util.timeout import _DEFAULT_TIMEOUT, Timeout
 
 from .test_response import MockChunkedEncodingResponse, MockSock
 
@@ -410,8 +410,8 @@ class TestConnectionPool:
             conn = pool._new_conn()
             assert conn.__class__ == HTTPConnection
             assert pool.timeout.__class__ == Timeout
-            assert pool.timeout._read == Timeout.DEFAULT_TIMEOUT
-            assert pool.timeout._connect == Timeout.DEFAULT_TIMEOUT
+            assert pool.timeout._read == _DEFAULT_TIMEOUT
+            assert pool.timeout._connect == _DEFAULT_TIMEOUT
             assert pool.timeout.total is None
 
             pool = HTTPConnectionPool(host="localhost", timeout=SHORT_TIMEOUT)
diff --git a/test/test_util.py b/test/test_util.py
index 64989b4c..d8ddfcd2 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -33,7 +33,7 @@ from urllib3.util.ssl_ import (
     resolve_ssl_version,
     ssl_wrap_socket,
 )
-from urllib3.util.timeout import Timeout
+from urllib3.util.timeout import _DEFAULT_TIMEOUT, Timeout
 from urllib3.util.url import Url, _encode_invalid_chars, parse_url
 from urllib3.util.util import to_bytes, to_str
 
@@ -602,7 +602,7 @@ class TestUtil:
         assert timeout.connect_timeout == 2
 
         timeout = Timeout()
-        assert timeout.connect_timeout == Timeout.DEFAULT_TIMEOUT
+        assert timeout.connect_timeout == _DEFAULT_TIMEOUT
 
         # Connect takes 5 seconds, leaving 5 seconds for read
         timeout = Timeout(total=10, read=7)
@@ -629,6 +629,15 @@ class TestUtil:
         timeout = Timeout(5)
         assert timeout.total == 5
 
+    def test_timeout_default_resolve(self) -> None:
+        """The timeout default is resolved when read_timeout is accessed."""
+        timeout = Timeout()
+        with patch("urllib3.util.timeout.getdefaulttimeout", return_value=2):
+            assert timeout.read_timeout == 2
+
+        with patch("urllib3.util.timeout.getdefaulttimeout", return_value=3):
+            assert timeout.read_timeout == 3
+
     def test_timeout_str(self) -> None:
         timeout = Timeout(connect=1, read=2, total=3)
         assert str(timeout) == "Timeout(connect=1, read=2, total=3)"