From b2cf3b68b35050567f02fc5e21a12c41023a361b Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Fri, 4 Nov 2016 13:00:14 +0100 Subject: [PATCH 1/9] requestDesktopSize() should always return a value --- core/rfb.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/rfb.js b/core/rfb.js index 421bf540..e3238241 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -343,7 +343,7 @@ requestDesktopSize: function (width, height) { if (this._rfb_connection_state !== 'connected' || this._view_only) { - return; + return false; } if (this._supportsSetDesktopSize) { From d24de750b1a382c489445f6a56bcd632ed02b4f5 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Sat, 5 Nov 2016 18:02:08 +0100 Subject: [PATCH 2/9] Add fallback handling for uncought errors --- app/styles/base.css | 37 +++++++++++++++++++++++++++++++++++++ app/ui.js | 15 +++++++++++++++ vnc.html | 6 ++++++ 3 files changed, 58 insertions(+) diff --git a/app/styles/base.css b/app/styles/base.css index 03f6583e..b90bcb26 100644 --- a/app/styles/base.css +++ b/app/styles/base.css @@ -172,6 +172,43 @@ input[type=button]:active, select:active { pointer-events: auto; } +/* ---------------------------------------- + * Fallback error + * ---------------------------------------- + */ + +#noVNC_fallback_error { + position: fixed; + z-index: 3; + left: 50%; + transform: translate(-50%, -50px); + transition: 0.5s ease-in-out; + + visibility: hidden; + opacity: 0; + + top: 60px; + padding: 15px; + width: auto; + + text-align: center; + font-weight: bold; + word-wrap: break-word; + color: #fff; + + border-radius: 10px; + box-shadow: 6px 6px 0px rgba(0, 0, 0, 0.5); + background: rgba(200,55,55,0.8); +} +#noVNC_fallback_error.noVNC_open { + transform: translate(-50%, 0); + visibility: visible; + opacity: 1; +} +#noVNC_fallback_errormsg { + font-weight: normal; +} + /* ---------------------------------------- * Control Bar * ---------------------------------------- diff --git a/app/ui.js b/app/ui.js index 1e55652b..ceb72092 100644 --- a/app/ui.js +++ b/app/ui.js @@ -25,6 +25,21 @@ var UI; (function () { "use strict"; + // Fallback for all uncought errors + window.addEventListener('error', function(msg, url, line) { + try { + document.getElementById('noVNC_fallback_error') + .classList.add("noVNC_open"); + document.getElementById('noVNC_fallback_errormsg').innerHTML = + url + ' (' + line + ')

' + msg; + } catch (exc) { + document.write("noVNC encountered an error."); + } + // Don't return true since this would prevent the error + // from being printed to the browser console. + return false; + }); + /* [begin skip-as-module] */ // Load supporting scripts WebUtil.load_scripts( diff --git a/vnc.html b/vnc.html index 01828b44..ef880d20 100644 --- a/vnc.html +++ b/vnc.html @@ -65,6 +65,12 @@ + +
+
noVNC encountered an error:
+
+
+
From 376864d6d15817acc8c3f9a647da58638cf2b499 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Sat, 5 Nov 2016 18:18:15 +0100 Subject: [PATCH 3/9] Handle errors while opening a Websocket For example, previously if the user typed in illegal characters in the port field, no error would be displayed in the interface and the page would stop at "connecting". --- core/rfb.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/rfb.js b/core/rfb.js index e3238241..5b1579a8 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -372,7 +372,15 @@ uri += '://' + this._rfb_host + ':' + this._rfb_port + '/' + this._rfb_path; Util.Info("connecting to " + uri); - this._sock.open(uri, this._wsProtocols); + try { + this._sock.open(uri, this._wsProtocols); + } catch (e) { + if (e.name === 'SyntaxError') { + this._fail("Invalid host or port value given"); + } else { + this._fail("Error while opening websocket (" + e + ")"); + } + } Util.Debug("<< RFB.connect"); }, From c4b274ebb8cf7d6870db96cd9efdd6ebeee5b02a Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Tue, 8 Nov 2016 16:07:47 +0100 Subject: [PATCH 4/9] Move disconnect actions to a separate funciton Done in order to be consistent with connect() and to separate state actions from connection actions. --- core/rfb.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 5b1579a8..e253f0b2 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -385,6 +385,14 @@ Util.Debug("<< RFB.connect"); }, + _disconnect: function () { + Util.Debug(">> RFB.disconnect"); + this._cleanup(); + this._sock.close(); + this._print_stats(); + Util.Debug("<< RFB.disconnect"); + }, + _init_vars: function () { // reset state this._FBU.rects = 0; @@ -503,15 +511,12 @@ break; case 'disconnecting': - this._cleanup(); - this._sock.close(); // transitions to 'disconnected' + this._disconnect(); this._disconnTimer = setTimeout(function () { this._rfb_disconnect_reason = "Disconnect timeout"; this._updateConnectionState('disconnected'); }.bind(this), this._disconnectTimeout * 1000); - - this._print_stats(); break; default: From e48d1b254ea14a3679fc40e241cf80176343fca1 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Tue, 8 Nov 2016 16:27:31 +0100 Subject: [PATCH 5/9] Separate state actions from connection actions --- core/rfb.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index e253f0b2..d83b691d 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -361,6 +361,7 @@ _connect: function () { Util.Debug(">> RFB.connect"); + this._init_vars(); var uri; if (typeof UsingSocketIO !== 'undefined') { @@ -373,6 +374,7 @@ Util.Info("connecting to " + uri); try { + // WebSocket.onopen transitions to the RFB init states this._sock.open(uri, this._wsProtocols); } catch (e) { if (e.name === 'SyntaxError') { @@ -504,9 +506,6 @@ break; case 'connecting': - this._init_vars(); - - // WebSocket.onopen transitions to the RFB init states this._connect(); break; From b45905ab863903ba0d1894fca9b4110a1a7d1004 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Tue, 8 Nov 2016 16:33:01 +0100 Subject: [PATCH 6/9] Handle server-side disconnections Don't handle socket-close events when connected as errors. You could for example, in the VNC session run 'vncconfig -disconnect'. --- core/rfb.js | 7 ++++++- tests/test.rfb.js | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index d83b691d..9d968bbe 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -233,11 +233,16 @@ case 'connecting': this._fail('Failed to connect to server' + msg); break; + case 'connected': + // Handle disconnects that were initiated server-side + this._updateConnectionState('disconnecting'); + this._updateConnectionState('disconnected'); + break; case 'disconnected': Util.Error("Received onclose while disconnected" + msg); break; default: - this._fail("Server disconnected" + msg); + this._fail("Unexpected server disconnect" + msg); break; } this._sock.off('close'); diff --git a/tests/test.rfb.js b/tests/test.rfb.js index b8bc9d22..828456af 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -2112,10 +2112,10 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(client._rfb_connection_state).to.equal('disconnected'); }); - it('should transition to failed if we get a close event from any non-"disconnection" state', function () { + it('should fail if we get a close event while connecting', function () { sinon.spy(client, "_fail"); client.connect('host', 8675); - client._rfb_connection_state = 'connected'; + client._rfb_connection_state = 'connecting'; client._sock._websocket.close(); expect(client._fail).to.have.been.calledOnce; }); From b2e961d48d6c4c05f695437ebf410ca56c46b8fe Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Tue, 8 Nov 2016 16:56:35 +0100 Subject: [PATCH 7/9] Ensure proper connection state transitions Makes the state machine more rubust and clear. --- core/rfb.js | 63 +++++++++++++++++++++++++++++++++-------------- tests/test.rfb.js | 40 ++++++++++++++++++++++-------- 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 9d968bbe..91d9a6ff 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -239,7 +239,7 @@ this._updateConnectionState('disconnected'); break; case 'disconnected': - Util.Error("Received onclose while disconnected" + msg); + this._fail("Received onclose while disconnected" + msg); break; default: this._fail("Unexpected server disconnect" + msg); @@ -473,19 +473,7 @@ return; } - this._rfb_connection_state = state; - - var smsg = "New state '" + state + "', was '" + oldstate + "'."; - Util.Debug(smsg); - - if (this._disconnTimer && state !== 'disconnecting') { - Util.Debug("Clearing disconnect timer"); - clearTimeout(this._disconnTimer); - this._disconnTimer = null; - this._sock.off('close'); // make sure we don't get a double event - } - - this._onUpdateState(this, state, oldstate); + // Ensure proper transitions before doing anything switch (state) { case 'connected': if (oldstate !== 'connecting') { @@ -501,7 +489,50 @@ "previous connection state: " + oldstate); return; } + break; + case 'connecting': + if (oldstate !== '') { + Util.Error("Bad transition to connecting state, " + + "previous connection state: " + oldstate); + return; + } + break; + + case 'disconnecting': + if (oldstate !== 'connected' && oldstate !== 'connecting') { + Util.Error("Bad transition to disconnecting state, " + + "previous connection state: " + oldstate); + return; + } + break; + + default: + Util.Error("Unknown connection state: " + state); + return; + } + + // State change actions + + this._rfb_connection_state = state; + this._onUpdateState(this, state, oldstate); + + var smsg = "New state '" + state + "', was '" + oldstate + "'."; + Util.Debug(smsg); + + if (this._disconnTimer && state !== 'disconnecting') { + Util.Debug("Clearing disconnect timer"); + clearTimeout(this._disconnTimer); + this._disconnTimer = null; + + // make sure we don't get a double event + this._sock.off('close'); + } + + switch (state) { + case 'disconnected': + // Call onDisconnected callback after onUpdateState since + // we don't know if the UI only displays the latest message if (this._rfb_disconnect_reason !== "") { this._onDisconnected(this, this._rfb_disconnect_reason); } else { @@ -522,10 +553,6 @@ this._updateConnectionState('disconnected'); }.bind(this), this._disconnectTimeout * 1000); break; - - default: - Util.Error("Unknown connection state: " + state); - return; } }, diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 828456af..60bbe9a6 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -330,7 +330,7 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should clear the disconnect timer if the state is not "disconnecting"', function () { var spy = sinon.spy(); client._disconnTimer = setTimeout(spy, 50); - client._updateConnectionState('connected'); + client._updateConnectionState('connecting'); this.clock.tick(51); expect(spy).to.not.have.been.called; expect(client._disconnTimer).to.be.null; @@ -338,27 +338,37 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should call the updateState callback', function () { client.set_onUpdateState(sinon.spy()); - client._updateConnectionState('a specific state'); + client._updateConnectionState('connecting'); var spy = client.get_onUpdateState(); expect(spy).to.have.been.calledOnce; - expect(spy.args[0][1]).to.equal('a specific state'); + expect(spy.args[0][1]).to.equal('connecting'); }); it('should set the rfb_connection_state', function () { - client._updateConnectionState('a specific state'); - expect(client._rfb_connection_state).to.equal('a specific state'); + client._rfb_connection_state = 'disconnecting'; + client._updateConnectionState('disconnected'); + expect(client._rfb_connection_state).to.equal('disconnected'); }); it('should not change the state when we are disconnected', function () { client._rfb_connection_state = 'disconnected'; - client._updateConnectionState('a specific state'); - expect(client._rfb_connection_state).to.not.equal('a specific state'); + client._updateConnectionState('connecting'); + expect(client._rfb_connection_state).to.not.equal('connecting'); }); it('should ignore state changes to the same state', function () { client.set_onUpdateState(sinon.spy()); - client._rfb_connection_state = 'a specific state'; - client._updateConnectionState('a specific state'); + client._rfb_connection_state = 'connecting'; + client._updateConnectionState('connecting'); + var spy = client.get_onUpdateState(); + expect(spy).to.not.have.been.called; + }); + + it('should ignore illegal state changes', function () { + client.set_onUpdateState(sinon.spy()); + client._rfb_connection_state = 'connected'; + client._updateConnectionState('disconnected'); + expect(client._rfb_connection_state).to.not.equal('disconnected'); var spy = client.get_onUpdateState(); expect(spy).to.not.have.been.called; }); @@ -391,11 +401,13 @@ describe('Remote Frame Buffer Protocol Client', function() { }); it('should set disconnect_reason', function () { + client._rfb_connection_state = 'connected'; client._fail('a reason'); expect(client._rfb_disconnect_reason).to.equal('a reason'); }); it('should result in disconnect callback with message when reason given', function () { + client._rfb_connection_state = 'connected'; client.set_onDisconnected(sinon.spy()); client._fail('a reason'); var spy = client.get_onDisconnected(); @@ -542,7 +554,7 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should call the updateState callback before the disconnect callback', function () { client.set_onDisconnected(sinon.spy()); client.set_onUpdateState(sinon.spy()); - client._rfb_connection_state = 'other state'; + client._rfb_connection_state = 'disconnecting'; client._updateConnectionState('disconnected'); var updateStateSpy = client.get_onUpdateState(); var disconnectSpy = client.get_onDisconnected(); @@ -2120,6 +2132,14 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(client._fail).to.have.been.calledOnce; }); + it('should fail if we get a close event while disconnected', function () { + sinon.spy(client, "_fail"); + client.connect('host', 8675); + client._rfb_connection_state = 'disconnected'; + client._sock._websocket.close(); + expect(client._fail).to.have.been.calledOnce; + }); + it('should unregister close event handler', function () { sinon.spy(client._sock, 'off'); client.connect('host', 8675); From 9f909d9b460675cc80624f5aeb018f46c3f41fae Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Tue, 8 Nov 2016 18:18:19 +0100 Subject: [PATCH 8/9] Don't wait for websocket closes on failure RFB._fail() can be called at any time in any state, it is not certain that we will get a close event on the socket since the socket might not be open. This caused us to hit the disconnect timeout in such cases. Fixes issue #678 --- core/rfb.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/rfb.js b/core/rfb.js index 91d9a6ff..8e9c5700 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -572,7 +572,11 @@ break; } this._rfb_disconnect_reason = msg; + + // Transition to disconnected without waiting for socket to close this._updateConnectionState('disconnecting'); + this._updateConnectionState('disconnected'); + return false; }, From 67cd2072ab81449c4285a15c5ea3df38ad8c7232 Mon Sep 17 00:00:00 2001 From: Samuel Mannehed Date: Wed, 9 Nov 2016 15:54:10 +0100 Subject: [PATCH 9/9] Allow specifying details when calling RFB._fail() RFB's _fail function logs the error, disconnects the session and sets disconnect_reason. The disconnect_reason is upon disconnection sent to the user interface. It is thus not suitable for including error details that aren't user friendly. The idea is that you will look in the browser console for a full log with details of the error. --- core/rfb.js | 106 +++++++++++++++++++++++++++++++--------------- tests/test.rfb.js | 15 +++++-- 2 files changed, 83 insertions(+), 38 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 8e9c5700..d10d6662 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -213,7 +213,7 @@ this._rfb_init_state = 'ProtocolVersion'; Util.Debug("Starting VNC handshake"); } else { - this._fail("Got unexpected WebSocket connection"); + this._fail("Unexpected server connection"); } }.bind(this)); this._sock.on('close', function (e) { @@ -231,7 +231,7 @@ this._updateConnectionState('disconnected'); break; case 'connecting': - this._fail('Failed to connect to server' + msg); + this._fail('Failed to connect to server', msg); break; case 'connected': // Handle disconnects that were initiated server-side @@ -239,10 +239,12 @@ this._updateConnectionState('disconnected'); break; case 'disconnected': - this._fail("Received onclose while disconnected" + msg); + this._fail("Unexpected server disconnect", + "Already disconnected: " + msg); break; default: - this._fail("Unexpected server disconnect" + msg); + this._fail("Unexpected server disconnect", + "Not in any state yet: " + msg); break; } this._sock.off('close'); @@ -383,9 +385,9 @@ this._sock.open(uri, this._wsProtocols); } catch (e) { if (e.name === 'SyntaxError') { - this._fail("Invalid host or port value given"); + this._fail("Invalid host or port value given", e); } else { - this._fail("Error while opening websocket (" + e + ")"); + this._fail("Error while connecting", e); } } @@ -556,22 +558,31 @@ } }, - _fail: function (msg) { + /* Print errors and disconnect + * + * The optional parameter 'details' is used for information that + * should be logged but not sent to the user interface. + */ + _fail: function (msg, details) { + var fullmsg = msg; + if (typeof details !== 'undefined') { + fullmsg = msg + "(" + details + ")"; + } switch (this._rfb_connection_state) { case 'disconnecting': - Util.Error("Error while disconnecting: " + msg); + Util.Error("Failed when disconnecting: " + fullmsg); break; case 'connected': - Util.Error("Error while connected: " + msg); + Util.Error("Failed while connected: " + fullmsg); break; case 'connecting': - Util.Error("Error while connecting: " + msg); + Util.Error("Failed when connecting: " + fullmsg); break; default: - Util.Error("RFB error: " + msg); + Util.Error("RFB failure: " + fullmsg); break; } - this._rfb_disconnect_reason = msg; + this._rfb_disconnect_reason = msg; //This is sent to the UI // Transition to disconnected without waiting for socket to close this._updateConnectionState('disconnecting'); @@ -717,7 +728,8 @@ _negotiate_protocol_version: function () { if (this._sock.rQlen() < 12) { - return this._fail("Incomplete protocol version"); + return this._fail("Error while negotiating with server", + "Incomplete protocol version"); } var sversion = this._sock.rQshiftStr(12).substr(4, 7); @@ -742,7 +754,8 @@ this._rfb_version = 3.8; break; default: - return this._fail("Invalid server version " + sversion); + return this._fail("Unsupported server", + "Invalid server version: " + sversion); } if (is_repeater) { @@ -775,7 +788,8 @@ if (num_types === 0) { var strlen = this._sock.rQshift32(); var reason = this._sock.rQshiftStr(strlen); - return this._fail("Security failure: " + reason); + return this._fail("Error while negotiating with server", + "Security failure: " + reason); } this._rfb_auth_scheme = 0; @@ -797,7 +811,8 @@ } if (this._rfb_auth_scheme === 0) { - return this._fail("Unsupported security types: " + types); + return this._fail("Unsupported server", + "Unsupported security types: " + types); } this._sock.send([this._rfb_auth_scheme]); @@ -867,12 +882,16 @@ if (serverSupportedTunnelTypes[0]) { if (serverSupportedTunnelTypes[0].vendor != clientSupportedTunnelTypes[0].vendor || serverSupportedTunnelTypes[0].signature != clientSupportedTunnelTypes[0].signature) { - return this._fail("Client's tunnel type had the incorrect vendor or signature"); + return this._fail("Unsupported server", + "Client's tunnel type had the incorrect " + + "vendor or signature"); } this._sock.send([0, 0, 0, 0]); // use NOTUNNEL return false; // wait until we receive the sub auth count to continue } else { - return this._fail("Server wanted tunnels, but doesn't support the notunnel type"); + return this._fail("Unsupported server", + "Server wanted tunnels, but doesn't support " + + "the notunnel type"); } }, @@ -925,12 +944,15 @@ this._rfb_auth_scheme = 2; return this._init_msg(); default: - return this._fail("Unsupported tiny auth scheme: " + authType); + return this._fail("Unsupported server", + "Unsupported tiny auth scheme: " + + authType); } } } - return this._fail("No supported sub-auth types!"); + return this._fail("Unsupported server", + "No supported sub-auth types!"); }, _negotiate_authentication: function () { @@ -939,7 +961,7 @@ if (this._sock.rQwait("auth reason", 4)) { return false; } var strlen = this._sock.rQshift32(); var reason = this._sock.rQshiftStr(strlen); - return this._fail("Auth failure: " + reason); + return this._fail("Authentication failure", reason); case 1: // no auth if (this._rfb_version >= 3.8) { @@ -959,7 +981,9 @@ return this._negotiate_tight_auth(); default: - return this._fail("Unsupported auth scheme: " + this._rfb_auth_scheme); + return this._fail("Unsupported server", + "Unsupported auth scheme: " + + this._rfb_auth_scheme); } }, @@ -975,15 +999,16 @@ var length = this._sock.rQshift32(); if (this._sock.rQwait("SecurityResult reason", length, 8)) { return false; } var reason = this._sock.rQshiftStr(length); - return this._fail(reason); + return this._fail("Authentication failure", reason); } else { return this._fail("Authentication failure"); } return false; case 2: - return this._fail("Too many auth attempts"); + return this._fail("Too many authentication attempts"); default: - return this._fail("Unknown SecurityResult"); + return this._fail("Unsupported server", + "Unknown SecurityResult"); } }, @@ -1131,7 +1156,7 @@ return this._negotiate_server_init(); default: - return this._fail("Unknown init state: " + + return this._fail("Internal error", "Unknown init state: " + this._rfb_init_state); } }, @@ -1196,7 +1221,8 @@ */ if (!(flags & (1<<31))) { - return this._fail("Unexpected fence response"); + return this._fail("Internal error", + "Unexpected fence response"); } // Filter out unsupported flags @@ -1228,7 +1254,8 @@ this._onXvpInit(this._rfb_xvp_ver); break; default: - this._fail("Disconnected: illegal server XVP message " + xvp_msg); + this._fail("Unexpected server message", + "Illegal server XVP message " + xvp_msg); break; } @@ -1287,7 +1314,7 @@ return this._handle_xvp_msg(); default: - this._fail("Disconnected: illegal server message type " + msg_type); + this._fail("Unexpected server message", "Type:" + msg_type); Util.Debug("sock.rQslice(0, 30): " + this._sock.rQslice(0, 30)); return true; } @@ -1348,7 +1375,8 @@ 'encodingName': this._encNames[this._FBU.encoding]}); if (!this._encNames[this._FBU.encoding]) { - this._fail("Disconnected: unsupported encoding " + + this._fail("Unexpected server message", + "Unsupported encoding " + this._FBU.encoding); return false; } @@ -1855,7 +1883,8 @@ if (this._sock.rQwait("HEXTILE subencoding", this._FBU.bytes)) { return false; } var subencoding = rQ[rQi]; // Peek if (subencoding > 30) { // Raw - this._fail("Disconnected: illegal hextile subencoding " + subencoding); + this._fail("Unexpected server message", + "Illegal hextile subencoding: " + subencoding); return false; } @@ -1987,7 +2016,9 @@ display_tight: function (isTightPNG) { if (this._fb_depth === 1) { - this._fail("Tight protocol handler only implements true color mode"); + this._fail("Internal error", + "Tight protocol handler only implements " + + "true color mode"); } this._FBU.bytes = 1; // compression-control byte @@ -2217,10 +2248,13 @@ else if (ctl === 0x0A) cmode = "png"; else if (ctl & 0x04) cmode = "filter"; else if (ctl < 0x04) cmode = "copy"; - else return this._fail("Illegal tight compression received, ctl: " + ctl); + else return this._fail("Unexpected server message", + "Illegal tight compression received, " + + "ctl: " + ctl); if (isTightPNG && (cmode === "filter" || cmode === "copy")) { - return this._fail("filter/copy received in tightPNG mode"); + return this._fail("Unexpected server message", + "filter/copy received in tightPNG mode"); } switch (cmode) { @@ -2281,7 +2315,9 @@ } else { // Filter 0, Copy could be valid here, but servers don't send it as an explicit filter // Filter 2, Gradient is valid but not use if jpeg is enabled - this._fail("Unsupported tight subencoding received, filter: " + filterId); + this._fail("Unexpected server message", + "Unsupported tight subencoding received, " + + "filter: " + filterId); } break; case "copy": diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 60bbe9a6..ae51bffc 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -406,6 +406,12 @@ describe('Remote Frame Buffer Protocol Client', function() { expect(client._rfb_disconnect_reason).to.equal('a reason'); }); + it('should not include details in disconnect_reason', function () { + client._rfb_connection_state = 'connected'; + client._fail('a reason', 'details'); + expect(client._rfb_disconnect_reason).to.equal('a reason'); + }); + it('should result in disconnect callback with message when reason given', function () { client._rfb_connection_state = 'connected'; client.set_onDisconnected(sinon.spy()); @@ -729,7 +735,8 @@ describe('Remote Frame Buffer Protocol Client', function() { client._sock._websocket._receive_data(failure_data); expect(client._fail).to.have.been.calledOnce; - expect(client._fail).to.have.been.calledWith('Security failure: whoops'); + expect(client._fail).to.have.been.calledWith( + 'Error while negotiating with server','Security failure: whoops'); }); it('should transition to the Authentication state and continue on successful negotiation', function () { @@ -768,7 +775,8 @@ describe('Remote Frame Buffer Protocol Client', function() { sinon.spy(client, '_fail'); client._sock._websocket._receive_data(new Uint8Array(data)); - expect(client._fail).to.have.been.calledWith('Auth failure: Whoopsies'); + expect(client._fail).to.have.been.calledWith( + 'Authentication failure', 'Whoopsies'); }); it('should transition straight to SecurityResult on "no auth" (1) for versions >= 3.8', function () { @@ -1000,7 +1008,8 @@ describe('Remote Frame Buffer Protocol Client', function() { sinon.spy(client, '_fail'); var failure_data = [0, 0, 0, 1, 0, 0, 0, 6, 119, 104, 111, 111, 112, 115]; client._sock._websocket._receive_data(new Uint8Array(failure_data)); - expect(client._fail).to.have.been.calledWith('whoops'); + expect(client._fail).to.have.been.calledWith( + 'Authentication failure', 'whoops'); }); it('should fail on an error code of 1 with a standard message for version < 3.8', function () {