From 2f4516f293d4c64394f28ca62640cad5bb773656 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 20 Oct 2017 16:46:36 +0200 Subject: [PATCH] Integrate connect() in to constructor An RFB object represents a single connection so it doesn't make sense to have one without it trying to connect right away. Matches the behaviour of other APIs, e.g. WebSocket. --- app/ui.js | 39 +++++-------- core/rfb.js | 38 ++++++------ docs/API.md | 88 ++++++++++++---------------- tests/playback.js | 11 +--- tests/test.rfb.js | 143 ++++++++++++++++++---------------------------- vnc_lite.html | 28 ++++----- 6 files changed, 134 insertions(+), 213 deletions(-) diff --git a/app/ui.js b/app/ui.js index 6d910dd4..52fcf161 100644 --- a/app/ui.js +++ b/app/ui.js @@ -199,27 +199,6 @@ var UI = { } }, - initRFB: function() { - try { - UI.rfb = new RFB(document.getElementById('noVNC_canvas')); - UI.rfb.onnotification = UI.notification; - UI.rfb.onupdatestate = UI.updateState; - UI.rfb.ondisconnected = UI.disconnectFinished; - UI.rfb.oncredentialsrequired = UI.credentials; - UI.rfb.oncapabilities = function () { UI.updatePowerButton(); UI.initialResize(); }; - UI.rfb.onclipboard = UI.clipboardReceive; - UI.rfb.onbell = UI.bell; - UI.rfb.onfbresize = UI.updateSessionSize; - UI.rfb.ondesktopname = UI.updateDesktopName; - return true; - } catch (exc) { - var msg = "Unable to create RFB client -- " + exc; - Log.Error(msg); - UI.showStatus(msg, 'error'); - return false; - } - }, - /* ------^------- * /INIT * ============== @@ -1042,8 +1021,6 @@ var UI = { return; } - if (!UI.initRFB()) return; - UI.closeAllPanels(); UI.closeConnectPanel(); @@ -1059,9 +1036,19 @@ var UI = { } url += '/' + path; - UI.rfb.connect(url, { shared: UI.getSetting('shared'), - repeaterID: UI.getSetting('repeaterID'), - credentials: { password: password } }); + UI.rfb = new RFB(document.getElementById('noVNC_canvas'), url, + { shared: UI.getSetting('shared'), + repeaterID: UI.getSetting('repeaterID'), + credentials: { password: password } }); + UI.rfb.onnotification = UI.notification; + UI.rfb.onupdatestate = UI.updateState; + UI.rfb.ondisconnected = UI.disconnectFinished; + UI.rfb.oncredentialsrequired = UI.credentials; + UI.rfb.oncapabilities = function () { UI.updatePowerButton(); UI.initialResize(); }; + UI.rfb.onclipboard = UI.clipboardReceive; + UI.rfb.onbell = UI.bell; + UI.rfb.onfbresize = UI.updateSessionSize; + UI.rfb.ondesktopname = UI.updateDesktopName; }, disconnect: function() { diff --git a/core/rfb.js b/core/rfb.js index 39e02dd2..28e1a4a4 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -31,12 +31,22 @@ import { encodings, encodingName } from "./encodings.js"; // How many seconds to wait for a disconnect to finish var DISCONNECT_TIMEOUT = 3; -export default function RFB(target) { +export default function RFB(target, url, options) { + if (!target) { + throw Error("Must specify target"); + } + if (!url) { + throw Error("Must specify URL"); + } + this._target = target; + this._url = url; // Connection details - this._url = ''; - this._rfb_credentials = {}; + options = options || {} + this._rfb_credentials = options.credentials || {}; + this._shared = 'shared' in options ? !!options.shared : true; + this._repeaterID = options.repeaterID || ''; // Internal state this._rfb_connection_state = ''; @@ -213,6 +223,10 @@ export default function RFB(target) { Log.Warn("WebSocket on-error event"); }); + // Slight delay of the actual connection so that the caller has + // time to set up callbacks + setTimeout(this._updateConnectionState.bind(this, 'connecting')); + Log.Debug("<< RFB.constructor"); }; @@ -265,24 +279,6 @@ RFB.prototype = { // ===== PUBLIC METHODS ===== - connect: function (url, options) { - if (!url) { - this._fail(_("Must specify URL")); - return; - } - - this._url = url; - - options = options || {} - - this._rfb_credentials = options.credentials || {}; - this._shared = 'shared' in options ? !!options.shared : true; - this._repeaterID = options.repeaterID || ''; - - this._rfb_init_state = ''; - this._updateConnectionState('connecting'); - }, - disconnect: function () { this._updateConnectionState('disconnecting'); this._sock.off('error'); diff --git a/docs/API.md b/docs/API.md index 7b6596b9..8627f6c9 100644 --- a/docs/API.md +++ b/docs/API.md @@ -92,9 +92,6 @@ protocol stream. ### Methods -[`RFB.connect()`](#rfbconnect) - - Connect to a server. - [`RFB.disconnect()`](#rfbdisconnect) - Disconnect from the server. @@ -135,13 +132,12 @@ protocol stream. #### RFB() -The `RFB()` constructor returns a new `RFB` object. The object will -initially be disconnected and [`RFB.connect()`](#rfbconnect) must be -called before the object will be useful. +The `RFB()` constructor returns a new `RFB` object and initiates a new +connection to a specified VNC server. ##### Syntax - var rfb = new RFB( target ); + var rfb = new RFB( target, url [, options] ); ###### Parameters @@ -150,6 +146,35 @@ called before the object will be useful. that specifies where graphics should be rendered and input events should be monitored. +**`url`** + - A `DOMString` specifying the VNC server to connect to. This must be + a valid WebSocket URL. + +**`options`** *Optional* + - An `Object` specifying extra details about how the connection + should be made. + + Possible options: + + `shared` + - A `boolean` indicating if the remote server should be shared or + if any other connected clients should be disconnected. Enabled + by default. + + `credentials` + - An `Object` specifying the credentials to provide to the server + when authenticating. The following credentials are possible: + + | name | type | description + | ------------ | ----------- | ----------- + | `"username"` | `DOMString` | The user that authenticates + | `"password"` | `DOMString` | Password for the user + | `"target"` | `DOMString` | Target machine or session + + `repeaterID` + - A `DOMString` specifying the ID to provide to any VNC repeater + encountered. + #### RFB.onupdatestate The `onupdatestate` event handler is fired after the noVNC connection @@ -201,9 +226,9 @@ termination. #### RFB.oncredentialsrequired The `oncredentialsrequired` event handler is fired when the server -requests more credentials than were specified to -[`RFB.connect()`](#rfbconnect). The **`types`** argument is a list of -all the credentials that are required. +requests more credentials than were specified to [`RFB()`](#rfb-1). The +**`types`** argument is a list of all the credentials that are +required. ##### Syntax @@ -254,46 +279,6 @@ or removed from `RFB.capabilities`. RFB.oncapabilities = function(rfb, capabilites) { ... } -#### RFB.connect() - -The `RFB.connect()` method is used to initiate a new connection to a -specified VNC server. - -##### Syntax - - RFB.connect( url [, options] ); - -###### Parameters - -**`url`** - - A `DOMString` specifying the VNC server to connect to. This must be - a valid WebSocket URL. - -**`options`** *Optional* - - An `Object` specifying extra details about how the connection - should be made. - - Possible options: - - `shared` - - A `boolean` indicating if the remote server should be shared or - if any other connected clients should be disconnected. Enabled - by default. - - `credentials` - - An `Object` specifying the credentials to provide to the server - when authenticating. The following credentials are possible: - - | name | type | description - | ------------ | ----------- | ----------- - | `"username"` | `DOMString` | The user that authenticates - | `"password"` | `DOMString` | Password for the user - | `"target"` | `DOMString` | Target machine or session - - `repeaterID` - - A `DOMString` specifying the ID to provide to any VNC repeater - encountered. - #### RFB.disconnect() The `RFB.disconnect()` method is used to disconnect from the currently @@ -316,8 +301,7 @@ credentials after `RFB.oncredentialsrequired` has been fired. **`credentials`** - An `Object` specifying the credentials to provide to the server - when authenticating. See [`RFB.connect()`](#rfbconnect) for - details. + when authenticating. See [`RFB()`](#rfb-1) for details. #### RFB.sendKey() diff --git a/tests/playback.js b/tests/playback.js index 03748c58..2cd74998 100644 --- a/tests/playback.js +++ b/tests/playback.js @@ -77,7 +77,7 @@ export default function RecordingPlayer (frames, encoding, disconnected, notific RecordingPlayer.prototype = { run: function (realtime, trafficManagement) { // initialize a new RFB - this._rfb = new RFB(document.getElementById('VNC_canvas')); + this._rfb = new RFB(document.getElementById('VNC_canvas'), 'wss://test'); this._rfb.viewOnly = true; this._rfb.ondisconnected = this._handleDisconnect.bind(this); this._rfb.onnotification = this._notification; @@ -92,9 +92,6 @@ RecordingPlayer.prototype = { this._running = true; - // launch the tests - this._rfb.connect('wss://test'); - this._queueNextPacket(); }, @@ -104,12 +101,8 @@ RecordingPlayer.prototype = { this._rfb._sock.close = function () {}; this._rfb._sock.flush = function () {}; this._rfb._checkEvents = function () {}; - this._rfb.connect = function (url) { - this._url = url; - this._rfb_credentials = {}; + this._rfb._connect = function () { this._sock.init('binary', 'ws'); - this._rfb_connection_state = 'connecting'; - this._rfb_init_state = 'ProtocolVersion'; }; }, diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 5a41918b..c218ae07 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -9,10 +9,6 @@ import { encodings } from '../core/encodings.js'; import FakeWebSocket from './fake.websocket.js'; import sinon from '../vendor/sinon.js'; -function make_rfb () { - return new RFB(document.createElement('canvas')); -} - var push8 = function (arr, num) { "use strict"; arr.push(num & 0xFF); @@ -33,12 +29,13 @@ var push32 = function (arr, num) { }; describe('Remote Frame Buffer Protocol Client', function() { - "use strict"; + var clock; + before(FakeWebSocket.replace); after(FakeWebSocket.restore); before(function () { - this.clock = sinon.useFakeTimers(); + this.clock = clock = sinon.useFakeTimers(); // Use a single set of buffers instead of reallocating to // speed up tests var sock = new Websock(); @@ -58,38 +55,46 @@ describe('Remote Frame Buffer Protocol Client', function() { this.clock.restore(); }); + function make_rfb (url, options) { + url = url || 'wss://host:8675'; + var rfb = new RFB(document.createElement('canvas'), url, options); + clock.tick(); + rfb._sock._websocket._open(); + rfb._rfb_connection_state = 'connected'; + return rfb; + } + describe('Connecting/Disconnecting', function () { - var client; - beforeEach(function () { - client = make_rfb(); - }); - - describe('#connect', function () { - beforeEach(function () { client._updateConnectionState = sinon.spy(); }); - + describe('#RFB', function () { it('should set the current state to "connecting"', function () { - client.connect('wss://host:8675'); - expect(client._updateConnectionState).to.have.been.calledOnce; - expect(client._updateConnectionState).to.have.been.calledWith('connecting'); + var client = new RFB(document.createElement('canvas'), 'wss://host:8675'); + client.onupdatestate = sinon.spy(); + this.clock.tick(); + expect(client.onupdatestate).to.have.been.calledOnce; + expect(client.onupdatestate).to.have.been.calledWith(client, 'connecting'); }); - it('should not try to connect if we are missing a URL', function () { - client._fail = sinon.spy(); - client._rfb_connection_state = ''; - client.connect(undefined); - expect(client._fail).to.have.been.calledOnce; - expect(client._updateConnectionState).to.not.have.been.called; - expect(client._rfb_connection_state).to.equal(''); + it('should actually connect to the websocket', function () { + var client = new RFB(document.createElement('canvas'), 'ws://HOST:8675/PATH'); + sinon.spy(client._sock, 'open'); + this.clock.tick(); + expect(client._sock.open).to.have.been.calledOnce; + expect(client._sock.open).to.have.been.calledWith('ws://HOST:8675/PATH'); }); }); describe('#disconnect', function () { - beforeEach(function () { client._updateConnectionState = sinon.spy(); }); + var client; + beforeEach(function () { + client = make_rfb(); + }); it('should set the current state to "disconnecting"', function () { + client.onupdatestate = sinon.spy(); client.disconnect(); - expect(client._updateConnectionState).to.have.been.calledOnce; - expect(client._updateConnectionState).to.have.been.calledWith('disconnecting'); + expect(client.onupdatestate).to.have.been.calledTwice; + expect(client.onupdatestate).to.have.been.calledWith(client, 'disconnecting'); + expect(client.onupdatestate).to.have.been.calledWith(client, 'disconnected'); }); it('should unregister error event handler', function () { @@ -112,6 +117,12 @@ describe('Remote Frame Buffer Protocol Client', function() { }); describe('#sendCredentials', function () { + var client; + beforeEach(function () { + client = make_rfb(); + client._rfb_connection_state = 'connecting'; + }); + it('should set the rfb credentials properly"', function () { client.sendCredentials({ password: 'pass' }); expect(client._rfb_credentials).to.deep.equal({ password: 'pass' }); @@ -130,10 +141,6 @@ describe('Remote Frame Buffer Protocol Client', function() { var client; beforeEach(function () { client = make_rfb(); - client._sock = new Websock(); - client._sock.open('ws://', 'binary'); - client._sock._websocket._open(); - client._rfb_connection_state = 'connected'; }); describe('#sendCtrlAlDel', function () { @@ -304,7 +311,8 @@ 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('connecting'); + client._rfb_connection_state = 'connecting'; + client._updateConnectionState('connected'); this.clock.tick(51); expect(spy).to.not.have.been.called; expect(client._disconnTimer).to.be.null; @@ -312,9 +320,9 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should call the updateState callback', function () { client.onupdatestate = sinon.spy(); - client._updateConnectionState('connecting'); + client._updateConnectionState('disconnecting'); var spy = client.onupdatestate; - expect(spy.args[0][1]).to.equal('connecting'); + expect(spy.args[0][1]).to.equal('disconnecting'); }); it('should set the rfb_connection_state', function () { @@ -351,7 +359,6 @@ describe('Remote Frame Buffer Protocol Client', function() { var client; beforeEach(function () { client = make_rfb(); - client.connect('wss://host:8675'); }); it('should close the WebSocket connection', function () { @@ -416,29 +423,10 @@ describe('Remote Frame Buffer Protocol Client', function() { }); describe('Connection States', function () { - describe('connecting', function () { - var client; - beforeEach(function () { client = make_rfb(); }); - - it('should actually connect to the websocket', function () { - sinon.spy(client._sock, 'open'); - client._updateConnectionState('connecting'); - expect(client._sock.open).to.have.been.calledOnce; - }); - - it('should use a url specified to connect', function () { - sinon.spy(client._sock, 'open'); - client._url = 'ws://HOST:8675/PATH'; - client._updateConnectionState('connecting'); - expect(client._sock.open).to.have.been.calledWith('ws://HOST:8675/PATH'); - }); - }); - describe('disconnecting', function () { var client; beforeEach(function () { client = make_rfb(); - client.connect('wss://host:8675'); }); it('should force disconnect if we do not call Websock.onclose within the disconnection timeout', function () { @@ -482,6 +470,7 @@ describe('Remote Frame Buffer Protocol Client', function() { it('should not call the disconnect callback if the state is not "disconnected"', function () { client.ondisconnected = sinon.spy(); + client._sock._websocket.close = function () {}; // explicitly don't call onclose client._updateConnectionState('disconnecting'); var spy = client.ondisconnected; expect(spy).to.not.have.been.called; @@ -515,8 +504,7 @@ describe('Remote Frame Buffer Protocol Client', function() { var client; beforeEach(function () { client = make_rfb(); - client.connect('wss://host:8675'); - client._sock._websocket._open(); + client._rfb_connection_state = 'connecting'; }); describe('ProtocolVersion', function () { @@ -597,9 +585,8 @@ describe('Remote Frame Buffer Protocol Client', function() { describe('Repeater', function () { beforeEach(function () { - client = make_rfb(); - client.connect('wss://host:8675', { repeaterID: "12345" }); - client._sock._websocket._open(); + client = make_rfb('wss://host:8675', { repeaterID: "12345" }); + client._rfb_connection_state = 'connecting'; }); it('should interpret version 000.000 as a repeater', function () { @@ -933,31 +920,25 @@ describe('Remote Frame Buffer Protocol Client', function() { }); describe('ClientInitialisation', function () { - var client; - - beforeEach(function () { - client = make_rfb(); - }); - it('should transition to the ServerInitialisation state', function () { - client.connect('wss://host:8675'); - client._sock._websocket._open(); + var client = make_rfb(); + client._rfb_connection_state = 'connecting'; client._rfb_init_state = 'SecurityResult'; client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 0])); expect(client._rfb_init_state).to.equal('ServerInitialisation'); }); it('should send 1 if we are in shared mode', function () { - client.connect('wss://host:8675', { shared: true }); - client._sock._websocket._open(); + var client = make_rfb('wss://host:8675', { shared: true }); + client._rfb_connection_state = 'connecting'; client._rfb_init_state = 'SecurityResult'; client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 0])); expect(client._sock).to.have.sent(new Uint8Array([1])); }); it('should send 0 if we are not in shared mode', function () { - client.connect('wss://host:8675', { shared: false }); - client._sock._websocket._open(); + var client = make_rfb('wss://host:8675', { shared: false }); + client._rfb_connection_state = 'connecting'; client._rfb_init_state = 'SecurityResult'; client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 0])); expect(client._sock).to.have.sent(new Uint8Array([0])); @@ -1120,9 +1101,6 @@ describe('Remote Frame Buffer Protocol Client', function() { beforeEach(function () { client = make_rfb(); - client.connect('wss://host:8675'); - client._sock._websocket._open(); - client._rfb_connection_state = 'connected'; client._fb_name = 'some device'; client._fb_width = 640; client._fb_height = 20; @@ -1722,10 +1700,6 @@ describe('Remote Frame Buffer Protocol Client', function() { var client; beforeEach(function () { client = make_rfb(); - client._sock = new Websock(); - client._sock.open('ws://', 'binary'); - client._sock._websocket._open(); - client._rfb_connection_state = 'connected'; }); describe('Mouse event handlers', function () { @@ -1854,28 +1828,21 @@ describe('Remote Frame Buffer Protocol Client', function() { }); describe('WebSocket event handlers', function () { - var client; - beforeEach(function () { - client = make_rfb(); - client.connect('wss://host:8675'); - }); - // message events it ('should do nothing if we receive an empty message and have nothing in the queue', function () { - client._rfb_connection_state = 'connected'; client._normal_msg = sinon.spy(); client._sock._websocket._receive_data(new Uint8Array([])); expect(client._normal_msg).to.not.have.been.called; }); it('should handle a message in the connected state as a normal message', function () { - client._rfb_connection_state = 'connected'; client._normal_msg = sinon.spy(); client._sock._websocket._receive_data(new Uint8Array([1, 2, 3])); expect(client._normal_msg).to.have.been.calledOnce; }); it('should handle a message in any non-disconnected/failed state like an init message', function () { + client._rfb_connection_state = 'connecting'; client._rfb_init_state = 'ProtocolVersion'; client._init_msg = sinon.spy(); client._sock._websocket._receive_data(new Uint8Array([1, 2, 3])); @@ -1883,8 +1850,6 @@ describe('Remote Frame Buffer Protocol Client', function() { }); it('should process all normal messages directly', function () { - client._sock._websocket._open(); - client._rfb_connection_state = 'connected'; client.onbell = sinon.spy(); client._sock._websocket._receive_data(new Uint8Array([0x02, 0x02])); expect(client.onbell).to.have.been.calledTwice; @@ -1892,6 +1857,8 @@ describe('Remote Frame Buffer Protocol Client', function() { // open events it('should update the state to ProtocolVersion on open (if the state is "connecting")', function () { + client = new RFB(document.createElement('canvas'), 'wss://host:8675'); + this.clock.tick(); client._sock._websocket._open(); expect(client._rfb_init_state).to.equal('ProtocolVersion'); }); diff --git a/vnc_lite.html b/vnc_lite.html index 153244b9..f457423b 100644 --- a/vnc_lite.html +++ b/vnc_lite.html @@ -255,20 +255,6 @@ status('Must specify host and port in URL', 'error'); } - try { - rfb = new RFB(document.getElementById('noVNC_canvas')); - rfb.viewOnly = WebUtil.getConfigVar('view_only', false); - rfb.onnotification = notification; - rfb.onupdatestate = updateState; - rfb.ondisconnected = disconnected; - rfb.oncapabilities = function () { updatePowerButtons(); initialResize(); }; - rfb.oncredentialsrequired = credentials; - rfb.ondesktopname = updateDesktopName; - } catch (exc) { - status('Unable to create RFB client -- ' + exc, 'error'); - return; // don't continue trying to connect - } - var url; if (WebUtil.getConfigVar('encrypt', @@ -284,9 +270,17 @@ } url += '/' + path; - rfb.connect(url, { repeaterID: WebUtil.getConfigVar('repeaterID', ''), - shared: WebUtil.getConfigVar('shared', true), - credentials: { password: password } }); + rfb = new RFB(document.getElementById('noVNC_canvas'), url, + { repeaterID: WebUtil.getConfigVar('repeaterID', ''), + shared: WebUtil.getConfigVar('shared', true), + credentials: { password: password } }); + rfb.viewOnly = WebUtil.getConfigVar('view_only', false); + rfb.onnotification = notification; + rfb.onupdatestate = updateState; + rfb.ondisconnected = disconnected; + rfb.oncapabilities = function () { updatePowerButtons(); initialResize(); }; + rfb.oncredentialsrequired = credentials; + rfb.ondesktopname = updateDesktopName; })();