Fixed a race condition when attaching to an existing socket

[mirror_novnc.git] / core / rfb.js

diff --git a/core/rfb.js b/core/rfb.js
index 26cdfcd0b57c202ab0d6414ae60103031455f581..f8eeb51b4e8b08b9ea184bae7625a7e352a81ed9 100644 (file)
--- a/core/rfb.js
+++ b/core/rfb.js
@@ -66,20 +66,25 @@ const extendedClipboardActionPeek    = 1 << 26;
 const extendedClipboardActionNotify  = 1 << 27;
 const extendedClipboardActionProvide = 1 << 28;
 
-
 export default class RFB extends EventTargetMixin {
-    constructor(target, url, options) {
+    constructor(target, urlOrChannel, options) {
         if (!target) {
             throw new Error("Must specify target");
         }
-        if (!url) {
-            throw new Error("Must specify URL");
+        if (!urlOrChannel) {
+            throw new Error("Must specify URL, WebSocket or RTCDataChannel");
         }
 
         super();
 
         this._target = target;
-        this._url = url;
+
+        if (typeof urlOrChannel === "string") {
+            this._url = urlOrChannel;
+        } else {
+            this._url = null;
+            this._rawChannel = urlOrChannel;
+        }
 
         // Connection details
         options = options || {};
@@ -275,12 +280,23 @@ export default class RFB extends EventTargetMixin {
                     break;
             }
             this._sock.off('close');
+            // Delete reference to raw channel to allow cleanup.
+            this._rawChannel = null;
         });
         this._sock.on('error', e => 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'));
+        // time to set up callbacks.
+        // This it not possible when a pre-existing socket is passed in and is just opened.
+        // If the caller creates this object in the open() callback of a socket and there's
+        // data pending doing it next tick causes a packet to be lost.
+        // This is particularly noticable for RTCDataChannel's where the other end creates
+        // the channel and the client, this end, gets notified it exists.
+        if (typeof urlOrChannel === 'string') {
+            setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+        } else {
+            this._updateConnectionState('connecting');
+        }
 
         Log.Debug("<< RFB.constructor");
 
@@ -501,16 +517,23 @@ export default class RFB extends EventTargetMixin {
     _connect() {
         Log.Debug(">> RFB.connect");
 
-        Log.Info("connecting to " + this._url);
-
-        try {
-            // WebSocket.onopen transitions to the RFB init states
-            this._sock.open(this._url, this._wsProtocols);
-        } catch (e) {
-            if (e.name === 'SyntaxError') {
-                this._fail("Invalid host or port (" + e + ")");
-            } else {
-                this._fail("Error when opening socket (" + e + ")");
+        if (this._url) {
+            try {
+                Log.Info(`connecting to ${this._url}`);
+                this._sock.open(this._url, this._wsProtocols);
+            } catch (e) {
+                if (e.name === 'SyntaxError') {
+                    this._fail("Invalid host or port (" + e + ")");
+                } else {
+                    this._fail("Error when opening socket (" + e + ")");
+                }
+            }
+        } else {
+            try {
+                Log.Info(`attaching ${this._rawChannel} to Websock`);
+                this._sock.attach(this._rawChannel);
+            } catch (e) {
+                this._fail("Error attaching channel (" + e + ")");
             }
         }
 
@@ -1427,8 +1450,8 @@ export default class RFB extends EventTargetMixin {
 
         // negotiated Plain subtype, server waits for password
         if (this._rfbVeNCryptState == 4) {
-            if (!this._rfbCredentials.username ||
-                !this._rfbCredentials.password) {
+            if (this._rfbCredentials.username === undefined ||
+                this._rfbCredentials.password === undefined) {
                 this.dispatchEvent(new CustomEvent(
                     "credentialsrequired",
                     { detail: { types: ["username", "password"] } }));
@@ -1438,9 +1461,18 @@ export default class RFB extends EventTargetMixin {
             const user = encodeUTF8(this._rfbCredentials.username);
             const pass = encodeUTF8(this._rfbCredentials.password);
 
-            // XXX we assume lengths are <= 255 (should not be an issue in the real world)
-            this._sock.send([0, 0, 0, user.length]);
-            this._sock.send([0, 0, 0, pass.length]);
+            this._sock.send([
+                (user.length >> 24) & 0xFF,
+                (user.length >> 16) & 0xFF,
+                (user.length >> 8) & 0xFF,
+                user.length & 0xFF
+            ]);
+            this._sock.send([
+                (pass.length >> 24) & 0xFF,
+                (pass.length >> 16) & 0xFF,
+                (pass.length >> 8) & 0xFF,
+                pass.length & 0xFF
+            ]);
             this._sock.sendString(user);
             this._sock.sendString(pass);