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-`