WebSocket subprotocols for client/proxy are chosen without asking the server we proxy to #459
Labels
Comments
|
I think I'm observing this with 4.1.1 and jupyterlab-nvdashboard, which uses bokeh: This causes |
|
And it worked before in 4.1.0? |
|
Yes it was working before w/4.1.0 |
|
@rcthomas can you help me reproduce this? So far I've not seen the warning in jupyter server logs, or in javascript console logs, and the dashboard works for me. I've tried this locally and with a locally run jupyterhub so far. Where was the |
|
Thanks @consideRatio! Relieved to see there's a configuration out there that's known to work. I observed the message in the Jupyter server logs. I've turned on debug-level logging. Here's the log output when I try to open the machine resources tab. Is the "client sent protocols" line what you would expect? log snippetThe warnings about "legend" apparently come from us not yet upgrading the nvdashboard to 0.9, but for now I am leaving 0.8 in place. Looking at the diff there I don't think that would fix what I'm seeing. I can revert and add the log details for that if it helps. Here is a summary of the environment in case that helps trace down any differences in our environments: environment |
|
Thank you @rcthomas! I could reproduce this now and conclude that the surgical change between 4.1.1 and 4.1.0 makes this difference as well. I'll work this with highest prio since we need a release out with things in 4.1.1, but if it doesn't work thats trouble. |
|
Okay I've figured out the issue, but I'm out of time to work this further today, will continue tomorrow or if I find free time. |
|
Wow thanks! But if you find free time, use it for yourself! |
consideRatio
added a commit
to consideRatio/jupyter-server-proxy
that referenced
this issue
Mar 13, 2024
When jupyter-server-proxy proxies websockets, its finalizes the websocket handshake between client/proxy before it initiates the proxy/server websocket handshake. In 4.1.1 the thinking was that it was a better compromise to not forward all subprotocol choices in the proxy/server handshake if we had prematurely picked a single choice in the client/proxy handshake. This turns out to have introduced a regression though, as at least bokeh had been using secondary subprotocol choices to pass other information such as base64 encoded JSON with keys like `session_id`, `session_expiry` and `__bk__zlib_`. This commit makes sure we keep passing all requested subprotocols, even though we have prematurely picked a specific ahead of time - which is a bug tracked in jupyterhub#459.
consideRatio
added a commit
to consideRatio/jupyter-server-proxy
that referenced
this issue
Mar 13, 2024
When jupyter-server-proxy proxies websockets, its finalizes the websocket handshake between client/proxy before it initiates the proxy/server websocket handshake. In 4.1.1 the thinking was that it was a better compromise to not forward all subprotocol choices in the proxy/server handshake if we had prematurely picked a single choice in the client/proxy handshake. This turns out to have introduced a regression though, as at least bokeh had been using secondary subprotocol choices to pass other information such as base64 encoded JSON with keys like `session_id`, `session_expiry` and `__bk__zlib_`. This commit makes sure we keep passing all requested subprotocols, even though we have prematurely picked a specific ahead of time - which is a bug tracked in jupyterhub#459.
consideRatio
added a commit
to consideRatio/jupyter-server-proxy
that referenced
this issue
Mar 13, 2024
When jupyter-server-proxy proxies websockets, its finalizes the websocket handshake between client/proxy before it initiates the proxy/server websocket handshake. In 4.1.1 the thinking was that it was a better compromise to not forward all subprotocol choices in the proxy/server handshake if we had prematurely picked a single choice in the client/proxy handshake. This turns out to have introduced a regression though, as at least bokeh had been using secondary subprotocol choices to pass other information such as base64 encoded JSON with keys like `session_id`, `session_expiry` and `__bk__zlib_`. This commit makes sure we keep passing all requested subprotocols, even though we have prematurely picked a specific ahead of time - which is a bug tracked in jupyterhub#459.
|
@consideRatio #462 appears to have fixed it! Thanks! |
|
Thank you for the quick testing and reporting @rcthomas! |
This was referenced Mar 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Looking at #442 led me to conclude that when a websocket connection is being proxied, we establish the client/proxy socket fully before establishing a proxy/server connection. This is a problem, because when a websocket is established, a client requests subprotocols it knows how to speak, and the server is to choose one it supports. The choice of subprotocol influences the format of the sent messages, so its critical that the server we proxy to gets to decide this. Currently though, we just pick the first subprotocol instead and hope that the server we proxy to will later accept it.
In #458 I added test cases to confirm our bugged behavior, and test cases we could switch to if we resolve this by not finalizing the client/proxy handshake before we establish a server/proxy handshake.
This bug is probably not very problematic, and with #458, I'm looking to provide a warning in case it could be a problem. The problem would arise mostly if the client doesn't know how to correctly interact with the server, so I think this can be left unresolved until there is a clear example of when this becomes relevant. The complexity to solve this bug could be notable.
Related
The text was updated successfully, but these errors were encountered: