Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cheroot/test/ssl/test_ssl_pyopenssl.py: DAR101, DAR201, WPS226
cheroot/test/ssl/test_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
Expand Down
53 changes: 46 additions & 7 deletions cheroot/ssl/pyopenssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
pyopenssl
"""

import errno
import os
import socket
import sys
import threading
Expand Down Expand Up @@ -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.
Comment on lines +90 to +91
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be using sphinx-native param lists, not napoleon.

""" # noqa: DAR301
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be possible to drop

Suggested change
""" # 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."""

Expand Down Expand Up @@ -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

Expand Down
106 changes: 106 additions & 0 deletions cheroot/test/ssl/test_ssl_pyopenssl.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to ‎cheroot/test/ssl/test_pyopenssl.py to mimic the directory layout of the main project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have the second ssl_.

Original file line number Diff line number Diff line change
@@ -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)

Check failure

Code scanning / CodeQL

Non-callable called Error test

Call to a
non-callable
of
class SSLConnectionProxyMeta
.

# 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
3 changes: 3 additions & 0 deletions docs/changelog-fragments.d/786.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
OpenSSL system call errors are now intercepted and re-raised as Python exceptions derived from :exc:`ConnectionError`.

-- by :user:`julianz-`
Loading