From de9fc9508cb4bc6776792f0c7731da17519d4f24 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 18 Apr 2021 13:55:11 +0200 Subject: [PATCH] Don't fake open events in Websock We don't know if the caller is prepared to receive those events right now as normally they would get them on a fresh new stack later. We also can't delay delivery since then we might deliver the event after any pending "message" events. Better to push the problem one layer up to the caller, which now needs to be more aware of the state of the WebSocket object it is trying to use. --- core/rfb.js | 4 ++++ core/websock.js | 8 +------- tests/test.rfb.js | 14 ++++++++++++++ tests/test.websock.js | 15 +-------------- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 2257da3c..074eb519 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -484,6 +484,10 @@ export default class RFB extends EventTargetMixin { if (this._sock.readyState === 'closed') { this._fail("Cannot use already closed WebSocket/RTCDataChannel"); } + + if (this._sock.readyState === 'open') { + this._socketOpen(); + } } // Make our elements part of the page diff --git a/core/websock.js b/core/websock.js index 52e27be5..37b33fcc 100644 --- a/core/websock.js +++ b/core/websock.js @@ -247,7 +247,7 @@ export default class Websock { this._websocket.binaryType = "arraybuffer"; this._websocket.onmessage = this._recvMessage.bind(this); - const onOpen = () => { + this._websocket.onopen = () => { Log.Debug('>> WebSock.onopen'); if (this._websocket.protocol) { Log.Info("Server choose sub-protocol: " + this._websocket.protocol); @@ -257,12 +257,6 @@ export default class Websock { Log.Debug("<< WebSock.onopen"); }; - if (this.readyState === 'open') { - onOpen(); - } else { - this._websocket.onopen = onOpen; - } - this._websocket.onclose = (e) => { Log.Debug(">> WebSock.onclose"); this._eventHandlers.close(e); diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 2c112f3a..c99edc75 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -183,6 +183,20 @@ describe('Remote Frame Buffer Protocol Client', function () { expect(attach).to.have.been.calledOnceWithExactly(sock); }); + it('should handle already open WebSocket/RTCDataChannel objects', function () { + let sock = new FakeWebSocket('ws://HOST:8675/PATH', []); + sock._open(); + const client = new RFB(document.createElement('div'), sock); + let callback = sinon.spy(); + client.addEventListener('disconnect', callback); + this.clock.tick(); + expect(open).to.not.have.been.called; + expect(attach).to.have.been.calledOnceWithExactly(sock); + // Check if it is ready for some data + sock._receiveData(new Uint8Array(['R', 'F', 'B', '0', '0', '3', '0', '0', '8'])); + expect(callback).to.not.have.been.called; + }); + it('should refuse closed WebSocket/RTCDataChannel objects', function () { let sock = new FakeWebSocket('ws://HOST:8675/PATH', []); sock.readyState = WebSocket.CLOSED; diff --git a/tests/test.websock.js b/tests/test.websock.js index 1c2aa13c..857fdca8 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -271,23 +271,10 @@ describe('Websock', function () { }); describe('attaching', function () { - it('should attach to an existing open websocket', function () { + it('should attach to an existing websocket', function () { let ws = new FakeWebSocket('ws://localhost:8675'); - ws._open(); - let callback = sinon.spy(); - sock.on('open', callback); sock.attach(ws); expect(WebSocket).to.not.have.been.called; - expect(callback).to.have.been.calledOnce; - }); - - it('should attach to an existing connecting websocket', function () { - let ws = new FakeWebSocket('ws://localhost:8675'); - let callback = sinon.spy(); - sock.on('open', callback); - sock.attach(ws); - expect(WebSocket).to.not.have.been.called; - expect(callback).to.not.have.been.called; }); });