Skip to content

Commit b768b11

Browse files
committed
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.
1 parent 4a8dc43 commit b768b11

File tree

4 files changed

+156
-7
lines changed

4 files changed

+156
-7
lines changed

.flake8

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ per-file-ignores =
138138
cheroot/test/test_core.py: C815, DAR101, DAR201, DAR401, I003, I004, N805, N806, S101, WPS110, WPS111, WPS114, WPS121, WPS202, WPS204, WPS226, WPS229, WPS324, WPS421, WPS422, WPS432, WPS602
139139
cheroot/test/test_dispatch.py: DAR101, DAR201, S101, WPS111, WPS121, WPS422, WPS430
140140
cheroot/test/test_ssl.py: C818, DAR101, DAR201, DAR301, DAR401, E800, I001, I003, I004, I005, S101, S309, S404, S603, WPS100, WPS110, WPS111, WPS114, WPS121, WPS130, WPS201, WPS202, WPS204, WPS210, WPS211, WPS218, WPS219, WPS222, WPS226, WPS231, WPS235, WPS301, WPS324, WPS335, WPS336, WPS408, WPS420, WPS421, WPS422, WPS432, WPS441, WPS509, WPS608
141+
cheroot/test/test_ssl_pyopenssl.py: DAR101, DAR201, WPS226
141142
cheroot/test/test_server.py: DAR101, DAR201, DAR301, I001, I003, I004, I005, S101, WPS110, WPS111, WPS118, WPS121, WPS122, WPS130, WPS201, WPS202, WPS210, WPS218, WPS226, WPS229, WPS324, WPS421, WPS422, WPS430, WPS432, WPS473, WPS509, WPS608
142143
cheroot/test/test_conn.py: DAR101, DAR201, DAR301, DAR401, E800, I001, I003, I004, I005, N802, N805, RST304, S101, S310, WPS100, WPS110, WPS111, WPS114, WPS115, WPS120, WPS121, WPS122, WPS201, WPS202, WPS204, WPS210, WPS211, WPS213, WPS214, WPS218, WPS219, WPS226, WPS231, WPS301, WPS402, WPS420, WPS421, WPS422, WPS429, WPS430, WPS432, WPS435, WPS447, WPS504, WPS509, WPS602
143144
cheroot/test/webtest.py: DAR101, DAR201, DAR401, I001, I003, I004, N802, RST303, RST304, S101, S104, WPS100, WPS110, WPS111, WPS115, WPS120, WPS121, WPS122, WPS201, WPS202, WPS204, WPS210, WPS211, WPS213, WPS214, WPS220, WPS221, WPS223, WPS229, WPS230, WPS231, WPS236, WPS237, WPS301, WPS338, WPS414, WPS420, WPS421, WPS422, WPS430, WPS432, WPS501, WPS505, WPS601

