diff --git a/CHANGES/4099.breaking.rst b/CHANGES/4099.breaking.rst new file mode 100644 index 00000000000..19124fe9ce7 --- /dev/null +++ b/CHANGES/4099.breaking.rst @@ -0,0 +1,9 @@ +``ClientConnectorCertificateError.ssl`` now returns the value passed to the +``ssl`` parameter, an :class:`ssl.SSLContext`, a :class:`bool` or a +:class:`~aiohttp.Fingerprint`, matching +:attr:`ClientConnectorError.ssl `. It +previously returned ``ConnectionKey.is_ssl``, a plain :class:`bool`, so the +same attribute meant different things depending on which of the two exceptions +was caught. Code reading ``ssl`` on this exception as a boolean should use +``is_ssl`` on the connection key instead +-- by :user:`ArockiaRajamanickam`. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 3748922ee71..4cb95362d93 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -54,6 +54,7 @@ Anton Kasyanov Anton Zhdan-Pushkin Arcadiy Ivanov Arie Bovenberg +Arockia Rajamanickam Arseny Timoniq Arsh Smith Arshiya Tabasum diff --git a/aiohttp/client_exceptions.py b/aiohttp/client_exceptions.py index 826533af81a..63dac84261d 100644 --- a/aiohttp/client_exceptions.py +++ b/aiohttp/client_exceptions.py @@ -382,13 +382,10 @@ def host(self) -> str: def port(self) -> int | None: return self._conn_key.port - @property - def ssl(self) -> bool: - return self._conn_key.is_ssl - def __str__(self) -> str: return ( - f"Cannot connect to host {self.host}:{self.port} ssl:{self.ssl} " + f"Cannot connect to host {self.host}:{self.port} " + f"ssl:{'default' if self.ssl is True else self.ssl} " f"[{self.certificate_error.__class__.__name__}: " f"{self.certificate_error.args}]" ) diff --git a/tests/test_client_exceptions.py b/tests/test_client_exceptions.py index 164bbf58219..3843394a75c 100644 --- a/tests/test_client_exceptions.py +++ b/tests/test_client_exceptions.py @@ -1,5 +1,6 @@ import errno import pickle +import ssl import sys import pytest @@ -176,7 +177,9 @@ def test_ctor(self) -> None: assert err.certificate_error == certificate_error assert err.host == "example.com" assert err.port == 8080 - assert err.ssl is False + # the fixture sets is_ssl=False and ssl=True, so this asserts that the + # attribute follows `ssl` and not `is_ssl`, matching ClientConnectorError + assert err.ssl is True if sys.version_info >= (3, 11): assert_type(err.args, tuple[client_reqrep.ConnectionKey, Exception]) @@ -192,7 +195,7 @@ def test_pickle(self) -> None: assert err2.certificate_error.args == ("Bad certificate",) assert err2.host == "example.com" assert err2.port == 8080 - assert err2.ssl is False + assert err2.ssl is True assert err2.foo == "bar" def test_repr(self) -> None: @@ -211,7 +214,7 @@ def test_str(self) -> None: connection_key=self.connection_key, certificate_error=certificate_error ) assert str(err) == ( - "Cannot connect to host example.com:8080 ssl:False" + "Cannot connect to host example.com:8080 ssl:default" " [Exception: ('Bad certificate',)]" ) @@ -224,6 +227,28 @@ def test_oserror(self) -> None: assert err.errno == 1 assert err.strerror == "Bad certificate" + def test_ssl_is_the_same_as_on_the_base_class(self) -> None: + """`ssl` must carry the same value as on ClientConnectorError. + + ConnectionKey has both `is_ssl` (a bool) and `ssl` (the SSLContext, + bool or Fingerprint the caller passed). Reading `is_ssl` here made the + same attribute mean two different things depending on which of the two + exceptions was caught. + """ + context = ssl.create_default_context() + connection_key = self.connection_key._replace(is_ssl=True, ssl=context) + certificate_error = Exception("Bad certificate") + + err = client.ClientConnectorCertificateError( + connection_key=connection_key, certificate_error=certificate_error + ) + base_err = client.ClientConnectorError( + connection_key=connection_key, os_error=OSError(1, "Bad certificate") + ) + + assert err.ssl is context + assert err.ssl is base_err.ssl + class TestServerDisconnectedError: def test_ctor(self) -> None: