Skip to content

Fix websocket requests not mapped via mappath #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix websocket requests not mapped via mappath #526

wants to merge 6 commits into from

Conversation

2xB
Copy link

@2xB 2xB commented Apr 13, 2025

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

2xB and others added 2 commits April 13, 2025 02:03
@2xB
Fix websocket requests not mapped via mappath
b75474e 
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 jupyterhub#525
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
72d6eef 
for more information, see https://pre-commit.ci
@2xB
Copy link
Author

2xB commented Apr 13, 2025 •
edited
Loading

I believe tests may fail thanks to the custom NewHandler implemented for the tests in https://github.com/jupyterhub/jupyter-server-proxy/blame/main/tests/resources/proxyextension.py that for some reason always calls ProxyHandler.proxy with proxied_path = "" as written in https://github.com/jupyterhub/jupyter-server-proxy/blame/8c5906a68f90252951342ccd626e0332f8531872/tests/resources/proxyextension.py#L36 ...

2xB and others added 4 commits April 15, 2025 03:37
@2xB
Fix rawsocket from prev. commit
3347c40 
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.
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
fe5ab66 
for more information, see https://pre-commit.ci
@2xB
Add test for websocket+mappath
ff623df
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
f29201a 
for more information, see https://pre-commit.ci
@2xB
Copy link
Author

2xB commented Apr 15, 2025

Fixed the rawsocket handler. Its proxy method previously errored always, but my new code relies on the proxy method to delegate to the new get_websocket method.

The belief I wrote in the above comment therefore was wrong, the CI failed due to an actual issue.

I also added a test for websocket+mappath, testing for the behavior introduced by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Websocket requests are not mapped via mappath
1 participant
@2xB