From b75474e4e3fa47204bf44054b83bcbdf749dcae5 Mon Sep 17 00:00:00 2001 From: 2xB <31772910+2xB@users.noreply.github.com> Date: Sun, 13 Apr 2025 02:03:32 +0200 Subject: [PATCH 1/6] Fix websocket requests not mapped via mappath This is done by instead of WebSocketHandlerMixin providing the default `get` method and delegating to `ProxyHandler.http_get`, it now provides `get_websocket` and `ProxyHandler.proxy` delegates to it. Code is also removed that was never reached since it checked for the "Upgrade" HTTP header after removing it among others defined in `hop_by_hop_headers`. Fixes #525 --- jupyter_server_proxy/handlers.py | 20 +++++++++----------- jupyter_server_proxy/websocket.py | 7 ++----- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/jupyter_server_proxy/handlers.py b/jupyter_server_proxy/handlers.py index a5987fc8..278c2a47 100644 --- a/jupyter_server_proxy/handlers.py +++ b/jupyter_server_proxy/handlers.py @@ -16,7 +16,7 @@ from jupyter_server.base.handlers import JupyterHandler, utcnow from jupyter_server.utils import ensure_async, url_path_join from simpervisor import SupervisedProcess -from tornado import httpclient, httputil, web +from tornado import httpclient, httputil, web, websocket from tornado.simple_httpclient import SimpleAsyncHTTPClient from traitlets import Bytes, Dict, Instance, Integer, Unicode, Union, default, observe from traitlets.traitlets import HasTraits @@ -171,6 +171,9 @@ async def http_get(self, host, port, proxy_path=""): raise NotImplementedError( "Subclasses of ProxyHandler should implement http_get" ) + + async def get(self, *args, **kwargs): + return await self.http_get(*args, **kwargs) def post(self, host, port, proxy_path=""): raise NotImplementedError( @@ -333,6 +336,11 @@ async def proxy(self, host, port, proxied_path): "See https://jupyter-server-proxy.readthedocs.io/en/latest/arbitrary-ports-hosts.html for info.", ) + self._record_activity() + + if self.request.method == "GET" and self.request.headers.get("Upgrade", "").lower() == "websocket": + return await ensure_async(self.get_websocket(proxied_path)) + # 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 # for more information @@ -351,16 +359,6 @@ async def proxy(self, host, port, proxied_path): if header_to_remove in self.request.headers: del self.request.headers[header_to_remove] - self._record_activity() - - if self.request.headers.get("Upgrade", "").lower() == "websocket": - # We wanna websocket! - # jupyterhub/jupyter-server-proxy@36b3214 - self.log.info( - "we wanna websocket, but we don't define WebSocketProxyHandler" - ) - self.set_status(500) - body = self.request.body if not body: if self.request.method in {"POST", "PUT"}: diff --git a/jupyter_server_proxy/websocket.py b/jupyter_server_proxy/websocket.py index a43b7795..608edefe 100644 --- a/jupyter_server_proxy/websocket.py +++ b/jupyter_server_proxy/websocket.py @@ -96,8 +96,5 @@ def undisallow(*args2, **kwargs2): setattr(self, method, wrapper(method)) nextparent.__init__(self, *args, **kwargs) - async def get(self, *args, **kwargs): - if self.request.headers.get("Upgrade", "").lower() != "websocket": - return await self.http_get(*args, **kwargs) - else: - await ensure_async(super().get(*args, **kwargs)) + async def get_websocket(self, *args, **kwargs): + await ensure_async(super().get(*args, **kwargs)) From 72d6eefaa9254f8dfe96a1abe194b204ba64cea1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 13 Apr 2025 00:05:47 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_server_proxy/handlers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jupyter_server_proxy/handlers.py b/jupyter_server_proxy/handlers.py index 278c2a47..9427b5a0 100644 --- a/jupyter_server_proxy/handlers.py +++ b/jupyter_server_proxy/handlers.py @@ -16,7 +16,7 @@ from jupyter_server.base.handlers import JupyterHandler, utcnow from jupyter_server.utils import ensure_async, url_path_join from simpervisor import SupervisedProcess -from tornado import httpclient, httputil, web, websocket +from tornado import httpclient, httputil, web from tornado.simple_httpclient import SimpleAsyncHTTPClient from traitlets import Bytes, Dict, Instance, Integer, Unicode, Union, default, observe from traitlets.traitlets import HasTraits @@ -171,7 +171,7 @@ async def http_get(self, host, port, proxy_path=""): raise NotImplementedError( "Subclasses of ProxyHandler should implement http_get" ) - + async def get(self, *args, **kwargs): return await self.http_get(*args, **kwargs) @@ -338,7 +338,10 @@ async def proxy(self, host, port, proxied_path): self._record_activity() - if self.request.method == "GET" and self.request.headers.get("Upgrade", "").lower() == "websocket": + if ( + self.request.method == "GET" + and self.request.headers.get("Upgrade", "").lower() == "websocket" + ): return await ensure_async(self.get_websocket(proxied_path)) # Remove hop-by-hop headers that don't necessarily apply to the request we are making From 3347c402492fe35716a552b0244c52acee695842 Mon Sep 17 00:00:00 2001 From: 2xB <31772910+2xB@users.noreply.github.com> Date: Tue, 15 Apr 2025 03:37:15 +0200 Subject: [PATCH 3/6] Fix rawsocket from prev. commit Previously, `proxy` was changed to be used also for websockets and rawsockets so that mappath was also applied. This commit fixes the rawsocket `proxy` method erroring by default, which led to errors in the CI tests together with the previous change. --- jupyter_server_proxy/rawsocket.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jupyter_server_proxy/rawsocket.py b/jupyter_server_proxy/rawsocket.py index 89159168..dcfa0f86 100644 --- a/jupyter_server_proxy/rawsocket.py +++ b/jupyter_server_proxy/rawsocket.py @@ -55,6 +55,12 @@ def _create_ws_connection(self, proto: asyncio.BaseProtocol): return loop.create_connection(proto, "localhost", self.port) async def proxy(self, port, path): + if ( + self.request.method == "GET" + and self.request.headers.get("Upgrade", "").lower() == "websocket" + ): + return await super().proxy(port, path) + raise web.HTTPError( 405, "this raw_socket_proxy backend only supports websocket connections" ) From fe5ab66c929ce5aca2a7bea3b2af062e36139ced Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 15 Apr 2025 01:38:45 +0000 Subject: [PATCH 4/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- jupyter_server_proxy/rawsocket.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyter_server_proxy/rawsocket.py b/jupyter_server_proxy/rawsocket.py index dcfa0f86..bc222691 100644 --- a/jupyter_server_proxy/rawsocket.py +++ b/jupyter_server_proxy/rawsocket.py @@ -60,7 +60,7 @@ async def proxy(self, port, path): and self.request.headers.get("Upgrade", "").lower() == "websocket" ): return await super().proxy(port, path) - + raise web.HTTPError( 405, "this raw_socket_proxy backend only supports websocket connections" ) From ff623df52e30839420fe894dbae2e180be5d14b6 Mon Sep 17 00:00:00 2001 From: 2xB <31772910+2xB@users.noreply.github.com> Date: Tue, 15 Apr 2025 03:42:08 +0200 Subject: [PATCH 5/6] Add test for websocket+mappath --- tests/resources/jupyter_server_config.py | 7 +++++++ tests/test_proxies.py | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/resources/jupyter_server_config.py b/tests/resources/jupyter_server_config.py index ac1e0dfe..ac97d3ff 100644 --- a/tests/resources/jupyter_server_config.py +++ b/tests/resources/jupyter_server_config.py @@ -17,6 +17,9 @@ def mappathf(path): p = path + "mapped" return p +def mappathf_socket(path): + return path + "socket" + def translate_ciao(path, host, response, orig_response, port): # Assume that the body has not been modified by any previous rewrite @@ -83,6 +86,10 @@ def my_env(): "X-Custom-Header": "pytest-23456", }, }, + "python-websocket-mappathf_socket": { + "command": [sys.executable, _get_path("websocket.py"), "--port={port}"], + "mappath": mappathf_socket, + }, "python-eventstream": { "command": [sys.executable, _get_path("eventstream.py"), "--port={port}"] }, diff --git a/tests/test_proxies.py b/tests/test_proxies.py index 4573517c..d7a0430f 100644 --- a/tests/test_proxies.py +++ b/tests/test_proxies.py @@ -401,6 +401,19 @@ async def test_server_proxy_websocket_headers(a_server_port_and_token: Tuple[int assert headers["X-Custom-Header"] == "pytest-23456" +async def test_server_proxy_websocket_messages_mappath( + a_server_port_and_token: Tuple[int, str] +) -> None: + PORT, TOKEN = a_server_port_and_token + # Mappath is configured to add "socket" to websocket paths + url = f"ws://{LOCALHOST}:{PORT}/python-websocket-mappathf_socket/echo?token={TOKEN}" + conn = await websocket_connect(url) + expected_msg = "Hello, world!" + await conn.write_message(expected_msg) + msg = await conn.read_message() + assert msg == expected_msg + + @pytest.mark.parametrize( "client_requested,server_received,server_responded,proxy_responded", [ From f29201a137b77116bd61cf3cab07207e7d5e9141 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 15 Apr 2025 01:42:17 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/resources/jupyter_server_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/resources/jupyter_server_config.py b/tests/resources/jupyter_server_config.py index ac97d3ff..8bbe541d 100644 --- a/tests/resources/jupyter_server_config.py +++ b/tests/resources/jupyter_server_config.py @@ -17,6 +17,7 @@ def mappathf(path): p = path + "mapped" return p + def mappathf_socket(path): return path + "socket"