Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Errors in Firefox and Chrome with 'base64' protocols parameter #63

Closed
wants to merge 1 commit into from

2 participants

@kreneskyp

Receiving an error in both Firefox and Chrome when trying to connect. I tracked it down to line 256 in include/websock.py where WebSocket objects are created. This line was changed in d890e86 adding the 2nd parameter, "base64".

    new WebSocket(uri, 'base64')

Not sure if this is required for other browsers, but reverting that line fixes connections for both Firefox and Chrome.

@kanaka
Owner

@kreneskyp, can you help me tracking this down a bit further. The second parameter is the protocols parameter which indicates to the server which sub-protocols the client supports. I added this to support backwards compatibility in the future when the browsers add native support for binary data.

I have not seen this problem with Chrome or firefox. Are you certain you are running the latest version of the proxy? Are you getting an error from the proxy? Do you get more information from the Chrome debugger of firebug about what error is happening in the browser?

Thanks.

@kreneskyp

Firefox 4.01
Chromium 11.0.696.71 (86024) Ubuntu 11.04

I tested with noVNC 3859e1d. It works fine when this patch is applied.

We were using our own proxy as it supports additional features we need. I can get you the errors but it was not very helpful to us.

@kanaka
Owner

Ah, okay, if you are using your own proxy then it probably needs to be updated to properly handle the sub-protocol selection. The protocols parameter gets passed as the protocols header in the handshake. The server (proxy) needs to pick one of the sub-protocols specified by the client and return it back. This was a deficiency in websockify that I fixed (before that it was just sending back 'sample' as the protocol). If you implemented the incomplete handshake method from an older version of websockify (or wsproxy before it was split off) then you need to update to properly handle sub-protocol selection.

In the future, the client will send two protocol values ('binary' and 'base64') if the browser supports binary data in the WebSockets API, rather than the single 'base64' that it is currently sending. If the server picks 'binary' then the client will send the data without base64 encoding, otherwise if the server picks 'base64' then the client will continue to send base64 encoded strings as it currently does.

I'm closing this request. If you still think there is something that needs to be addressed in the noVNC code please re-open then bug.

@kanaka kanaka closed this
@kreneskyp

After talking with our developer working on this it's clear that it was indeed our proxy. We were only supporting 75/76 which did not have support for subprotocols. Everything works now with a fix on our end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  include/websock.js
View
2  include/websock.js
@@ -253,7 +253,7 @@ function init() {
function open(uri) {
init();
- websocket = new WebSocket(uri, 'base64');
+ websocket = new WebSocket(uri);
// TODO: future native binary support
//websocket = new WebSocket(uri, ['binary', 'base64']);
Something went wrong with that request. Please try again.