From 122d4f4a454fafbf1b250d6ce06aae27cb171f61 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 5 Aug 2026 14:53:54 -0700 Subject: [PATCH] Fix /events AttributeError on browsers hitting the SSE endpoint Refs: #69958 A browser (or any Accept: text/html client) hitting /events triggered html_override_tool to raise cherrypy.InternalRedirect. That redirect propagates through cherrypy._cpwsgi.AppResponse.__init__ where the `except BaseException: self.close()` cleanup path in cherrypy 18.10.0 crashes with AttributeError: 'AppResponse' object has no attribute 'iter_response' (iter_response is only assigned after self.run() returns). Skip the html override for any handler that has opted into response.stream (the SSE /events endpoint and the /ws websocket endpoint). Diverting a streaming handler to the HTML app is wrong regardless of the cherrypy bug -- SSE clients ask for text/event-stream, not text/html -- so this narrows the tool's blast radius as well. --- changelog/69958.fixed.md | 1 + salt/netapi/rest_cherrypy/app.py | 13 +++ .../netapi/cherrypy/test_html_override.py | 89 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 changelog/69958.fixed.md create mode 100644 tests/pytests/unit/netapi/cherrypy/test_html_override.py diff --git a/changelog/69958.fixed.md b/changelog/69958.fixed.md new file mode 100644 index 000000000000..84676f678f34 --- /dev/null +++ b/changelog/69958.fixed.md @@ -0,0 +1 @@ +Fixed ``AttributeError: 'AppResponse' object has no attribute 'iter_response'`` when a browser (or any ``Accept: text/html`` client) hit the ``/events`` SSE endpoint. ``salt.netapi.rest_cherrypy.app.html_override_tool`` now skips endpoints that opt into ``response.stream`` instead of raising ``cherrypy.InternalRedirect`` on them, avoiding a cherrypy 18.10.0 WSGI cleanup crash. diff --git a/salt/netapi/rest_cherrypy/app.py b/salt/netapi/rest_cherrypy/app.py index 6d66875838cb..03ed65869701 100644 --- a/salt/netapi/rest_cherrypy/app.py +++ b/salt/netapi/rest_cherrypy/app.py @@ -660,6 +660,19 @@ def html_override_tool(): if request.path_info.startswith(url_blacklist): return + # Streaming endpoints (``/events`` SSE, ``/ws`` websocket) must never be + # diverted to the HTML app. A browser hitting ``/events`` sends + # ``Accept: text/html,*/*`` which would otherwise raise + # ``InternalRedirect`` here; that redirect propagates through + # ``cherrypy._cpwsgi.AppResponse.__init__`` where the ``except + # BaseException: self.close()`` cleanup path in cherrypy 18.10.0 crashes + # with ``AttributeError: 'AppResponse' object has no attribute + # 'iter_response'`` (``iter_response`` is only assigned after + # ``self.run()`` returns). Skip the redirect for any handler that has + # opted into ``response.stream``. See issue #69958. + if request.config.get("response.stream"): + return + if request.headers.get("Accept") == "*/*": return diff --git a/tests/pytests/unit/netapi/cherrypy/test_html_override.py b/tests/pytests/unit/netapi/cherrypy/test_html_override.py new file mode 100644 index 000000000000..0aff7d39b554 --- /dev/null +++ b/tests/pytests/unit/netapi/cherrypy/test_html_override.py @@ -0,0 +1,89 @@ +""" +Tests for ``salt.netapi.rest_cherrypy.app.html_override_tool``. + +The tool short-circuits normal request handling to serve the single-page +JS app when a browser asks for ``text/html``. It must not fire for +endpoints that stream (``/events`` SSE, ``/ws`` websocket); if it does, +the ``cherrypy.InternalRedirect`` it raises propagates through +``cherrypy._cpwsgi.AppResponse.__init__`` where cherrypy 18.10.0's +``except BaseException: self.close()`` cleanup path crashes with +``AttributeError: 'AppResponse' object has no attribute 'iter_response'`` +because ``iter_response`` is only assigned after the wrapped ``run()`` +returns. See issue #69958. +""" + +from types import SimpleNamespace + +import pytest + +import salt.netapi.rest_cherrypy.app as cherrypy_app +from tests.support.mock import patch + + +class _MockHTTPError(Exception): + def __init__(self, status=None, message=None): + self.status = status + self.message = message + super().__init__(f"{status}: {message}") + + +class _MockInternalRedirect(Exception): + def __init__(self, path): + self.path = path + super().__init__(path) + + +def _cherrypy_for_html_override(path_info, accept, request_config=None): + """Build a ``cherrypy``-shaped namespace rich enough that + ``html_override_tool`` can reach its redirect decision.""" + apiopts = {"app": "/opt/salt-app", "app_path": "/app", "static_path": "/static"} + return SimpleNamespace( + config={"apiopts": apiopts}, + request=SimpleNamespace( + path_info=path_info, + headers={"Accept": accept}, + config=request_config or {}, + ), + HTTPError=_MockHTTPError, + InternalRedirect=_MockInternalRedirect, + # cherrypy.lib.cptools.accept is monkey-patched per-test. + lib=SimpleNamespace( + cptools=SimpleNamespace(accept=lambda *a, **kw: "text/html") + ), + ) + + +def test_html_override_skips_streaming_endpoints(): + """Regression for #69958. + + A browser hitting ``/events`` sends ``Accept: text/html,*/*``. Prior + to the fix, ``html_override_tool`` raised + ``cherrypy.InternalRedirect('/app')`` and cherrypy 18.10.0's WSGI + layer then crashed with ``AttributeError: 'AppResponse' object has + no attribute 'iter_response'``. The ``/events`` handler opts into + ``response.stream = True`` via its ``_cp_config``; the tool must + honor that and return without diverting the request.""" + cherrypy_mock = _cherrypy_for_html_override( + path_info="/events", + accept="text/html,application/xhtml+xml,*/*;q=0.8", + request_config={"response.stream": True}, + ) + with patch("salt.netapi.rest_cherrypy.app.cherrypy", cherrypy_mock): + # Must return None (no redirect). Any raised exception here + # would reproduce the reported bug. + assert cherrypy_app.html_override_tool() is None + + +def test_html_override_still_redirects_non_streaming_html_request(): + """Sanity check: the tool's original behavior for non-streaming + endpoints is preserved. A browser hitting ``/`` with an HTML Accept + header still gets diverted to the app.""" + cherrypy_mock = _cherrypy_for_html_override( + path_info="/", + accept="text/html,application/xhtml+xml,*/*;q=0.8", + request_config={}, + ) + with patch("salt.netapi.rest_cherrypy.app.cherrypy", cherrypy_mock): + with pytest.raises(_MockInternalRedirect) as excinfo: + cherrypy_app.html_override_tool() + assert excinfo.value.path == "/app"