Bump cheroot from 10.0.0 to 10.0.1 (#2310)

* Bump cheroot from 10.0.0 to 10.0.1

Bumps [cheroot](https://github.com/cherrypy/cheroot) from 10.0.0 to 10.0.1.
- [Release notes](https://github.com/cherrypy/cheroot/releases)
- [Changelog](https://github.com/cherrypy/cheroot/blob/main/CHANGES.rst)
- [Commits](https://github.com/cherrypy/cheroot/compare/v10.0.0...v10.0.1)

---
updated-dependencies:
- dependency-name: cheroot
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update cheroot==10.0.1

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: JonnyWong16 <9099342+JonnyWong16@users.noreply.github.com>

[skip ci]

lib/cheroot/connections.py

@@ -292,7 +292,20 @@ class ConnectionManager:
             if self.server.ssl_adapter is not None:
                 try:
                     s, ssl_env = self.server.ssl_adapter.wrap(s)
-                except errors.NoSSLError:
+                except errors.FatalSSLAlert as tls_connection_drop_error:
+                    self.server.error_log(
+                        f'Client {addr !s} lost — peer dropped the TLS '
+                        'connection suddenly, during handshake: '
+                        f'{tls_connection_drop_error !s}',
+                    )
+                    return
+                except errors.NoSSLError as http_over_https_err:
+                    self.server.error_log(
+                        f'Client {addr !s} attempted to speak plain HTTP into '
+                        'a TCP connection configured for TLS-only traffic — '
+                        'trying to send back a plain HTTP error response: '
+                        f'{http_over_https_err !s}',
+                    )
                     msg = (
                         'The client sent a plain HTTP request, but '
                         'this server only speaks HTTPS on this port.'
@@ -311,8 +324,6 @@ class ConnectionManager:
                         if ex.args[0] not in errors.socket_errors_to_ignore:
                             raise
                     return
-                if not s:
-                    return
                 mf = self.server.ssl_adapter.makefile
                 # Re-apply our timeout since we may have a new socket object
                 if hasattr(s, 'settimeout'):

lib/cheroot/server.py

@@ -157,7 +157,7 @@ QUOTED_SLASH = b'%2F'
 QUOTED_SLASH_REGEX = re.compile(b''.join((b'(?i)', QUOTED_SLASH)))
 
 
-_STOPPING_FOR_INTERRUPT = object()  # sentinel used during shutdown
+_STOPPING_FOR_INTERRUPT = Exception()  # sentinel used during shutdown
 
 
 comma_separated_headers = [
@@ -209,7 +209,11 @@ class HeaderReader:
             if not line.endswith(CRLF):
                 raise ValueError('HTTP requires CRLF terminators')
 
-            if line[0] in (SPACE, TAB):
+            if line[:1] in (SPACE, TAB):
+                # NOTE: `type(line[0]) is int` and `type(line[:1]) is bytes`.
+                # NOTE: The former causes a the following warning:
+                # NOTE: `BytesWarning('Comparison between bytes and int')`
+                # NOTE: The latter is equivalent and does not.
                 # It's a continuation line.
                 v = line.strip()
             else:
@@ -1725,16 +1729,16 @@ class HTTPServer:
         """Run the server forever, and stop it cleanly on exit."""
         try:
             self.start()
-        except (KeyboardInterrupt, IOError):
-            # The time.sleep call might raise
-            # "IOError: [Errno 4] Interrupted function call" on KBInt.
-            self.error_log('Keyboard Interrupt: shutting down')
-            self.stop()
-            raise
-        except SystemExit:
-            self.error_log('SystemExit raised: shutting down')
-            self.stop()
-            raise
+        except KeyboardInterrupt as kb_intr_exc:
+            underlying_interrupt = self.interrupt
+            if not underlying_interrupt:
+                self.interrupt = kb_intr_exc
+            raise kb_intr_exc from underlying_interrupt
+        except SystemExit as sys_exit_exc:
+            underlying_interrupt = self.interrupt
+            if not underlying_interrupt:
+                self.interrupt = sys_exit_exc
+            raise sys_exit_exc from underlying_interrupt
 
     def prepare(self):  # noqa: C901  # FIXME
         """Prepare server to serving requests.
@@ -2111,6 +2115,13 @@ class HTTPServer:
         has completed.
         """
         self._interrupt = _STOPPING_FOR_INTERRUPT
+
+        if isinstance(interrupt, KeyboardInterrupt):
+            self.error_log('Keyboard Interrupt: shutting down')
+
+        if isinstance(interrupt, SystemExit):
+            self.error_log('SystemExit raised: shutting down')
+
         self.stop()
         self._interrupt = interrupt
 

lib/cheroot/ssl/builtin.py

@@ -27,12 +27,9 @@ except ImportError:
 
 from . import Adapter
 from .. import errors
-from .._compat import IS_ABOVE_OPENSSL10
 from ..makefile import StreamReader, StreamWriter
 from ..server import HTTPServer
 
-generic_socket_error = OSError
-
 
 def _assert_ssl_exc_contains(exc, *msgs):
     """Check whether SSL exception contains either of messages provided."""
@@ -265,62 +262,35 @@ class BuiltinSSLAdapter(Adapter):
 
     def wrap(self, sock):
         """Wrap and return the given socket, plus WSGI environ entries."""
-        EMPTY_RESULT = None, {}
         try:
             s = self.context.wrap_socket(
                 sock, do_handshake_on_connect=True, server_side=True,
             )
-        except ssl.SSLError as ex:
-            if ex.errno == ssl.SSL_ERROR_EOF:
-                # This is almost certainly due to the cherrypy engine
-                # 'pinging' the socket to assert it's connectable;
-                # the 'ping' isn't SSL.
-                return EMPTY_RESULT
-            elif ex.errno == ssl.SSL_ERROR_SSL:
-                if _assert_ssl_exc_contains(ex, 'http request'):
-                    # The client is speaking HTTP to an HTTPS server.
-                    raise errors.NoSSLError
+        except (
+            ssl.SSLEOFError,
+            ssl.SSLZeroReturnError,
+        ) as tls_connection_drop_error:
+            raise errors.FatalSSLAlert(
+                *tls_connection_drop_error.args,
+            ) from tls_connection_drop_error
+        except ssl.SSLError as generic_tls_error:
+            peer_speaks_plain_http_over_https = (
+                generic_tls_error.errno == ssl.SSL_ERROR_SSL and
+                _assert_ssl_exc_contains(generic_tls_error, 'http request')
+            )
+            if peer_speaks_plain_http_over_https:
+                reraised_connection_drop_exc_cls = errors.NoSSLError
+            else:
+                reraised_connection_drop_exc_cls = errors.FatalSSLAlert
 
-                # Check if it's one of the known errors
-                # Errors that are caught by PyOpenSSL, but thrown by
-                # built-in ssl
-                _block_errors = (
-                    'unknown protocol', 'unknown ca', 'unknown_ca',
-                    'unknown error',
-                    'https proxy request', 'inappropriate fallback',
-                    'wrong version number',
-                    'no shared cipher', 'certificate unknown',
-                    'ccs received early',
-                    'certificate verify failed',  # client cert w/o trusted CA
-                    'version too low',  # caused by SSL3 connections
-                    'unsupported protocol',  # caused by TLS1 connections
-                )
-                if _assert_ssl_exc_contains(ex, *_block_errors):
-                    # Accepted error, let's pass
-                    return EMPTY_RESULT
-            elif _assert_ssl_exc_contains(ex, 'handshake operation timed out'):
-                # This error is thrown by builtin SSL after a timeout
-                # when client is speaking HTTP to an HTTPS server.
-                # The connection can safely be dropped.
-                return EMPTY_RESULT
-            raise
-        except generic_socket_error as exc:
-            """It is unclear why exactly this happens.
+            raise reraised_connection_drop_exc_cls(
+                *generic_tls_error.args,
+            ) from generic_tls_error
+        except OSError as tcp_connection_drop_error:
+            raise errors.FatalSSLAlert(
+                *tcp_connection_drop_error.args,
+            ) from tcp_connection_drop_error
 
-            It's reproducible only with openssl>1.0 and stdlib
-            :py:mod:`ssl` wrapper.
-            In CherryPy it's triggered by Checker plugin, which connects
-            to the app listening to the socket port in TLS mode via plain
-            HTTP during startup (from the same process).
-
-
-            Ref: https://github.com/cherrypy/cherrypy/issues/1618
-            """
-            is_error0 = exc.args == (0, 'Error')
-
-            if is_error0 and IS_ABOVE_OPENSSL10:
-                return EMPTY_RESULT
-            raise
         return s, self.get_environ(s)
 
     def get_environ(self, sock):

lib/cheroot/ssl/pyopenssl.py

@@ -150,7 +150,7 @@ class SSLFileobjectMixin:
         return self._safe_call(
             False,
             super(SSLFileobjectMixin, self).sendall,
-            *args, **kwargs
+            *args, **kwargs,
         )
 
     def send(self, *args, **kwargs):
@@ -158,7 +158,7 @@ class SSLFileobjectMixin:
         return self._safe_call(
             False,
             super(SSLFileobjectMixin, self).send,
-            *args, **kwargs
+            *args, **kwargs,
         )
 
 
@@ -196,6 +196,7 @@ class SSLConnectionProxyMeta:
 
         def lock_decorator(method):
             """Create a proxy method for a new class."""
+
             def proxy_wrapper(self, *args):
                 self._lock.acquire()
                 try:
@@ -212,6 +213,7 @@ class SSLConnectionProxyMeta:
 
         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_

lib/cheroot/test/conftest.py

@@ -12,7 +12,10 @@ import pytest
 from .._compat import IS_MACOS, IS_WINDOWS  # noqa: WPS436
 from ..server import Gateway, HTTPServer
 from ..testing import (  # noqa: F401  # pylint: disable=unused-import
-    native_server, wsgi_server,
+    native_server,
+    thread_and_wsgi_server,
+    thread_and_native_server,
+    wsgi_server,
 )
 from ..testing import get_server_client
 
@@ -31,6 +34,28 @@ def http_request_timeout():
     return computed_timeout
 
 
+@pytest.fixture
+# pylint: disable=redefined-outer-name
+def wsgi_server_thread(thread_and_wsgi_server):  # noqa: F811
+    """Set up and tear down a Cheroot WSGI server instance.
+
+    This exposes the server thread.
+    """
+    server_thread, _srv = thread_and_wsgi_server
+    return server_thread
+
+
+@pytest.fixture
+# pylint: disable=redefined-outer-name
+def native_server_thread(thread_and_native_server):  # noqa: F811
+    """Set up and tear down a Cheroot HTTP server instance.
+
+    This exposes the server thread.
+    """
+    server_thread, _srv = thread_and_native_server
+    return server_thread
+
+
 @pytest.fixture
 # pylint: disable=redefined-outer-name
 def wsgi_server_client(wsgi_server):  # noqa: F811

lib/cheroot/test/test_conn.py

@@ -1,7 +1,9 @@
 """Tests for TCP connection handling, including proper and timely close."""
 
 import errno
+from re import match as _matches_pattern
 import socket
+import sys
 import time
 import logging
 import traceback as traceback_
@@ -17,6 +19,7 @@ from cheroot._compat import IS_CI, IS_MACOS, IS_PYPY, IS_WINDOWS
 import cheroot.server
 
 
+IS_PY36 = sys.version_info[:2] == (3, 6)
 IS_SLOW_ENV = IS_MACOS or IS_WINDOWS
 
 
@@ -53,7 +56,8 @@ class Controller(helper.Controller):
                 "'POST' != request.method %r" %
                 req.environ['REQUEST_METHOD'],
             )
-        return "thanks for '%s'" % req.environ['wsgi.input'].read()
+        input_contents = req.environ['wsgi.input'].read().decode('utf-8')
+        return f"thanks for '{input_contents !s}'"
 
     def custom_204(req, resp):
         """Render response with status 204."""
@@ -605,18 +609,18 @@ def test_keepalive_conn_management(test_client):
         pytest.param(RuntimeError, 666, True, id='RuntimeError(666)'),
         pytest.param(socket.error, -1, True, id='socket.error(-1)'),
     ) + (
-            pytest.param(
-                ConnectionResetError, errno.ECONNRESET, False,
-                id='ConnectionResetError(ECONNRESET)',
-            ),
-            pytest.param(
-                BrokenPipeError, errno.EPIPE, False,
-                id='BrokenPipeError(EPIPE)',
-            ),
-            pytest.param(
-                BrokenPipeError, errno.ESHUTDOWN, False,
-                id='BrokenPipeError(ESHUTDOWN)',
-            ),
+        pytest.param(
+            ConnectionResetError, errno.ECONNRESET, False,
+            id='ConnectionResetError(ECONNRESET)',
+        ),
+        pytest.param(
+            BrokenPipeError, errno.EPIPE, False,
+            id='BrokenPipeError(EPIPE)',
+        ),
+        pytest.param(
+            BrokenPipeError, errno.ESHUTDOWN, False,
+            id='BrokenPipeError(ESHUTDOWN)',
+        ),
     ),
 )
 def test_broken_connection_during_tcp_fin(
@@ -699,6 +703,275 @@ def test_broken_connection_during_tcp_fin(
     assert _close_kernel_socket.exception_leaked is exception_leaks
 
 
+def test_broken_connection_during_http_communication_fallback(  # noqa: WPS118
+        monkeypatch,
+        test_client,
+        testing_server,
+        wsgi_server_thread,
+):
+    """Test that unhandled internal error cascades into shutdown."""
+    def _raise_connection_reset(*_args, **_kwargs):
+        raise ConnectionResetError(666)
+
+    def _read_request_line(self):
+        monkeypatch.setattr(self.conn.rfile, 'close', _raise_connection_reset)
+        monkeypatch.setattr(self.conn.wfile, 'write', _raise_connection_reset)
+        _raise_connection_reset()
+
+    monkeypatch.setattr(
+        test_client.server_instance.ConnectionClass.RequestHandlerClass,
+        'read_request_line',
+        _read_request_line,
+    )
+
+    test_client.get_connection().send(b'GET / HTTP/1.1')
+    wsgi_server_thread.join()  # no extra logs upon server termination
+
+    actual_log_entries = testing_server.error_log.calls[:]
+    testing_server.error_log.calls.clear()  # prevent post-test assertions
+
+    expected_log_entries = (
+        (logging.WARNING, r'^socket\.error 666$'),
+        (
+            logging.INFO,
+            '^Got a connection error while handling a connection '
+            r'from .*:\d{1,5} \(666\)',
+        ),
+        (
+            logging.CRITICAL,
+            r'A fatal exception happened\. Setting the server interrupt flag '
+            r'to ConnectionResetError\(666,?\) and giving up\.\n\nPlease, '
+            'report this on the Cheroot tracker at '
+            r'<https://github\.com/cherrypy/cheroot/issues/new/choose>, '
+            'providing a full reproducer with as much context and details '
+            r'as possible\.$',
+        ),
+    )
+
+    assert len(actual_log_entries) == len(expected_log_entries)
+
+    for (  # noqa: WPS352
+            (expected_log_level, expected_msg_regex),
+            (actual_msg, actual_log_level, _tb),
+    ) in zip(expected_log_entries, actual_log_entries):
+        assert expected_log_level == actual_log_level
+        assert _matches_pattern(expected_msg_regex, actual_msg) is not None, (
+            f'{actual_msg !r} does not match {expected_msg_regex !r}'
+        )
+
+
+def test_kb_int_from_http_handler(
+        test_client,
+        testing_server,
+        wsgi_server_thread,
+):
+    """Test that a keyboard interrupt from HTTP handler causes shutdown."""
+    def _trigger_kb_intr(_req, _resp):
+        raise KeyboardInterrupt('simulated test handler keyboard interrupt')
+    testing_server.wsgi_app.handlers['/kb_intr'] = _trigger_kb_intr
+
+    http_conn = test_client.get_connection()
+    http_conn.putrequest('GET', '/kb_intr', skip_host=True)
+    http_conn.putheader('Host', http_conn.host)
+    http_conn.endheaders()
+    wsgi_server_thread.join()  # no extra logs upon server termination
+
+    actual_log_entries = testing_server.error_log.calls[:]
+    testing_server.error_log.calls.clear()  # prevent post-test assertions
+
+    expected_log_entries = (
+        (
+            logging.DEBUG,
+            '^Got a server shutdown request while handling a connection '
+            r'from .*:\d{1,5} \(simulated test handler keyboard interrupt\)$',
+        ),
+        (
+            logging.DEBUG,
+            '^Setting the server interrupt flag to KeyboardInterrupt'
+            r"\('simulated test handler keyboard interrupt',?\)$",
+        ),
+        (
+            logging.INFO,
+            '^Keyboard Interrupt: shutting down$',
+        ),
+    )
+
+    assert len(actual_log_entries) == len(expected_log_entries)
+
+    for (  # noqa: WPS352
+            (expected_log_level, expected_msg_regex),
+            (actual_msg, actual_log_level, _tb),
+    ) in zip(expected_log_entries, actual_log_entries):
+        assert expected_log_level == actual_log_level
+        assert _matches_pattern(expected_msg_regex, actual_msg) is not None, (
+            f'{actual_msg !r} does not match {expected_msg_regex !r}'
+        )
+
+
+@pytest.mark.xfail(
+    IS_CI and IS_PYPY and IS_PY36 and not IS_SLOW_ENV,
+    reason='Fails under PyPy 3.6 under Ubuntu 20.04 in CI for unknown reason',
+    # NOTE: Actually covers any Linux
+    strict=False,
+)
+def test_unhandled_exception_in_request_handler(
+        mocker,
+        monkeypatch,
+        test_client,
+        testing_server,
+        wsgi_server_thread,
+):
+    """Ensure worker threads are resilient to in-handler exceptions."""
+
+    class SillyMistake(BaseException):  # noqa: WPS418, WPS431
+        """A simulated crash within an HTTP handler."""
+
+    def _trigger_scary_exc(_req, _resp):
+        raise SillyMistake('simulated unhandled exception 💣 in test handler')
+
+    testing_server.wsgi_app.handlers['/scary_exc'] = _trigger_scary_exc
+
+    server_connection_close_spy = mocker.spy(
+        test_client.server_instance.ConnectionClass,
+        'close',
+    )
+
+    http_conn = test_client.get_connection()
+    http_conn.putrequest('GET', '/scary_exc', skip_host=True)
+    http_conn.putheader('Host', http_conn.host)
+    http_conn.endheaders()
+
+    # NOTE: This spy ensure the log entry gets recorded before we're testing
+    # NOTE: them and before server shutdown, preserving their order and making
+    # NOTE: the log entry presence non-flaky.
+    while not server_connection_close_spy.called:  # noqa: WPS328
+        pass
+
+    assert len(testing_server.requests._threads) == 10
+    while testing_server.requests.idle < 10:  # noqa: WPS328
+        pass
+    assert len(testing_server.requests._threads) == 10
+    testing_server.interrupt = SystemExit('test requesting shutdown')
+    assert not testing_server.requests._threads
+    wsgi_server_thread.join()  # no extra logs upon server termination
+
+    actual_log_entries = testing_server.error_log.calls[:]
+    testing_server.error_log.calls.clear()  # prevent post-test assertions
+
+    expected_log_entries = (
+        (
+            logging.ERROR,
+            '^Unhandled error while processing an incoming connection '
+            'SillyMistake'
+            r"\('simulated unhandled exception 💣 in test handler',?\)$",
+        ),
+        (
+            logging.INFO,
+            '^SystemExit raised: shutting down$',
+        ),
+    )
+
+    assert len(actual_log_entries) == len(expected_log_entries)
+
+    for (  # noqa: WPS352
+            (expected_log_level, expected_msg_regex),
+            (actual_msg, actual_log_level, _tb),
+    ) in zip(expected_log_entries, actual_log_entries):
+        assert expected_log_level == actual_log_level
+        assert _matches_pattern(expected_msg_regex, actual_msg) is not None, (
+            f'{actual_msg !r} does not match {expected_msg_regex !r}'
+        )
+
+
+@pytest.mark.xfail(
+    IS_CI and IS_PYPY and IS_PY36 and not IS_SLOW_ENV,
+    reason='Fails under PyPy 3.6 under Ubuntu 20.04 in CI for unknown reason',
+    # NOTE: Actually covers any Linux
+    strict=False,
+)
+def test_remains_alive_post_unhandled_exception(
+        mocker,
+        monkeypatch,
+        test_client,
+        testing_server,
+        wsgi_server_thread,
+):
+    """Ensure worker threads are resilient to unhandled exceptions."""
+
+    class ScaryCrash(BaseException):  # noqa: WPS418, WPS431
+        """A simulated crash during HTTP parsing."""
+
+    _orig_read_request_line = (
+        test_client.server_instance.
+        ConnectionClass.RequestHandlerClass.
+        read_request_line
+    )
+
+    def _read_request_line(self):
+        _orig_read_request_line(self)
+        raise ScaryCrash(666)
+
+    monkeypatch.setattr(
+        test_client.server_instance.ConnectionClass.RequestHandlerClass,
+        'read_request_line',
+        _read_request_line,
+    )
+
+    server_connection_close_spy = mocker.spy(
+        test_client.server_instance.ConnectionClass,
+        'close',
+    )
+
+    # NOTE: The initial worker thread count is 10.
+    assert len(testing_server.requests._threads) == 10
+
+    test_client.get_connection().send(b'GET / HTTP/1.1')
+
+    # NOTE: This spy ensure the log entry gets recorded before we're testing
+    # NOTE: them and before server shutdown, preserving their order and making
+    # NOTE: the log entry presence non-flaky.
+    while not server_connection_close_spy.called:  # noqa: WPS328
+        pass
+
+    # NOTE: This checks for whether there's any crashed threads
+    while testing_server.requests.idle < 10:  # noqa: WPS328
+        pass
+    assert len(testing_server.requests._threads) == 10
+    assert all(
+        worker_thread.is_alive()
+        for worker_thread in testing_server.requests._threads
+    )
+    testing_server.interrupt = SystemExit('test requesting shutdown')
+    assert not testing_server.requests._threads
+    wsgi_server_thread.join()  # no extra logs upon server termination
+
+    actual_log_entries = testing_server.error_log.calls[:]
+    testing_server.error_log.calls.clear()  # prevent post-test assertions
+
+    expected_log_entries = (
+        (
+            logging.ERROR,
+            '^Unhandled error while processing an incoming connection '
+            r'ScaryCrash\(666,?\)$',
+        ),
+        (
+            logging.INFO,
+            '^SystemExit raised: shutting down$',
+        ),
+    )
+
+    assert len(actual_log_entries) == len(expected_log_entries)
+
+    for (  # noqa: WPS352
+            (expected_log_level, expected_msg_regex),
+            (actual_msg, actual_log_level, _tb),
+    ) in zip(expected_log_entries, actual_log_entries):
+        assert expected_log_level == actual_log_level
+        assert _matches_pattern(expected_msg_regex, actual_msg) is not None, (
+            f'{actual_msg !r} does not match {expected_msg_regex !r}'
+        )
+
+
 @pytest.mark.parametrize(
     'timeout_before_headers',
     (
@@ -917,7 +1190,7 @@ def test_100_Continue(test_client):
     status_line, _actual_headers, actual_resp_body = webtest.shb(response)
     actual_status = int(status_line[:3])
     assert actual_status == 200
-    expected_resp_body = ("thanks for '%s'" % body).encode()
+    expected_resp_body = f"thanks for '{body.decode() !s}'".encode()
     assert actual_resp_body == expected_resp_body
     conn.close()
 
@@ -987,7 +1260,7 @@ def test_readall_or_close(test_client, max_request_body_size):
     status_line, actual_headers, actual_resp_body = webtest.shb(response)
     actual_status = int(status_line[:3])
     assert actual_status == 200
-    expected_resp_body = ("thanks for '%s'" % body).encode()
+    expected_resp_body = f"thanks for '{body.decode() !s}'".encode()
     assert actual_resp_body == expected_resp_body
     conn.close()
 

lib/cheroot/test/test_core.py

@@ -134,7 +134,7 @@ def test_query_string_request(test_client):
         '/hello',  # plain
         '/query_string?test=True',  # query
         '/{0}?{1}={2}'.format(  # quoted unicode
-            *map(urllib.parse.quote, ('Юххууу', 'ї', 'йо'))
+            *map(urllib.parse.quote, ('Юххууу', 'ї', 'йо')),
         ),
     ),
 )

lib/cheroot/testing.py

@@ -31,7 +31,7 @@ config = {
 
 
 @contextmanager
-def cheroot_server(server_factory):
+def cheroot_server(server_factory):  # noqa: WPS210
     """Set up and tear down a Cheroot server instance."""
     conf = config[server_factory].copy()
     bind_port = conf.pop('bind_addr')[-1]
@@ -41,7 +41,7 @@ def cheroot_server(server_factory):
             actual_bind_addr = (interface, bind_port)
             httpserver = server_factory(  # create it
                 bind_addr=actual_bind_addr,
-                **conf
+                **conf,
             )
         except OSError:
             pass
@@ -50,27 +50,52 @@ def cheroot_server(server_factory):
 
     httpserver.shutdown_timeout = 0  # Speed-up tests teardown
 
-    threading.Thread(target=httpserver.safe_start).start()  # spawn it
+    # FIXME: Expose this thread through a fixture so that it
+    # FIXME: could be awaited in tests.
+    server_thread = threading.Thread(target=httpserver.safe_start)
+    server_thread.start()  # spawn it
     while not httpserver.ready:  # wait until fully initialized and bound
         time.sleep(0.1)
 
-    yield httpserver
-
-    httpserver.stop()  # destroy it
+    try:
+        yield server_thread, httpserver
+    finally:
+        httpserver.stop()  # destroy it
+        server_thread.join()  # wait for the thread to be turn down
 
 
 @pytest.fixture
-def wsgi_server():
+def thread_and_wsgi_server():
+    """Set up and tear down a Cheroot WSGI server instance.
+
+    This emits a tuple of a thread and a server instance.
+    """
+    with cheroot_server(cheroot.wsgi.Server) as (server_thread, srv):
+        yield server_thread, srv
+
+
+@pytest.fixture
+def thread_and_native_server():
+    """Set up and tear down a Cheroot HTTP server instance.
+
+    This emits a tuple of a thread and a server instance.
+    """
+    with cheroot_server(cheroot.server.HTTPServer) as (server_thread, srv):
+        yield server_thread, srv
+
+
+@pytest.fixture
+def wsgi_server(thread_and_wsgi_server):  # noqa: WPS442
     """Set up and tear down a Cheroot WSGI server instance."""
-    with cheroot_server(cheroot.wsgi.Server) as srv:
-        yield srv
+    _server_thread, srv = thread_and_wsgi_server
+    return srv
 
 
 @pytest.fixture
-def native_server():
+def native_server(thread_and_native_server):  # noqa: WPS442
     """Set up and tear down a Cheroot HTTP server instance."""
-    with cheroot_server(cheroot.server.HTTPServer) as srv:
-        yield srv
+    _server_thread, srv = thread_and_native_server
+    return srv
 
 
 class _TestClient:

lib/cheroot/workers/threadpool.py

@@ -6,6 +6,7 @@
 """
 
 import collections
+import logging
 import threading
 import time
 import socket
@@ -30,7 +31,7 @@ class TrueyZero:
 
 trueyzero = TrueyZero()
 
-_SHUTDOWNREQUEST = None
+_SHUTDOWNREQUEST = object()
 
 
 class WorkerThread(threading.Thread):
@@ -99,39 +100,127 @@ class WorkerThread(threading.Thread):
         threading.Thread.__init__(self)
 
     def run(self):
-        """Process incoming HTTP connections.
+        """Set up incoming HTTP connection processing loop.
 
-        Retrieves incoming connections from thread pool.
+        This is the thread's entry-point. It performs lop-layer
+        exception handling and interrupt processing.
+        :exc:`KeyboardInterrupt` and :exc:`SystemExit` bubbling up
+        from the inner-layer code constitute a global server interrupt
+        request. When they happen, the worker thread exits.
+
+        :raises BaseException: when an unexpected non-interrupt
+                               exception leaks from the inner layers
+
+        # noqa: DAR401 KeyboardInterrupt SystemExit
         """
         self.server.stats['Worker Threads'][self.name] = self.stats
+        self.ready = True
         try:
-            self.ready = True
-            while True:
-                conn = self.server.requests.get()
-                if conn is _SHUTDOWNREQUEST:
-                    return
+            self._process_connections_until_interrupted()
+        except (KeyboardInterrupt, SystemExit) as interrupt_exc:
+            interrupt_cause = interrupt_exc.__cause__ or interrupt_exc
+            self.server.error_log(
+                f'Setting the server interrupt flag to {interrupt_cause !r}',
+                level=logging.DEBUG,
+            )
+            self.server.interrupt = interrupt_cause
+        except BaseException as underlying_exc:  # noqa: WPS424
+            # NOTE: This is the last resort logging with the last dying breath
+            # NOTE: of the worker. It is only reachable when exceptions happen
+            # NOTE: in the `finally` branch of the internal try/except block.
+            self.server.error_log(
+                'A fatal exception happened. Setting the server interrupt flag'
+                f' to {underlying_exc !r} and giving up.'
+                '\N{NEW LINE}\N{NEW LINE}'
+                'Please, report this on the Cheroot tracker at '
+                '<https://github.com/cherrypy/cheroot/issues/new/choose>, '
+                'providing a full reproducer with as much context and details as possible.',
+                level=logging.CRITICAL,
+                traceback=True,
+            )
+            self.server.interrupt = underlying_exc
+            raise
+        finally:
+            self.ready = False
 
-                self.conn = conn
-                is_stats_enabled = self.server.stats['Enabled']
+    def _process_connections_until_interrupted(self):
+        """Process incoming HTTP connections in an infinite loop.
+
+        Retrieves incoming connections from thread pool, processing
+        them one by one.
+
+        :raises SystemExit: on the internal requests to stop the
+                            server instance
+        """
+        while True:
+            conn = self.server.requests.get()
+            if conn is _SHUTDOWNREQUEST:
+                return
+
+            self.conn = conn
+            is_stats_enabled = self.server.stats['Enabled']
+            if is_stats_enabled:
+                self.start_time = time.time()
+            keep_conn_open = False
+            try:
+                keep_conn_open = conn.communicate()
+            except ConnectionError as connection_error:
+                keep_conn_open = False  # Drop the connection cleanly
+                self.server.error_log(
+                    'Got a connection error while handling a '
+                    f'connection from {conn.remote_addr !s}:'
+                    f'{conn.remote_port !s} ({connection_error !s})',
+                    level=logging.INFO,
+                )
+                continue
+            except (KeyboardInterrupt, SystemExit) as shutdown_request:
+                # Shutdown request
+                keep_conn_open = False  # Drop the connection cleanly
+                self.server.error_log(
+                    'Got a server shutdown request while handling a '
+                    f'connection from {conn.remote_addr !s}:'
+                    f'{conn.remote_port !s} ({shutdown_request !s})',
+                    level=logging.DEBUG,
+                )
+                raise SystemExit(
+                    str(shutdown_request),
+                ) from shutdown_request
+            except BaseException as unhandled_error:  # noqa: WPS424
+                # NOTE: Only a shutdown request should bubble up to the
+                # NOTE: external cleanup code. Otherwise, this thread dies.
+                # NOTE: If this were to happen, the threadpool would still
+                # NOTE: list a dead thread without knowing its state. And
+                # NOTE: the calling code would fail to schedule processing
+                # NOTE: of new requests.
+                self.server.error_log(
+                    'Unhandled error while processing an incoming '
+                    f'connection {unhandled_error !r}',
+                    level=logging.ERROR,
+                    traceback=True,
+                )
+                continue  # Prevent the thread from dying
+            finally:
+                # NOTE: Any exceptions coming from within `finally` may
+                # NOTE: kill the thread, causing the threadpool to only
+                # NOTE: contain references to dead threads rendering the
+                # NOTE: server defunct, effectively meaning a DoS.
+                # NOTE: Ideally, things called here should process
+                # NOTE: everything recoverable internally. Any unhandled
+                # NOTE: errors will bubble up into the outer try/except
+                # NOTE: block. They will be treated as fatal and turned
+                # NOTE: into server shutdown requests and then reraised
+                # NOTE: unconditionally.
+                if keep_conn_open:
+                    self.server.put_conn(conn)
+                else:
+                    conn.close()
                 if is_stats_enabled:
-                    self.start_time = time.time()
-                keep_conn_open = False
-                try:
-                    keep_conn_open = conn.communicate()
-                finally:
-                    if keep_conn_open:
-                        self.server.put_conn(conn)
-                    else:
-                        conn.close()
-                    if is_stats_enabled:
-                        self.requests_seen += self.conn.requests_seen
-                        self.bytes_read += self.conn.rfile.bytes_read
-                        self.bytes_written += self.conn.wfile.bytes_written
-                        self.work_time += time.time() - self.start_time
-                        self.start_time = None
-                    self.conn = None
-        except (KeyboardInterrupt, SystemExit) as ex:
-            self.server.interrupt = ex
+                    self.requests_seen += conn.requests_seen
+                    self.bytes_read += conn.rfile.bytes_read
+                    self.bytes_written += conn.wfile.bytes_written
+                    self.work_time += time.time() - self.start_time
+                    self.start_time = None
+                self.conn = None
 
 
 class ThreadPool:

requirements.txt

@@ -4,7 +4,7 @@ backports.zoneinfo==0.2.1;python_version<"3.9"
 beautifulsoup4==4.12.3
 bleach==6.1.0
 certifi==2024.2.2
-cheroot==10.0.0
+cheroot==10.0.1
 cherrypy==18.9.0
 cloudinary==1.40.0
 distro==1.9.0