Use web.HTTPError to safely return errors to browser

jupyterhub · May 29, 2024 · 3909949 · 3909949
1 parent 652849c
commit 3909949
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 16 deletions.
diff --git a/jupyter_server_proxy/handlers.py b/jupyter_server_proxy/handlers.py
@@ -8,7 +8,6 @@
 import socket
 from asyncio import Lock
 from copy import copy
-from html import escape
 from tempfile import mkdtemp
 from urllib.parse import quote, urlparse, urlunparse
 
@@ -324,14 +323,11 @@ async def proxy(self, host, port, proxied_path):
         """
 
         if not self._check_host_allowlist(host):
-            self.set_status(403)
-            self.write(
-                "Host '{host}' is not allowed. "
-                "See https://jupyter-server-proxy.readthedocs.io/en/latest/arbitrary-ports-hosts.html for info.".format(
-                    host=escape(host)
-                )
+            raise web.HTTPError(
+                403,
+                f"Host '{host}' is not allowed. "
+                "See https://jupyter-server-proxy.readthedocs.io/en/latest/arbitrary-ports-hosts.html for info.",
             )
-            return
 
         # Remove hop-by-hop headers that don't necessarily apply to the request we are making
         # to the backend. See https://github.com/jupyterhub/jupyter-server-proxy/pull/328
@@ -392,9 +388,7 @@ async def proxy(self, host, port, proxied_path):
             # Ref: https://www.tornadoweb.org/en/stable/httpclient.html#tornado.httpclient.AsyncHTTPClient.fetch
             if err.code == 599:
                 self._record_activity()
-                self.set_status(599)
-                self.write(escape(str(err)))
-                return
+                raise web.HTTPError(599, str(err))
             else:
                 raise
 
@@ -403,8 +397,7 @@ async def proxy(self, host, port, proxied_path):
 
         # For all non http errors...
         if response.error and type(response.error) is not httpclient.HTTPError:
-            self.set_status(500)
-            self.write(escape(str(response.error)))
+            raise web.HTTPError(500, str(response.error))
         else:
             # Represent the original response as a RewritableResponse object.
             original_response = RewritableResponse(orig_response=response)

diff --git a/tests/test_proxies.py b/tests/test_proxies.py
@@ -255,12 +255,15 @@ def test_server_proxy_host_absolute(a_server_port_and_token: Tuple[int, str]) ->
     assert "X-Proxycontextpath" not in s
 
 
-def test_server_proxy_host_invalid(a_server_port_and_token: Tuple[int, str]) -> None:
+@pytest.mark.parametrize("absolute", ["", "/absolute"])
+def test_server_proxy_host_invalid(
+    a_server_port_and_token: Tuple[int, str], absolute: str
+) -> None:
     PORT, TOKEN = a_server_port_and_token
-    r = request_get(PORT, "/proxy/absolute/<invalid>:54321/", TOKEN)
+    r = request_get(PORT, f"/proxy{absolute}/<invalid>:54321/", TOKEN)
     assert r.code == 403
     s = r.read().decode("ascii")
-    assert s.startswith("Host '&lt;invalid&gt;' is not allowed.")
+    assert "Host &#39;&lt;invalid&gt;&#39; is not allowed." in s
 
 
 def test_server_proxy_port_non_service_rewrite_response(