Translate OpenSSL SysCallError to ConnectionError

julianz- · julianz- · commit 85503d96cfc3 · 2025-11-01T09:58:26.000-07:00
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.
diff --git a/.flake8 b/.flake8
@@ -129,9 +129,10 @@ per-file-ignores =
   cheroot/makefile.py: DAR101, DAR201, DAR401, E800, I003, I004, N801, N802, S101, WPS100, WPS110, WPS111, WPS117, WPS120, WPS121, WPS122, WPS123, WPS130, WPS204, WPS210, WPS212, WPS213, WPS220, WPS229, WPS231, WPS232, WPS338, WPS420, WPS422, WPS429, WPS431, WPS504, WPS604, WPS606
   cheroot/server.py: DAR003, DAR101, DAR201, DAR202, DAR301, DAR401, E800, I001, I003, I004, I005, N806, RST201, RST301, RST303, RST304, WPS100, WPS110, WPS111, WPS115, WPS120, WPS121, WPS122, WPS130, WPS132, WPS201, WPS202, WPS204, WPS210, WPS211, WPS212, WPS213, WPS214, WPS220, WPS221, WPS225, WPS226, WPS229, WPS230, WPS231, WPS236, WPS237, WPS238, WPS301, WPS338, WPS342, WPS410, WPS420, WPS421, WPS422, WPS429, WPS432, WPS504, WPS505, WPS601, WPS602, WPS608, WPS617
   cheroot/ssl/builtin.py: DAR101, DAR201, DAR401, I001, I003, N806, RST304, WPS110, WPS111, WPS115, WPS117, WPS120, WPS121, WPS122, WPS130, WPS201, WPS210, WPS214, WPS229, WPS231, WPS338, WPS422, WPS501, WPS505, WPS529, WPS608, WPS612