cheroot/ssl/pyopenssl.py

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
pyopenssl
5151
"""
5252

53+
import errno
54+
import os
5355
import socket
5456
import sys
5557
import threading
@@ -77,6 +79,45 @@
7779
from . import Adapter
7880

7981

82+
@contextlib.contextmanager
83+
def _morph_syscall_to_connection_error(method_name, /):
84+
"""
85+
Handle :exc:`OpenSSL.SSL.SysCallError` in a wrapped method.
86+
87+
This context manager catches and re-raises SSL system call errors
88+
with appropriate exception types.
89+
90+
Yields:
91+
None: Execution continues within the context block.
92+
""" # noqa: DAR301
93+
try:
94+
yield
95+
except SSL.SysCallError as ssl_syscall_err:
96+
connection_error_map = {
97+
errno.EBADF: ConnectionError, # socket is gone?
98+
errno.ECONNABORTED: ConnectionAbortedError,
99+
errno.ECONNREFUSED: ConnectionRefusedError,
100+
errno.ECONNRESET: ConnectionResetError,
101+
errno.ENOTCONN: ConnectionError,
102+
errno.EPIPE: BrokenPipeError,
103+
errno.ESHUTDOWN: BrokenPipeError,
104+
}
105+
error_code = ssl_syscall_err.args[0] if ssl_syscall_err.args else None
106+
error_msg = (
107+
os.strerror(error_code)
108+
if error_code is not None
109+
else repr(ssl_syscall_err)
110+
)
111+
conn_err_cls = connection_error_map.get(
112+
error_code,
113+
ConnectionError,
114+
)
115+
raise conn_err_cls(
116+
error_code,
117+
f'Error in calling {method_name!s} on PyOpenSSL connection: {error_msg!s}',
118+
) from ssl_syscall_err
119+
120+
80121
class SSLFileobjectMixin:
81122
"""Base mixin for a TLS socket stream."""
82123

@@ -224,14 +265,12 @@ def lock_decorator(method):
224265
"""Create a proxy method for a new class."""
225266

226267
def proxy_wrapper(self, *args):
227-
self._lock.acquire()
228-
try:
229-
new_args = (
230-
args[:] if method not in proxy_methods_no_args else []
231-
)
268+
new_args = (
269+
args[:] if method not in proxy_methods_no_args else []
270+
)
271+
# translate any SysCallError to ConnectionError
272+
with _morph_syscall_to_connection_error(method), self._lock:
232273
return getattr(self._ssl_conn, method)(*new_args)
233-
finally:
234-
self._lock.release()
235274

236275
return proxy_wrapper
237276

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
"""Tests for :py:mod: `cheroot.ssl.pyopenssl` :py:class:`SSLConnection` wrapper."""
2+
3+
import errno
4+
5+
import pytest
6+
7+
from OpenSSL import SSL
8+
9+
from cheroot.ssl.pyopenssl import SSLConnection
10+
11+
12+
@pytest.fixture
13+
def mock_ssl_context(mocker):
14+
"""Fixture providing a mock instance of :py:class:`SSL.Context`."""
15+
mock_context = mocker.Mock(spec=SSL.Context)
16+
17+
# Add a mock _context attribute to simulate SSL.Context behavior
18+
mock_context._context = mocker.Mock()
19+
return mock_context
20+
21+
22+
@pytest.mark.filterwarnings('ignore:Non-callable called')
23+
@pytest.mark.parametrize(
24+
(
25+
'tested_method_name',
26+
'simulated_errno',
27+
'expected_exception',
28+
),
29+
(
30+
pytest.param('close', errno.EBADF, ConnectionError, id='close-EBADF'),
31+
pytest.param(
32+
'close',
33+
errno.ECONNABORTED,
34+
ConnectionAbortedError,
35+
id='close-ECONNABORTED',
36+
),
37+
pytest.param(
38+
'send',
39+
errno.EPIPE,
40+
BrokenPipeError,
41+
id='send-EPIPE',
42+
), # Expanded coverage
43+
pytest.param(
44+
'shutdown',
45+
errno.EPIPE,
46+
BrokenPipeError,
47+
id='shutdown-EPIPE',
48+
),
49+
pytest.param(
50+
'shutdown',
51+
errno.ECONNRESET,
52+
ConnectionResetError,
53+
id='shutdown-ECONNRESET',
54+
),
55+
pytest.param(
56+
'close',
57+
errno.ENOTCONN,
58+
ConnectionError,
59+
id='close-ENOTCONN',
60+
),
61+
pytest.param('close', errno.EPIPE, BrokenPipeError, id='close-EPIPE'),
62+
pytest.param(
63+
'close',
64+
errno.ESHUTDOWN,
65+
BrokenPipeError,
66+
id='close-ESHUTDOWN',
67+
),
68+
),
69+
)
70+
def test_close_morphs_syscall_error_correctly(
71+
mocker,
72+
mock_ssl_context,
73+
tested_method_name,
74+
simulated_errno,
75+
expected_exception,
76+
):
77+
"""Check ``SSLConnection.close()`` morphs ``SysCallError`` to ``ConnectionError``."""
78+
# Prevents the real OpenSSL.SSL.Connection.__init__ from running
79+
mocker.patch('OpenSSL.SSL.Connection')
80+
81+
# Create SSLConnection object. The patched SSL.Connection() call returns
82+
# a mock that is stored internally as conn._ssl_conn.
83+
conn = SSLConnection(mock_ssl_context)
84+
85+
# Define specific OpenSSL error based on the parameter
86+
simulated_error = SSL.SysCallError(
87+
simulated_errno,
88+
'Simulated connection error',
89+
)
90+
91+
# Dynamically retrieve the method on the underlying mock
92+
underlying_method = getattr(conn._ssl_conn, tested_method_name)
93+
94+
# Patch the method to raise the simulated error
95+
underlying_method.side_effect = simulated_error
96+
97+
expected_match = (
98+
f'.*Error in calling {tested_method_name} on PyOpenSSL connection.*'
99+
)
100+
101+
# Assert the expected exception is raised based on the parameter
102+
with pytest.raises(expected_exception, match=expected_match) as excinfo:
103+
getattr(conn, tested_method_name)()
104+
105+
# Assert the original SysCallError is included in the new exception's cause
106+
assert excinfo.value.__cause__ is simulated_error
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
OpenSSL system call errors are now intercepted and re-raised as standard Python :exc:`ConnectionErrors`.
2+
3+
-- by :user:`julianz-`

0 commit comments

Comments
 (0)