🐛 fix async connection shutdown in HTTP/1.1 and HTTP/2 leaving a `asy…

…ncio.TransportSocket` and `_SelectorSocketTransport` partially closed

CHANGES.rst

@@ -1,3 +1,8 @@
+2.11.908 (2024-11-03)
+=====================
+
+- Fixed async connection shutdown in HTTP/1.1 and HTTP/2 leaving a ``asyncio.TransportSocket`` and ``_SelectorSocketTransport`` partially closed.
+
 2.11.907 (2024-10-30)
 =====================
 

dummyserver/server.py

@@ -163,14 +163,32 @@ def ssl_options_to_context(  # type: ignore[no-untyped-def]
     alpn_protocols=None,
 ) -> ssl.SSLContext:
     """Return an equivalent SSLContext based on ssl.wrap_socket args."""
-    ssl_version = resolve_ssl_version(ssl_version)
+    _major, _minor, _patch = ssl.OPENSSL_VERSION_INFO[:3]
+    is_broken_old_ssl: bool = (
+        "OpenSSL" in ssl.OPENSSL_VERSION and _major < 1 or (_major == 1 and _minor < 1)
+    )
+
+    ssl_version = resolve_ssl_version(  # type: ignore[call-overload]
+        ssl_version, mitigate_tls_version=not is_broken_old_ssl
+    )
     cert_none = resolve_cert_reqs("CERT_NONE")
     if cert_reqs is None:
         cert_reqs = cert_none
     else:
         cert_reqs = resolve_cert_reqs(cert_reqs)
 
-    ctx = ssl.SSLContext(ssl_version)
+    if (
+        hasattr(ssl.SSLContext, "minimum_version")
+        and hasattr(ssl, "PROTOCOL_TLS_SERVER")
+        and is_broken_old_ssl is False
+    ):
+        ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
+        if hasattr(ssl, "TLSVersion") and isinstance(ssl_version, ssl.TLSVersion):
+            ctx.minimum_version = ssl_version
+            ctx.maximum_version = ssl_version
+    else:
+        ctx = ssl.SSLContext(ssl_version)  # type: ignore[arg-type]
+
     ctx.load_cert_chain(certfile, keyfile)
     ctx.verify_mode = cert_reqs
     if ctx.verify_mode != cert_none:

pyproject.toml

@@ -108,14 +108,12 @@ filterwarnings = [
     "error",
     '''default:No IPv6 support. Falling back to IPv4:urllib3.exceptions.HTTPWarning''',
     '''default:No IPv6 support. skipping:urllib3.exceptions.HTTPWarning''',
-    '''default:ssl\.TLSVersion\.TLSv1 is deprecated:DeprecationWarning''',
     '''default:ssl\.PROTOCOL_TLS is deprecated:DeprecationWarning''',
     '''default:ssl\.PROTOCOL_TLSv1 is deprecated:DeprecationWarning''',
     '''default:ssl\.TLSVersion\.TLSv1_1 is deprecated:DeprecationWarning''',
     '''default:ssl\.PROTOCOL_TLSv1_1 is deprecated:DeprecationWarning''',
     '''default:ssl\.PROTOCOL_TLSv1_2 is deprecated:DeprecationWarning''',
     '''default:unclosed .*:ResourceWarning''',
-    '''default:loop is closed:ResourceWarning''',
     '''default:ssl NPN is deprecated, use ALPN instead:DeprecationWarning''',
     # https://github.com/pytest-dev/pytest/issues/10977
     '''default:ast\.(Num|NameConstant|Str) is deprecated and will be removed in Python 3\.14; use ast\.Constant instead:DeprecationWarning:_pytest''',
@@ -129,6 +127,9 @@ filterwarnings = [
     '''ignore:Exception in thread:pytest.PytestUnhandledThreadExceptionWarning''',
     '''ignore:function _SSLProtocolTransport\.__del__:pytest.PytestUnraisableExceptionWarning''',
     '''ignore:The `hash` argument is deprecated in favor of `unsafe_hash`:DeprecationWarning''',
+    '''ignore:ssl\.TLSVersion\.TLSv1 is deprecated:DeprecationWarning''',
+    '''ignore:ssl\.TLSVersion\.TLSv1_1 is deprecated:DeprecationWarning''',
+    '''ignore:loop is closed:ResourceWarning''',
 ]
 
 [tool.isort]

src/urllib3/_async/connection.py

@@ -920,7 +920,7 @@ async def _ssl_wrap_socket_and_match_hostname(
     """
     default_ssl_context = False
     sharable_ext_options: dict[str, int | str | None] = {
-        "ssl_version": resolve_ssl_version(ssl_version),
+        "ssl_version": resolve_ssl_version(ssl_version, mitigate_tls_version=True),
         "ssl_minimum_version": ssl_minimum_version,
         "ssl_maximum_version": ssl_maximum_version,
         "cert_reqs": resolve_cert_reqs(cert_reqs),

src/urllib3/backend/_async/hface.py

@@ -190,7 +190,10 @@ async def _new_conn(self) -> AsyncSocket | None:  # type: ignore[override]
             # if conn target another host.
             if self._response and self._response.authority != self.host:
                 self._svn = None
-                await self._new_conn()  # restore socket defaults
+                self._response = None
+                if self.blocksize == UDP_DEFAULT_BLOCKSIZE:
+                    self.blocksize = DEFAULT_BLOCKSIZE
+                self.socket_kind = SOCK_STREAM
         else:
             if self.blocksize == UDP_DEFAULT_BLOCKSIZE:
                 self.blocksize = DEFAULT_BLOCKSIZE
@@ -1644,6 +1647,9 @@ async def close(self) -> None:  # type: ignore[override]
 
             try:
                 self.sock.close()
+                # this avoids having SelectorSocketTransport in "closing" state
+                # pending. Thus avoid a ResourceWarning.
+                await self.sock.wait_for_close()
             except OSError:
                 pass
 

src/urllib3/backend/hface.py

@@ -200,7 +200,10 @@ def _new_conn(self) -> socket.socket | None:
             # if conn target another host.
             if self._response and self._response.authority != self.host:
                 self._svn = None
-                self._new_conn()  # restore socket defaults
+                self._response = None  # type: ignore[assignment]
+                if self.blocksize == UDP_DEFAULT_BLOCKSIZE:
+                    self.blocksize = DEFAULT_BLOCKSIZE
+                self.socket_kind = SOCK_STREAM
         else:
             if self.blocksize == UDP_DEFAULT_BLOCKSIZE:
                 self.blocksize = DEFAULT_BLOCKSIZE

src/urllib3/connection.py

@@ -893,7 +893,7 @@ def _ssl_wrap_socket_and_match_hostname(
     """
     default_ssl_context = False
     sharable_ext_options: dict[str, int | str | None] = {
-        "ssl_version": resolve_ssl_version(ssl_version),
+        "ssl_version": resolve_ssl_version(ssl_version, mitigate_tls_version=True),
         "ssl_minimum_version": ssl_minimum_version,
         "ssl_maximum_version": ssl_maximum_version,
         "cert_reqs": resolve_cert_reqs(cert_reqs),

src/urllib3/contrib/resolver/_async/doq/_qh3.py

@@ -104,6 +104,7 @@ async def close(self) -> None:  # type: ignore[override]
                     await self._socket.sendall(data)
 
             self._socket.close()
+            await self._socket.wait_for_close()
             self._terminated = True
         if self._socket.should_connect():
             self._terminated = True

src/urllib3/contrib/resolver/_async/dou/_socket.py

@@ -70,6 +70,7 @@ async def close(self) -> None:  # type: ignore[override]
         if not self._terminated:
             with self._lock:
                 self._socket.close()
+                await self._socket.wait_for_close()
                 self._terminated = True
 
     def is_available(self) -> bool:

src/urllib3/contrib/resolver/_async/protocols.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import asyncio
 import socket
 import typing
 from abc import ABCMeta, abstractmethod
@@ -131,7 +132,11 @@ async def create_connection(  # type: ignore[override]
                 if source_address:
                     sock.bind(source_address)
 
-                await sock.connect(sa)
+                try:
+                    await sock.connect(sa)
+                except asyncio.CancelledError:
+                    sock.close()
+                    raise
 
                 # Break explicitly a reference cycle
                 err = None

src/urllib3/contrib/resolver/protocols.py

@@ -224,7 +224,10 @@ def create_connection(
                 if source_address is not None:
                     try:
                         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEPORT, 1)
-                    except (OSError, AttributeError):  # Defensive: very old OS?
+                    except (
+                        OSError,
+                        AttributeError,
+                    ):  # Defensive: Windows or very old OS?
                         try:
                             sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
                         except (

src/urllib3/contrib/ssa/__init__.py

@@ -27,6 +27,26 @@
     from ..._typing import _TYPE_PEER_CERT_RET, _TYPE_PEER_CERT_RET_DICT
 
 
+def _can_shutdown_and_close_selector_loop_bug() -> bool:
+    import platform
+
+    if platform.system() == "Windows" and platform.python_version_tuple()[:2] == (
+        "3",
+        "7",
+    ):
+        return int(platform.python_version_tuple()[-1]) >= 17
+
+    return True
+
+
+# Windows + asyncio bug where doing our shutdown procedure induce a crash
+# in SelectorLoop
+# File "C:\hostedtoolcache\windows\Python\3.7.9\x64\lib\selectors.py", line 314, in _select
+#     r, w, x = select.select(r, w, w, timeout)
+# [WinError 10038] An operation was attempted on something that is not a socket
+_CPYTHON_SELECTOR_CLOSE_BUG_EXIST = _can_shutdown_and_close_selector_loop_bug() is False
+
+
 class AsyncSocket:
     """
     This class is brought to add a level of abstraction to an asyncio transport (reader, or writer)
@@ -73,6 +93,23 @@ def __init__(
     def fileno(self) -> int:
         return self._fileno if self._fileno is not None else self._sock.fileno()
 
+    async def wait_for_close(self) -> None:
+        if self._connect_called:
+            return
+
+        if self._writer is None:
+            return
+
+        is_ssl = self._writer.get_extra_info("ssl_object") is not None
+
+        if is_ssl:
+            # Give the connection a chance to write any data in the buffer,
+            # and then forcibly tear down the SSL connection.
+            await asyncio.sleep(0)
+            self._writer.transport.abort()
+
+        await self._writer.wait_closed()
+
     def close(self) -> None:
         if self._writer is not None:
             self._writer.close()
@@ -83,15 +120,21 @@ def close(self) -> None:
             # probably not just uvloop.
             uvloop_edge_case_bug = False
 
+            # keep track of our clean exit procedure
+            shutdown_called = False
+            close_called = False
+
             if hasattr(self._sock, "shutdown"):
                 try:
                     self._sock.shutdown(SHUT_RD)
+                    shutdown_called = True
                 except TypeError:
                     uvloop_edge_case_bug = True
                     # uvloop don't support shutdown! and sometime does not support close()...
                     # see https://github.com/jawah/niquests/issues/166 for ctx.
                     try:
                         self._sock.close()
+                        close_called = True
                     except TypeError:
                         # last chance of releasing properly the underlying fd!
                         try:
@@ -101,6 +144,7 @@ def close(self) -> None:
                         else:
                             try:
                                 direct_sock.shutdown(SHUT_RD)
+                                shutdown_called = True
                             except OSError:
                                 warnings.warn(
                                     (
@@ -113,15 +157,34 @@ def close(self) -> None:
                                 )
                             finally:
                                 direct_sock.detach()
-            elif hasattr(self._sock, "close"):
-                self._sock.close()
-            # we have to force call close() on our sock object in UDP ctx. (even after shutdown)
+            # we have to force call close() on our sock object (even after shutdown).
             # or we'll get a resource warning for sure!
-            if self.type == socket.SOCK_DGRAM and hasattr(self._sock, "close"):
-                if not uvloop_edge_case_bug:
-                    self._sock.close()
-        except OSError:
-            pass
+            if isinstance(self._sock, socket.socket) and hasattr(self._sock, "close"):
+                if not uvloop_edge_case_bug and not _CPYTHON_SELECTOR_CLOSE_BUG_EXIST:
+                    try:
+                        self._sock.close()
+                        close_called = True
+                    except OSError:
+                        pass
+
+            if not close_called or not shutdown_called:
+                # this branch detect whether we have an asyncio.TransportSocket instead of socket.socket.
+                if (
+                    hasattr(self._sock, "_sock")
+                    and not _CPYTHON_SELECTOR_CLOSE_BUG_EXIST
+                ):
+                    try:
+                        self._sock._sock.detach()
+                    except (AttributeError, OSError):
+                        pass
+
+        except (
+            OSError
+        ):  # branch where we failed to connect and still try to release resource
+            try:
+                self._sock.close()
+            except (OSError, TypeError, AttributeError):
+                pass
 
         self._connect_called = False
         self._established.clear()

src/urllib3/util/ssl_.py

@@ -224,16 +224,14 @@ def _is_has_never_check_common_name_reliable(
     # Python built against (very) restrictive ssl library may ship with a single TLS version
     # thus, it seems to make attribute "minimum_version" and "maximum_version" unavailable.
     # note: it raises an exception! maybe a CPython bug.
-    SUPPORT_MIN_MAX_TLS_VERSION = hasattr(
-        ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT), "maximum_version"
-    )
+    SUPPORT_MIN_MAX_TLS_VERSION = hasattr(ssl.SSLContext, "maximum_version")
 except ImportError:
     OP_NO_COMPRESSION = 0x20000  # type: ignore[assignment]
     OP_NO_TICKET = 0x4000  # type: ignore[assignment]
     OP_NO_SSLv2 = 0x1000000  # type: ignore[assignment]
     OP_NO_SSLv3 = 0x2000000  # type: ignore[assignment]
     PROTOCOL_SSLv23 = PROTOCOL_TLS = 2  # type: ignore[assignment]
-    PROTOCOL_TLS_CLIENT = 16  # type: ignore[assignment]
+    PROTOCOL_TLS_CLIENT = PROTOCOL_TLS
     OP_NO_RENEGOTIATION = None  # type: ignore[assignment]
     SUPPORT_MIN_MAX_TLS_VERSION = False
 
@@ -298,19 +296,59 @@ def resolve_cert_reqs(candidate: None | int | str) -> VerifyMode:
     return candidate  # type: ignore[return-value]
 
 
-def resolve_ssl_version(candidate: None | int | str) -> int:
+@typing.overload
+def resolve_ssl_version(
+    candidate: None | int | str,
+    *,
+    mitigate_tls_version: typing.Literal[True] = True,
+) -> ssl.TLSVersion:
+    ...
+
+
+@typing.overload
+def resolve_ssl_version(
+    candidate: None | int | str,
+    *,
+    mitigate_tls_version: typing.Literal[False] = False,
+) -> int:
+    ...
+
+
+def resolve_ssl_version(
+    candidate: None | int | str, mitigate_tls_version: bool = False
+) -> int | ssl.TLSVersion:
     """
     like resolve_cert_reqs
     """
     if candidate is None:
+        if mitigate_tls_version:
+            return PROTOCOL_TLS_CLIENT
         return PROTOCOL_TLS
 
     if isinstance(candidate, str):
+        if mitigate_tls_version and hasattr(ssl, "TLSVersion"):
+            res = getattr(ssl.TLSVersion, candidate, None)
+
+            if res is not None:
+                return res  # type: ignore[no-any-return]
+
+            res = getattr(ssl.TLSVersion, candidate.replace("PROTOCOL_", ""), None)
+
+            if res is not None:
+                return res  # type: ignore[no-any-return]
+
         res = getattr(ssl, candidate, None)
         if res is None:
             res = getattr(ssl, "PROTOCOL_" + candidate)
         return typing.cast(int, res)
 
+    if mitigate_tls_version:
+        if candidate in _SSL_VERSION_TO_TLS_VERSION:
+            return _SSL_VERSION_TO_TLS_VERSION[candidate]
+        if candidate == PROTOCOL_TLS_CLIENT or candidate == PROTOCOL_TLS:
+            return PROTOCOL_TLS_CLIENT
+        return ssl.TLSVersion.MAXIMUM_SUPPORTED
+
     return candidate
 
 
@@ -363,13 +401,17 @@ def create_urllib3_context(
             )
 
         else:
-            # Use 'ssl_minimum_version' and 'ssl_maximum_version' instead.
-            ssl_minimum_version = _SSL_VERSION_TO_TLS_VERSION.get(
-                ssl_version, TLSVersion.MINIMUM_SUPPORTED
-            )
-            ssl_maximum_version = _SSL_VERSION_TO_TLS_VERSION.get(
-                ssl_version, TLSVersion.MAXIMUM_SUPPORTED
-            )
+            if hasattr(ssl, "TLSVersion") and isinstance(ssl_version, ssl.TLSVersion):
+                ssl_minimum_version = ssl_version
+                ssl_maximum_version = ssl_version
+            else:
+                # Use 'ssl_minimum_version' and 'ssl_maximum_version' instead.
+                ssl_minimum_version = _SSL_VERSION_TO_TLS_VERSION.get(
+                    ssl_version, TLSVersion.MINIMUM_SUPPORTED
+                )
+                ssl_maximum_version = _SSL_VERSION_TO_TLS_VERSION.get(
+                    ssl_version, TLSVersion.MAXIMUM_SUPPORTED
+                )
 
     # PROTOCOL_TLS is deprecated in Python 3.10 so we always use PROTOCOL_TLS_CLIENT
     context = SSLContext(PROTOCOL_TLS_CLIENT)