From a26fd314ffbf3a439bee7124347721508177b266 Mon Sep 17 00:00:00 2001 From: julianz- <6255571+julianz-@users.noreply.github.com> Date: Fri, 31 Oct 2025 11:16:19 -0700 Subject: [PATCH] Translate OpenSSL SysCallError to ConnectionError PyOpenSSL raises SysCallError which cheroot does not translate, leading to unhandled exceptions when the underlying connection is closed or shut down abruptly. This commit ensures all relevant errno codes are translated into standard Python ConnectionError types for reliable handling by consumers. --- .flake8 | 1 + cheroot/ssl/pyopenssl.py | 53 +++++++++-- cheroot/test/ssl/test_ssl_pyopenssl.py | 106 ++++++++++++++++++++++ docs/changelog-fragments.d/786.bugfix.rst | 3 + 4 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 cheroot/test/ssl/test_ssl_pyopenssl.py create mode 100644 docs/changelog-fragments.d/786.bugfix.rst diff --git a/.flake8 b/.flake8 index aca9b2fffe..8aa29eed9c 100644 --- a/.flake8 +++ b/.flake8 @@ -132,6 +132,7 @@ per-file-ignores = cheroot/ssl/pyopenssl.py: C815, DAR101, DAR201, DAR401, I001, I003, I005, N801, N804, RST304, WPS100, WPS110, WPS111, WPS117, WPS120, WPS121, WPS130, WPS210, WPS220, WPS221, WPS225, WPS229, WPS231, WPS238, WPS301, WPS335, WPS338, WPS420, WPS422, WPS430, WPS432, WPS501, WPS504, WPS505, WPS601, WPS608, WPS615 cheroot/test/conftest.py: DAR101, DAR201, DAR301, I001, I003, I005, WPS100, WPS130, WPS325, WPS354, WPS420, WPS422, WPS430, WPS457 cheroot/test/helper.py: DAR101, DAR201, DAR401, I001, I003, I004, N802, WPS110, WPS111, WPS121, WPS201, WPS220, WPS231, WPS301, WPS414, WPS421, WPS422, WPS505 + cheroot/test/ssl/test_ssl_pyopenssl.py: DAR101, DAR201, WPS226 cheroot/test/test_cli.py: DAR101, DAR201, I001, I005, N802, S101, S108, WPS110, WPS421, WPS431, WPS473 cheroot/test/test_makefile.py: DAR101, DAR201, I004, RST304, S101, WPS110, WPS122 cheroot/test/test_wsgi.py: DAR101, DAR301, I001, I004, S101, WPS110, WPS111, WPS117, WPS118, WPS121, WPS210, WPS421, WPS430, WPS432, WPS441, WPS509 diff --git a/cheroot/ssl/pyopenssl.py b/cheroot/ssl/pyopenssl.py index d17db00468..eb5f7f5098 100644 --- a/cheroot/ssl/pyopenssl.py +++ b/cheroot/ssl/pyopenssl.py @@ -50,6 +50,8 @@ pyopenssl """ +import errno +import os import socket import sys import threading @@ -77,6 +79,45 @@ from . import Adapter +@contextlib.contextmanager +def _morph_syscall_to_connection_error(method_name, /): + """ + Handle :exc:`OpenSSL.SSL.SysCallError` in a wrapped method. + + This context manager catches and re-raises SSL system call errors + with appropriate exception types. + + Yields: + None: Execution continues within the context block. + """ # noqa: DAR301 + try: + yield + except SSL.SysCallError as ssl_syscall_err: + connection_error_map = { + errno.EBADF: ConnectionError, # socket is gone? + errno.ECONNABORTED: ConnectionAbortedError, + errno.ECONNREFUSED: ConnectionRefusedError, + errno.ECONNRESET: ConnectionResetError, + errno.ENOTCONN: ConnectionError, + errno.EPIPE: BrokenPipeError, + errno.ESHUTDOWN: BrokenPipeError, + } + error_code = ssl_syscall_err.args[0] if ssl_syscall_err.args else None + error_msg = ( + os.strerror(error_code) + if error_code is not None + else repr(ssl_syscall_err) + ) + conn_err_cls = connection_error_map.get( + error_code, + ConnectionError, + ) + raise conn_err_cls( + error_code, + f'Error in calling {method_name!s} on PyOpenSSL connection: {error_msg!s}', + ) from ssl_syscall_err + + class SSLFileobjectMixin: """Base mixin for a TLS socket stream.""" @@ -224,14 +265,12 @@ def lock_decorator(method): """Create a proxy method for a new class.""" def proxy_wrapper(self, *args): - self._lock.acquire() - try: - new_args = ( - args[:] if method not in proxy_methods_no_args else [] - ) + new_args = ( + args[:] if method not in proxy_methods_no_args else [] + ) + # translate any SysCallError to ConnectionError + with _morph_syscall_to_connection_error(method), self._lock: return getattr(self._ssl_conn, method)(*new_args) - finally: - self._lock.release() return proxy_wrapper diff --git a/cheroot/test/ssl/test_ssl_pyopenssl.py b/cheroot/test/ssl/test_ssl_pyopenssl.py new file mode 100644 index 0000000000..84a932f804 --- /dev/null +++ b/cheroot/test/ssl/test_ssl_pyopenssl.py @@ -0,0 +1,106 @@ +"""Tests for :py:mod:`cheroot.ssl.pyopenssl` :py:class:`cheroot.ssl.pyopenssl.SSLConnection` wrapper.""" + +import errno + +import pytest + +from OpenSSL import SSL + +from cheroot.ssl.pyopenssl import SSLConnection + + +@pytest.fixture +def mock_ssl_context(mocker): + """Fixture providing a mock instance of :py:class:`OpenSSL.SSL.Context`.""" + mock_context = mocker.Mock(spec=SSL.Context) + + # Add a mock _context attribute to simulate SSL.Context behavior + mock_context._context = mocker.Mock() + return mock_context + + +@pytest.mark.filterwarnings('ignore:Non-callable called') +@pytest.mark.parametrize( + ( + 'tested_method_name', + 'simulated_errno', + 'expected_exception', + ), + ( + pytest.param('close', errno.EBADF, ConnectionError, id='close-EBADF'), + pytest.param( + 'close', + errno.ECONNABORTED, + ConnectionAbortedError, + id='close-ECONNABORTED', + ), + pytest.param( + 'send', + errno.EPIPE, + BrokenPipeError, + id='send-EPIPE', + ), # Expanded coverage + pytest.param( + 'shutdown', + errno.EPIPE, + BrokenPipeError, + id='shutdown-EPIPE', + ), + pytest.param( + 'shutdown', + errno.ECONNRESET, + ConnectionResetError, + id='shutdown-ECONNRESET', + ), + pytest.param( + 'close', + errno.ENOTCONN, + ConnectionError, + id='close-ENOTCONN', + ), + pytest.param('close', errno.EPIPE, BrokenPipeError, id='close-EPIPE'), + pytest.param( + 'close', + errno.ESHUTDOWN, + BrokenPipeError, + id='close-ESHUTDOWN', + ), + ), +) +def test_close_morphs_syscall_error_correctly( + mocker, + mock_ssl_context, + tested_method_name, + simulated_errno, + expected_exception, +): + """Check ``SSLConnection.close()`` morphs ``SysCallError`` to ``ConnectionError``.""" + # Prevents the real OpenSSL.SSL.Connection.__init__ from running + mocker.patch('OpenSSL.SSL.Connection') + + # Create SSLConnection object. The patched SSL.Connection() call returns + # a mock that is stored internally as conn._ssl_conn. + conn = SSLConnection(mock_ssl_context) + + # Define specific OpenSSL error based on the parameter + simulated_error = SSL.SysCallError( + simulated_errno, + 'Simulated connection error', + ) + + # Dynamically retrieve the method on the underlying mock + underlying_method = getattr(conn._ssl_conn, tested_method_name) + + # Patch the method to raise the simulated error + underlying_method.side_effect = simulated_error + + expected_match = ( + f'.*Error in calling {tested_method_name} on PyOpenSSL connection.*' + ) + + # Assert the expected exception is raised based on the parameter + with pytest.raises(expected_exception, match=expected_match) as excinfo: + getattr(conn, tested_method_name)() + + # Assert the original SysCallError is included in the new exception's cause + assert excinfo.value.__cause__ is simulated_error diff --git a/docs/changelog-fragments.d/786.bugfix.rst b/docs/changelog-fragments.d/786.bugfix.rst new file mode 100644 index 0000000000..6b861cf66a --- /dev/null +++ b/docs/changelog-fragments.d/786.bugfix.rst @@ -0,0 +1,3 @@ +OpenSSL system call errors are now intercepted and re-raised as Python exceptions derived from :exc:`ConnectionError`. + +-- by :user:`julianz-`