Skip to content
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.

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

Ensure no blank Sec-Websocket-Protocol headers and warn if websocket subprotocol edge case occur #458

Merged
merged 5 commits into from
Feb 23, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Warn if proxied websockets' selected subprotocols mismatch
  • Loading branch information
consideRatio committed Feb 23, 2024
commit eda6136f46ec88247097887abe6395cda304f867
14 changes: 12 additions & 2 deletions jupyter_server_proxy/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,16 @@ async def start_websocket_connection():
)
self._record_activity()
self.log.info(f"Websocket connection established to {client_uri}")
if (
subprotocols
and self.ws.selected_subprotocol != self.selected_subprotocol
):
self.log.warn(
f"Websocket subprotocol between proxy/server ({self.ws.selected_subprotocol}) "
f"became different than for client/proxy ({self.selected_subprotocol}) "
"due to https://github.com/jupyterhub/jupyter-server-proxy/issues/459. "
f"Requested subprotocols were {subprotocols}."
)

# Wait for the WebSocket to be connected before resolving.
# Otherwise, messages sent by the client before the
Expand Down Expand Up @@ -539,7 +549,8 @@ def select_subprotocol(self, subprotocols):
Note that this subprotocol selection should really be delegated to the
server we proxy to, but we don't! For this to happen, we would need to
delay accepting the handshake with the client until we have successfully
handshaked with the server.
handshaked with the server. This issue is tracked via
https://github.com/jupyterhub/jupyter-server-proxy/issues/459.

Overrides `tornado.websocket.WebSocketHandler.select_subprotocol` that
includes an informative docstring:
Expand All @@ -549,7 +560,6 @@ def select_subprotocol(self, subprotocols):
self.log.debug(
f"Client sent subprotocols: {subprotocols}, selecting the first"
)
# TODO: warn if we select one out of multiple!
return subprotocols[0]
return None

Expand Down