-  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/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, WPS602, 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
@@ -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."""
 
@@ -175,94 +216,89 @@ class SSLFileobjectStreamWriter(SSLFileobjectMixin, StreamWriter):
     """SSL file object attached to a socket object."""
 
 
-class SSLConnectionProxyMeta:
-    """Metaclass for generating a bunch of proxy methods."""
-
-    def __new__(mcl, name, bases, nmspc):
-        """Attach a list of proxy methods to a new class."""
-        proxy_methods = (
-            'get_context',
-            'pending',
-            'send',
-            'write',
-            'recv',
-            'read',
-            'renegotiate',
-            'bind',
-            'listen',
-            'connect',
-            'accept',
-            'setblocking',
-            'fileno',
-            'close',
-            'get_cipher_list',
-            'getpeername',
-            'getsockname',
-            'getsockopt',
-            'setsockopt',
-            'makefile',
-            'get_app_data',
-            'set_app_data',
-            'state_string',
-            'sock_shutdown',
-            'get_peer_certificate',
-            'want_read',
-            'want_write',
-            'set_connect_state',
-            'set_accept_state',
-            'connect_ex',
-            'sendall',
-            'settimeout',
-            'gettimeout',
-            'shutdown',
-        )
-        proxy_methods_no_args = ('shutdown',)
-
-        proxy_props = ('family',)
-
-        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 []
-                    )
-                    return getattr(self._ssl_conn, method)(*new_args)
-                finally:
-                    self._lock.release()
-
-            return proxy_wrapper
-
-        for m in proxy_methods:
-            nmspc[m] = lock_decorator(m)
-            nmspc[m].__name__ = m
-
-        def make_property(property_):
-            """Create a proxy method for a new class."""
-
-            def proxy_prop_wrapper(self):
-                return getattr(self._ssl_conn, property_)
-
-            proxy_prop_wrapper.__name__ = property_
-            return property(proxy_prop_wrapper)
-
-        for p in proxy_props:
-            nmspc[p] = make_property(p)
+# Wrapper function to dynamically add thread-safe methods and properties
+def _wrap_ssl_connection_methods(cls):
+    """Dynamically attaches proxied methods and properties to the class."""
+    # 1. Attach Methods
+    for method in cls.proxy_methods:
+        wrapped_func = cls._lock_decorator(method)
+        setattr(cls, method, wrapped_func)
+
+    # 2. Attach Properties
+    for prop in cls.proxy_props:
+        setattr(cls, prop, cls._make_property(prop))
+
+    return cls
+
+
+@_wrap_ssl_connection_methods
+class SSLConnection:
+    """A thread-safe wrapper around :py:class:`SSL.Connection <pyopenssl:OpenSSL.SSL.Connection>`."""
+
+    proxy_methods = (
+        'get_context',
+        'pending',
+        'send',
+        'write',
+        'recv',
+        'read',
+        'renegotiate',
+        'bind',
+        'listen',
+        'connect',
+        'accept',
+        'setblocking',
+        'fileno',
+        'close',
+        'get_cipher_list',
+        'getpeername',
+        'getsockname',
+        'getsockopt',
+        'setsockopt',
+        'makefile',
+        'get_app_data',
+        'set_app_data',
+        'state_string',
+        'sock_shutdown',
+        'get_peer_certificate',
+        'want_read',
+        'want_write',
+        'set_connect_state',
+        'set_accept_state',
+        'connect_ex',
+        'sendall',
+        'settimeout',
+        'gettimeout',
+        'shutdown',
+    )
+    proxy_methods_no_args = ('shutdown',)
+    proxy_props = ('family',)
+
+    @staticmethod
+    def _lock_decorator(method):
+        """Create the thread-safe, error-translating proxy method."""
+
+        def proxy_wrapper(self, *args):
+            new_args = (
+                args[:]
+                if method not in SSLConnection.proxy_methods_no_args
+                else []
+            )
+            with self._lock, _morph_syscall_to_connection_error(method):
+                return getattr(self._ssl_conn, method)(*new_args)
 
-        # Doesn't work via super() for some reason.
-        # Falling back to type() instead:
-        return type(name, bases, nmspc)
+        proxy_wrapper.__name__ = method
+        return proxy_wrapper
 
+    @staticmethod
+    def _make_property(property_):
+        """Create the property proxy."""
 
-class SSLConnection(metaclass=SSLConnectionProxyMeta):
-    r"""A thread-safe wrapper for an ``SSL.Connection``.
+        def proxy_prop_wrapper(self):
+            return getattr(self._ssl_conn, property_)
 
-    :param tuple args: the arguments to create the wrapped \
-                        :py:class:`SSL.Connection(*args) \
-                        <pyopenssl:OpenSSL.SSL.Connection>`
-    """
+        proxy_prop_wrapper.__name__ = property_
+        return property(proxy_prop_wrapper)
 
     def __init__(self, *args):
         """Initialize SSLConnection instance."""
diff --git a/cheroot/ssl/pyopenssl.pyi b/cheroot/ssl/pyopenssl.pyi
@@ -18,10 +18,11 @@ class SSLFileobjectMixin:
 class SSLFileobjectStreamReader(SSLFileobjectMixin, StreamReader): ...  # type:ignore[misc]
 class SSLFileobjectStreamWriter(SSLFileobjectMixin, StreamWriter): ...  # type:ignore[misc]
 
-class SSLConnectionProxyMeta:
-    def __new__(mcl, name, bases, nmspc): ...
-
 class SSLConnection:
+    proxy_methods: tuple[str, ...]
+    proxy_methods_no_args: tuple[str, ...]
+    proxy_props: tuple[str, ...]
+
     def __init__(self, *args) -> None: ...
 
 class pyOpenSSLAdapter(Adapter):
diff --git a/cheroot/test/ssl/test_ssl_pyopenssl.py 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
@@ -0,0 +1,3 @@
+OpenSSL system call errors are now intercepted and re-raised as Python exceptions derived from :exc:`ConnectionError`.
+
+-- by :user:`julianz-`

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+OpenSSL system call errors are now intercepted and re-raised as Python exceptions derived from :exc:`ConnectionError`.
	`2`	`+`
	`3`	+-- by :user:`julianz-`