Issue #23197, asyncio: On SSL handshake failure, check if the waiter is

cancelled before setting its exception.

* Add unit tests for this case.
* Cleanup also sslproto.py

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index b2f29c706a098cda5b14f604eaba4e00cbf093bf..ca8626488659628e14c8e525a840ec49ee98657e 100644 (file)
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -750,7 +750,7 @@ class _SelectorSslTransport(_SelectorTransport):
             self._loop.remove_reader(self._sock_fd)
             self._loop.remove_writer(self._sock_fd)
             self._sock.close()
-            if self._waiter is not None:
+            if self._waiter is not None and not self._waiter.cancelled():
                 self._waiter.set_exception(exc)
             if isinstance(exc, Exception):
                 return

diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py
index 987c158ee73bafb338161fee62538f0e101db343..541e252774eb007877d15bf67abe263d57bb994d 100644 (file)
--- a/Lib/asyncio/sslproto.py
+++ b/Lib/asyncio/sslproto.py
@@ -530,10 +530,11 @@ class SSLProtocol(protocols.Protocol):
         self._in_handshake = False
 
         sslobj = self._sslpipe.ssl_object
-        peercert = None if handshake_exc else sslobj.getpeercert()
         try:
             if handshake_exc is not None:
                 raise handshake_exc
+
+            peercert = sslobj.getpeercert()
             if not hasattr(self._sslcontext, 'check_hostname'):
                 # Verify hostname if requested, Python 3.4+ uses check_hostname
                 # and checks the hostname in do_handshake()
@@ -551,7 +552,7 @@ class SSLProtocol(protocols.Protocol):
                                    self, exc_info=True)
             self._transport.close()
             if isinstance(exc, Exception):
-                if self._waiter is not None:
+                if self._waiter is not None and not self._waiter.cancelled():
                     self._waiter.set_exception(exc)
                 return
             else:

diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py
index 360327aa86913885fc66bf79902821d4f76de96e..64c2e650a20b0c6e1c6e657eed13e816277524be 100644 (file)
--- a/Lib/test/test_asyncio/test_selector_events.py
+++ b/Lib/test/test_asyncio/test_selector_events.py
@@ -1148,16 +1148,28 @@ class SelectorSslTransportTests(test_utils.TestCase):
         self.assertTrue(self.sslsock.close.called)
 
     def test_on_handshake_base_exc(self):
+        waiter = asyncio.Future(loop=self.loop)
         transport = _SelectorSslTransport(
-            self.loop, self.sock, self.protocol, self.sslcontext)
-        transport._waiter = asyncio.Future(loop=self.loop)
+            self.loop, self.sock, self.protocol, self.sslcontext, waiter)
         exc = BaseException()
         self.sslsock.do_handshake.side_effect = exc
         with test_utils.disable_logger():
             self.assertRaises(BaseException, transport._on_handshake, 0)
         self.assertTrue(self.sslsock.close.called)
-        self.assertTrue(transport._waiter.done())
-        self.assertIs(exc, transport._waiter.exception())
+        self.assertTrue(waiter.done())
+        self.assertIs(exc, waiter.exception())
+
+    def test_cancel_handshake(self):
+        # Python issue #23197: cancelling an handshake must not raise an
+        # exception or log an error, even if the handshake failed
+        waiter = asyncio.Future(loop=self.loop)
+        transport = _SelectorSslTransport(
+            self.loop, self.sock, self.protocol, self.sslcontext, waiter)
+        waiter.cancel()
+        exc = ValueError()
+        self.sslsock.do_handshake.side_effect = exc
+        with test_utils.disable_logger():
+            transport._on_handshake(0)
 
     def test_pause_resume_reading(self):
         tr = self._make_one()

diff --git a/Lib/test/test_asyncio/test_sslproto.py b/Lib/test/test_asyncio/test_sslproto.py
new file mode 100644 (file)
index 0000000..053fefe
--- /dev/null
+++ b/Lib/test/test_asyncio/test_sslproto.py
@@ -0,0 +1,45 @@
+"""Tests for asyncio/sslproto.py."""
+
+import unittest
+from unittest import mock
+
+import asyncio
+from asyncio import sslproto
+from asyncio import test_utils
+
+
+class SslProtoHandshakeTests(test_utils.TestCase):
+
+    def setUp(self):
+        self.loop = asyncio.new_event_loop()
+        self.set_event_loop(self.loop)
+
+    def test_cancel_handshake(self):
+        # Python issue #23197: cancelling an handshake must not raise an
+        # exception or log an error, even if the handshake failed
+        sslcontext = test_utils.dummy_ssl_context()
+        app_proto = asyncio.Protocol()
+        waiter = asyncio.Future(loop=self.loop)
+        ssl_proto = sslproto.SSLProtocol(self.loop, app_proto, sslcontext,
+                                         waiter)
+        handshake_fut = asyncio.Future(loop=self.loop)
+
+        def do_handshake(callback):
+            exc = Exception()
+            callback(exc)
+            handshake_fut.set_result(None)
+            return []
+
+        waiter.cancel()
+        transport = mock.Mock()
+        sslpipe = mock.Mock()
+        sslpipe.do_handshake.side_effect = do_handshake
+        with mock.patch('asyncio.sslproto._SSLPipe', return_value=sslpipe):
+            ssl_proto.connection_made(transport)
+
+        with test_utils.disable_logger():
+            self.loop.run_until_complete(handshake_fut)
+
+
+if __name__ == '__main__':
+    unittest.main()