Deprecate TCPConnector ssl (aio-libs#13829)

Author: Dreamsorcerer

CHANGES/13829.deprecation.rst

@@ -0,0 +1,5 @@
+Deprecated the ``ssl`` parameter of :class:`~aiohttp.TCPConnector`, scheduled for
+removal in 5.0 -- by :user:`Dreamsorcerer`.
+
+Pass ``ssl`` to :class:`~aiohttp.ClientSession` for a session-wide default, or to
+:meth:`~aiohttp.ClientSession.get` and the other request methods per request.

aiohttp/connector.py

@@ -954,11 +954,11 @@ def _make_ssl_context(verified: bool) -> SSLContext:
 class TCPConnector(BaseConnector):
     """TCP connector.
 
-    verify_ssl - Set to True to check ssl certifications.
-    fingerprint - Pass the binary sha256
-        digest of the expected certificate in DER format to verify
-        that the certificate the server presents matches. See also
-        https://en.wikipedia.org/wiki/HTTP_Public_Key_Pinning
+    ssl - DEPRECATED. Will be removed in aiohttp 5.0.
+        SSL validation mode: ``True`` for the default checks, ``False`` to
+        skip certificate validation, a Fingerprint for certificate pinning
+        or an ssl.SSLContext for custom validation. Pass ``ssl`` to
+        ClientSession or to the individual request instead.
     resolver - Enable DNS lookups and use this
         resolver
     use_dns_cache - Use memory cache for DNS lookups.
@@ -999,7 +999,7 @@ def __init__(
         ttl_dns_cache: int | None = 10,
         dns_cache_max_size: int = 1000,
         family: socket.AddressFamily = socket.AddressFamily.AF_UNSPEC,
-        ssl: bool | Fingerprint | SSLContext = True,
+        ssl: bool | Fingerprint | SSLContext | _SENTINEL = sentinel,
         local_addr: tuple[str, int] | None = None,
         resolver: AbstractResolver | None = None,
         keepalive_timeout: None | float | _SENTINEL = sentinel,
@@ -1022,12 +1022,23 @@ def __init__(
             timeout_ceil_threshold=timeout_ceil_threshold,
         )
 
-        if not isinstance(ssl, SSL_ALLOWED_TYPES):
-            raise TypeError(
-                "ssl should be SSLContext, Fingerprint, or bool, "
-                f"got {ssl!r} instead."
+        self._ssl: bool | Fingerprint | SSLContext
+        if ssl is sentinel:
+            self._ssl = True
+        else:
+            if not isinstance(ssl, SSL_ALLOWED_TYPES):
+                raise TypeError(
+                    "ssl should be SSLContext, Fingerprint, or bool, "
+                    f"got {ssl!r} instead."
+                )
+            warnings.warn(
+                "The ssl parameter is deprecated since 4.0 and scheduled for "
+                "removal in 5.0, pass ssl to ClientSession() or to the "
+                "individual request instead",
+                DeprecationWarning,
+                stacklevel=2,
             )
-        self._ssl = ssl
+            self._ssl = ssl
 
         self._resolver: AbstractResolver
         if resolver is None:

docs/client_advanced.rst

@@ -618,7 +618,7 @@ checks can be relaxed by setting *ssl* to ``False``::
 If you need to setup custom ssl parameters (use own certification
 files for example) you can create a :class:`ssl.SSLContext` instance and
 pass it into the :meth:`ClientSession.request` methods or set it for the
-entire session with ``ClientSession(connector=TCPConnector(ssl=ssl_context))``.
+entire session with ``ClientSession(ssl=ssl_context)``.
 
 There are explicit errors when ssl verification fails
 
@@ -660,7 +660,7 @@ installed or Python is unable to find them, resulting in a error like
 One way to work around this problem is to use the `certifi` package::
 
   ssl_context = ssl.create_default_context(cafile=certifi.where())
-  async with ClientSession(connector=TCPConnector(ssl=ssl_context)) as sess:
+  async with ClientSession(ssl=ssl_context) as sess:
       ...
 
 Example: Use self-signed certificate
@@ -706,9 +706,9 @@ DER with e.g::
    Tip: to convert from a hexadecimal digest to a binary byte-string,
    you can use :func:`binascii.unhexlify`.
 
-   *ssl* parameter could be passed
-   to :class:`TCPConnector` as default, the value from
-   :meth:`ClientSession.get` and others override default.
+   *ssl* parameter could be passed to :class:`ClientSession` as the
+   session-wide default, the value from :meth:`ClientSession.get` and
+   others override it.
 
 .. _aiohttp-client-proxy-support:
 

docs/client_reference.rst

@@ -1231,8 +1231,8 @@ is controlled by *force_close* constructor's parameter).
    Constructor accepts all parameters suitable for
    :class:`BaseConnector` plus several TCP-specific ones:
 
-      :param ssl: SSL validation mode. ``True`` for default SSL check
-                  (:func:`ssl.create_default_context` is used),
+      :param ssl: **(DEPRECATED)** SSL validation mode. ``True`` for default
+                  SSL check (:func:`ssl.create_default_context` is used),
                   ``False`` for skip SSL certificate validation,
                   :class:`aiohttp.Fingerprint` for fingerprint
                   validation, :class:`ssl.SSLContext` for custom SSL
@@ -1243,6 +1243,12 @@ is controlled by *force_close* constructor's parameter).
 
          .. versionadded:: 3.0
 
+         .. deprecated:: 4.0
+
+            Scheduled for removal in 5.0. Pass *ssl* to
+            :class:`ClientSession` for a session-wide default, or to
+            :meth:`ClientSession.get` and others per request.
+
    :param bool verify_ssl: perform SSL certificate validation for
       *HTTPS* requests (enabled by default). May be disabled to
       skip validation for sites with invalid certificates.

examples/fake_server.py

@@ -101,9 +101,9 @@ async def main() -> None:
     fake_facebook = FakeFacebook()
     info = await fake_facebook.start()
     resolver = FakeResolver(info)
-    connector = TCPConnector(resolver=resolver, ssl=False)
+    connector = TCPConnector(resolver=resolver)
 
-    async with ClientSession(connector=connector) as session:
+    async with ClientSession(connector=connector, ssl=False) as session:
         async with session.get(
             "https://graph.facebook.com/v2.7/me", params={"access_token": token}
         ) as resp:

tests/test_client_functional.py

@@ -706,15 +706,13 @@ async def test_ssl_client(
     aiohttp_client: AiohttpClient,
     client_ssl_ctx: ssl.SSLContext,
 ) -> None:
-    connector = aiohttp.TCPConnector(ssl=client_ssl_ctx)
-
     async def handler(request: web.Request) -> web.Response:
         return web.Response(text="Test message")
 
     app = web.Application()
     app.router.add_route("GET", "/", handler)
     server = await aiohttp_server(app, ssl=ssl_ctx)
-    client = await aiohttp_client(server, connector=connector)  # type: ignore[var-annotated]
+    client = await aiohttp_client(server, ssl=client_ssl_ctx)  # type: ignore[var-annotated]
 
     async with client.get("/") as resp:
         assert resp.status == 200
@@ -743,8 +741,8 @@ async def handler(request: web.Request) -> web.Response:
     server = await aiohttp_server(app, ssl=server_ctx)
     url = server.make_url("/")
 
-    connector = aiohttp.TCPConnector(ssl=client_ctx, limit=1, limit_per_host=1)
-    async with aiohttp.ClientSession(connector=connector) as session:
+    connector = aiohttp.TCPConnector(limit=1, limit_per_host=1)
+    async with aiohttp.ClientSession(connector=connector, ssl=client_ctx) as session:
         async with session.get(url, server_hostname="first.example") as resp:
             assert resp.status == 200
             await resp.read()
@@ -767,7 +765,7 @@ async def test_ssl_client_shutdown_timeout(
     with pytest.warns(
         DeprecationWarning, match="ssl_shutdown_timeout parameter is deprecated"
     ):
-        connector = aiohttp.TCPConnector(ssl=client_ssl_ctx, ssl_shutdown_timeout=0.1)
+        connector = aiohttp.TCPConnector(ssl_shutdown_timeout=0.1)
 
     async def streaming_handler(request: web.Request) -> NoReturn:
         # Create a streaming response that continuously sends data
@@ -784,7 +782,9 @@ async def streaming_handler(request: web.Request) -> NoReturn:
     app = web.Application()
     app.router.add_route("GET", "/stream", streaming_handler)
     server = await aiohttp_server(app, ssl=ssl_ctx)
-    client = await aiohttp_client(server, connector=connector)  # type: ignore[var-annotated]
+    client = await aiohttp_client(  # type: ignore[var-annotated]
+        server, connector=connector, ssl=client_ssl_ctx
+    )
 
     # Verify the connector has the correct timeout
     assert connector._ssl_shutdown_timeout == 0.1
@@ -836,8 +836,7 @@ async def handler(request: web.Request) -> web.Response:
     ssl_ctx.set_alpn_protocols(("http/1.1",))
     server = await aiohttp_server(app, ssl=ssl_ctx)
 
-    connector = aiohttp.TCPConnector(ssl=False)
-    client = await aiohttp_client(server, connector=connector)  # type: ignore[var-annotated]
+    client = await aiohttp_client(server, ssl=False)  # type: ignore[var-annotated]
     async with client.get("/") as resp:
         assert resp.status == 200
         txt = await resp.text()
@@ -855,11 +854,13 @@ async def test_tcp_connector_fingerprint_ok(
     async def handler(request: web.Request) -> web.Response:
         return web.Response(text="Test message")
 
-    connector = aiohttp.TCPConnector(ssl=tls_fingerprint)
+    connector = aiohttp.TCPConnector()
     app = web.Application()
     app.router.add_route("GET", "/", handler)
     server = await aiohttp_server(app, ssl=ssl_ctx)
-    client = await aiohttp_client(server, connector=connector)  # type: ignore[var-annotated]
+    client = await aiohttp_client(  # type: ignore[var-annotated]
+        server, connector=connector, ssl=tls_fingerprint
+    )
 
     async with client.get("/") as resp:
         assert resp.status == 200
@@ -882,12 +883,12 @@ async def handler(request: web.Request) -> NoReturn:
 
     bad_fingerprint = b"\x00" * len(tls_certificate_fingerprint_sha256)
 
-    connector = aiohttp.TCPConnector(ssl=Fingerprint(bad_fingerprint))
-
     app = web.Application()
     app.router.add_route("GET", "/", handler)
     server = await aiohttp_server(app, ssl=ssl_ctx)
-    client = await aiohttp_client(server, connector=connector)  # type: ignore[var-annotated]
+    client = await aiohttp_client(  # type: ignore[var-annotated]
+        server, ssl=Fingerprint(bad_fingerprint)
+    )
 
     with pytest.raises(ServerFingerprintMismatch) as cm:
         await client.get("/")
@@ -3487,10 +3488,10 @@ async def resolve(
         async def close(self) -> None:
             """Dummy"""
 
-    connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False)
+    connector = aiohttp.TCPConnector(resolver=FakeResolver())
 
     async with (
-        aiohttp.ClientSession(connector=connector) as client,
+        aiohttp.ClientSession(connector=connector, ssl=False) as client,
         client.get(
             url_from,
             headers={"Authorization": aiohttp.encode_basic_auth("user", "pass")},
@@ -3597,9 +3598,9 @@ async def resolve(
         async def close(self) -> None:
             """Dummy"""
 
-    connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False)
+    connector = aiohttp.TCPConnector(resolver=FakeResolver())
 
-    async with aiohttp.ClientSession(connector=connector) as client:
+    async with aiohttp.ClientSession(connector=connector, ssl=False) as client:
         async with client.get(
             url_from,
             headers={
@@ -3722,10 +3723,11 @@ async def resolve(
         async def close(self) -> None:
             """Dummy"""
 
-    connector = aiohttp.TCPConnector(resolver=FakeResolver(), ssl=False)
+    connector = aiohttp.TCPConnector(resolver=FakeResolver())
 
     async with aiohttp.ClientSession(
         connector=connector,
+        ssl=False,
         headers={"Authorization": "Basic dXNlcjpwYXNz"},
     ) as client:
         async with client.get(url_from) as resp:

tests/test_connector.py

@@ -2702,12 +2702,45 @@ async def test_invalid_ssl_param() -> None:
 
 async def test_tcp_connector_ctor_fingerprint_valid() -> None:
     valid = aiohttp.Fingerprint(hashlib.sha256(b"foo").digest())
-    conn = aiohttp.TCPConnector(ssl=valid)
+    with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"):
+        conn = aiohttp.TCPConnector(ssl=valid)
     assert conn._ssl is valid
 
     await conn.close()
 
 
+async def test_tcp_connector_ssl_deprecated() -> None:
+    with pytest.warns(
+        DeprecationWarning,
+        match="ssl parameter is deprecated since 4.0 and scheduled for removal in 5.0",
+    ):
+        conn = aiohttp.TCPConnector(ssl=False)
+    assert conn._ssl is False
+
+    await conn.close()
+
+
+async def test_tcp_connector_ssl_default_not_deprecated() -> None:
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        conn = aiohttp.TCPConnector()
+    assert conn._ssl is True
+
+    await conn.close()
+
+
+async def test_tcp_connector_fingerprint_from_deprecated_ssl_param() -> None:
+    """The deprecated connector-level ssl is still used when the request has none."""
+    fingerprint = aiohttp.Fingerprint(hashlib.sha256(b"foo").digest())
+    with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"):
+        conn = aiohttp.TCPConnector(ssl=fingerprint)
+    req = mock.Mock()
+    req.ssl = True
+    assert conn._get_fingerprint(req) is fingerprint
+
+    await conn.close()
+
+
 async def test_insecure_fingerprint_md5() -> None:
     with pytest.raises(ValueError):
         aiohttp.TCPConnector(ssl=aiohttp.Fingerprint(hashlib.md5(b"foo").digest()))
@@ -2787,7 +2820,8 @@ async def test___get_ssl_context2() -> None:
 
 async def test___get_ssl_context3() -> None:
     ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
-    conn = aiohttp.TCPConnector(ssl=ctx)
+    with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"):
+        conn = aiohttp.TCPConnector(ssl=ctx)
     req = mock.Mock()
     req.is_ssl.return_value = True
     req.ssl = True
@@ -2798,7 +2832,8 @@ async def test___get_ssl_context3() -> None:
 
 async def test___get_ssl_context4() -> None:
     ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
-    conn = aiohttp.TCPConnector(ssl=ctx)
+    with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"):
+        conn = aiohttp.TCPConnector(ssl=ctx)
     req = mock.Mock()
     req.is_ssl.return_value = True
     req.ssl = False
@@ -2809,7 +2844,8 @@ async def test___get_ssl_context4() -> None:
 
 async def test___get_ssl_context5() -> None:
     ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
-    conn = aiohttp.TCPConnector(ssl=ctx)
+    with pytest.warns(DeprecationWarning, match="ssl parameter is deprecated"):
+        conn = aiohttp.TCPConnector(ssl=ctx)
     req = mock.Mock()
     req.is_ssl.return_value = True
     req.ssl = aiohttp.Fingerprint(hashlib.sha256(b"1").digest())

tests/test_proxy_functional.py

@@ -58,10 +58,7 @@ async def get_request(
         trust_env: bool = False,
         **kwargs: Any,
     ) -> ClientResponse:
-        connector = aiohttp.TCPConnector(ssl=False)
-        async with aiohttp.ClientSession(
-            connector=connector, trust_env=trust_env
-        ) as client:
+        async with aiohttp.ClientSession(ssl=False, trust_env=trust_env) as client:
             async with client.request(method, url, **kwargs) as resp:
                 return resp
 

tests/test_web_sendfile_functional.py

@@ -742,8 +742,7 @@ async def test_static_file_ssl(
     app = web.Application()
     app.router.add_static("/static", dirname)
     server = await aiohttp_server(app, ssl=ssl_ctx)
-    conn = aiohttp.TCPConnector(ssl=client_ssl_ctx)
-    client = await aiohttp_client(server, connector=conn)  # type: ignore[var-annotated]
+    client = await aiohttp_client(server, ssl=client_ssl_ctx)  # type: ignore[var-annotated]
 
     resp = await client.get("/static/" + filename)
     assert 200 == resp.status
@@ -755,7 +754,6 @@ async def test_static_file_ssl(
 
     resp.release()
     await client.close()
-    await conn.close()
 
 
 async def test_static_file_directory_traversal_attack(