Merge branch 'slowdata' of https://github.com/CendioOssman/noVNC

core/rfb.js

@@ -57,7 +57,7 @@ export default class RFB extends EventTargetMixin {
         // Internal state
         this._rfb_connection_state = '';
         this._rfb_init_state = '';
-        this._rfb_auth_scheme = '';
+        this._rfb_auth_scheme = -1;
         this._rfb_clean_disconnect = true;
 
         // Server capabilities
@@ -182,7 +182,9 @@ export default class RFB extends EventTargetMixin {
         this._mouse.onmousemove = this._handleMouseMove.bind(this);
 
         this._sock = new Websock();
-        this._sock.on('message', this._handle_message.bind(this));
+        this._sock.on('message', () => {
+            this._handle_message();
+        });
         this._sock.on('open', () => {
             if ((this._rfb_connection_state === 'connecting') &&
                 (this._rfb_init_state === '')) {
@@ -783,8 +785,8 @@ export default class RFB extends EventTargetMixin {
     // Message Handlers
 
     _negotiate_protocol_version() {
-        if (this._sock.rQlen < 12) {
+        if (this._sock.rQwait("version", 12)) {
-            return this._fail("Received incomplete protocol version.");
+            return false;
         }
 
         const sversion = this._sock.rQshiftStr(12).substr(4, 7);
@@ -851,14 +853,16 @@ export default class RFB extends EventTargetMixin {
             if (this._sock.rQwait("security type", num_types, 1)) { return false; }
 
             if (num_types === 0) {
-                return this._handle_security_failure("no security types");
+                this._rfb_init_state = "SecurityReason";
+                this._security_context = "no security types";
+                this._security_status = 1;
+                return this._init_msg();
             }
 
             const types = this._sock.rQshiftBytes(num_types);
             Log.Debug("Server security types: " + types);
 
             // Look for each auth in preferred order
-            this._rfb_auth_scheme = 0;
             if (includes(1, types)) {
                 this._rfb_auth_scheme = 1; // None
             } else if (includes(22, types)) {
@@ -876,6 +880,13 @@ export default class RFB extends EventTargetMixin {
             // Server decides
             if (this._sock.rQwait("security scheme", 4)) { return false; }
             this._rfb_auth_scheme = this._sock.rQshift32();
+
+            if (this._rfb_auth_scheme == 0) {
+                this._rfb_init_state = "SecurityReason";
+                this._security_context = "authentication scheme";
+                this._security_status = 1;
+                return this._init_msg();
+            }
         }
 
         this._rfb_init_state = 'Authentication';
@@ -884,28 +895,7 @@ export default class RFB extends EventTargetMixin {
         return this._init_msg(); // jump to authentication
     }
 
-    /*
+    _handle_security_reason() {
-     * Get the security failure reason if sent from the server and
-     * send the 'securityfailure' event.
-     *
-     * - The optional parameter context can be used to add some extra
-     *   context to the log output.
-     *
-     * - The optional parameter security_result_status can be used to
-     *   add a custom status code to the event.
-     */
-    _handle_security_failure(context, security_result_status) {
-
-        if (typeof context === 'undefined') {
-            context = "";
-        } else {
-            context = " on " + context;
-        }
-
-        if (typeof security_result_status === 'undefined') {
-            security_result_status = 1; // fail
-        }
-
         if (this._sock.rQwait("reason length", 4)) {
             return false;
         }
@@ -913,23 +903,26 @@ export default class RFB extends EventTargetMixin {
         let reason = "";
 
         if (strlen > 0) {
-            if (this._sock.rQwait("reason", strlen, 8)) { return false; }
+            if (this._sock.rQwait("reason", strlen, 4)) { return false; }
             reason = this._sock.rQshiftStr(strlen);
         }
 
         if (reason !== "") {
             this.dispatchEvent(new CustomEvent(
                 "securityfailure",
-                { detail: { status: security_result_status, reason: reason } }));
+                { detail: { status: this._security_status,
+                            reason: reason } }));
 
-            return this._fail("Security negotiation failed" + context +
+            return this._fail("Security negotiation failed on " +
+                              this._security_context +
                               " (reason: " + reason + ")");
         } else {
             this.dispatchEvent(new CustomEvent(
                 "securityfailure",
-                { detail: { status: security_result_status } }));
+                { detail: { status: this._security_status } }));
 
-            return this._fail("Security negotiation failed" + context);
+            return this._fail("Security negotiation failed on " +
+                              this._security_context);
         }
     }
 
@@ -1075,9 +1068,6 @@ export default class RFB extends EventTargetMixin {
 
     _negotiate_authentication() {
         switch (this._rfb_auth_scheme) {
-            case 0:  // connection failed
-                return this._handle_security_failure("authentication scheme");
-
             case 1:  // no auth
                 if (this._rfb_version >= 3.8) {
                     this._rfb_init_state = 'SecurityResult';
@@ -1112,7 +1102,10 @@ export default class RFB extends EventTargetMixin {
             return this._init_msg();
         } else {
             if (this._rfb_version >= 3.8) {
-                return this._handle_security_failure("security result", status);
+                this._rfb_init_state = "SecurityReason";
+                this._security_context = "security result";
+                this._security_status = status;
+                return this._init_msg();
             } else {
                 this.dispatchEvent(new CustomEvent(
                     "securityfailure",
@@ -1281,6 +1274,9 @@ export default class RFB extends EventTargetMixin {
             case 'SecurityResult':
                 return this._handle_security_result();
 
+            case 'SecurityReason':
+                return this._handle_security_reason();
+
             case 'ClientInitialisation':
                 this._sock.send([this._shared ? 1 : 0]); // ClientInitialisation
                 this._rfb_init_state = 'ServerInitialisation';

tests/fake.websocket.js

@@ -63,7 +63,12 @@ export default class FakeWebSocket {
     }
 
     _receive_data(data) {
-        this.onmessage(make_event("message", { 'data': data }));
+        // Break apart the data to expose bugs where we assume data is
+        // neatly packaged
+        for (let i = 0;i < data.length;i++) {
+            let buf = data.subarray(i, i+1);
+            this.onmessage(make_event("message", { 'data': buf }));
+        }
     }
 }
 

tests/test.rfb.js

@@ -1039,14 +1039,14 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 const auth_scheme_raw = [1, 2, 3, 4];
                 const auth_scheme = (auth_scheme_raw[0] << 24) + (auth_scheme_raw[1] << 16) +
                                   (auth_scheme_raw[2] << 8) + auth_scheme_raw[3];
-                client._sock._websocket._receive_data(auth_scheme_raw);
+                client._sock._websocket._receive_data(new Uint8Array(auth_scheme_raw));
                 expect(client._rfb_auth_scheme).to.equal(auth_scheme);
             });
 
             it('should prefer no authentication is possible', function () {
                 client._rfb_version = 3.7;
                 const auth_schemes = [2, 1, 3];
-                client._sock._websocket._receive_data(auth_schemes);
+                client._sock._websocket._receive_data(new Uint8Array(auth_schemes));
                 expect(client._rfb_auth_scheme).to.equal(1);
                 expect(client._sock).to.have.sent(new Uint8Array([1, 1]));
             });
@@ -1054,7 +1054,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
             it('should choose for the most prefered scheme possible for versions >= 3.7', function () {
                 client._rfb_version = 3.7;
                 const auth_schemes = [2, 22, 16];
-                client._sock._websocket._receive_data(auth_schemes);
+                client._sock._websocket._receive_data(new Uint8Array(auth_schemes));
                 expect(client._rfb_auth_scheme).to.equal(22);
                 expect(client._sock).to.have.sent(new Uint8Array([22]));
             });
@@ -1063,7 +1063,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 sinon.spy(client, "_fail");
                 client._rfb_version = 3.7;
                 const auth_schemes = [1, 32];
-                client._sock._websocket._receive_data(auth_schemes);
+                client._sock._websocket._receive_data(new Uint8Array(auth_schemes));
                 expect(client._fail).to.have.been.calledOnce;
             });
 
@@ -1071,7 +1071,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 client._rfb_version = 3.7;
                 const failure_data = [0, 0, 0, 0, 6, 119, 104, 111, 111, 112, 115];
                 sinon.spy(client, '_fail');
-                client._sock._websocket._receive_data(failure_data);
+                client._sock._websocket._receive_data(new Uint8Array(failure_data));
 
                 expect(client._fail).to.have.been.calledOnce;
                 expect(client._fail).to.have.been.calledWith(
@@ -1082,7 +1082,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 client._rfb_version = 3.7;
                 const auth_schemes = [1, 1];
                 client._negotiate_authentication = sinon.spy();
-                client._sock._websocket._receive_data(auth_schemes);
+                client._sock._websocket._receive_data(new Uint8Array(auth_schemes));
                 expect(client._rfb_init_state).to.equal('Authentication');
                 expect(client._negotiate_authentication).to.have.been.calledOnce;
             });
@@ -1485,7 +1485,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 for (let i = 0; i < 16 + 32 + 48; i++) {
                     tight_data.push(i);
                 }
-                client._sock._websocket._receive_data(tight_data);
+                client._sock._websocket._receive_data(new Uint8Array(tight_data));
 
                 expect(client._rfb_connection_state).to.equal('connected');
             });
@@ -2325,7 +2325,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
             it('should handle a message in the connected state as a normal message', function () {
                 client._normal_msg = sinon.spy();
                 client._sock._websocket._receive_data(new Uint8Array([1, 2, 3]));
-                expect(client._normal_msg).to.have.been.calledOnce;
+                expect(client._normal_msg).to.have.been.called;
             });
 
             it('should handle a message in any non-disconnected/failed state like an init message', function () {
@@ -2333,7 +2333,7 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 client._rfb_init_state = 'ProtocolVersion';
                 client._init_msg = sinon.spy();
                 client._sock._websocket._receive_data(new Uint8Array([1, 2, 3]));
-                expect(client._init_msg).to.have.been.calledOnce;
+                expect(client._init_msg).to.have.been.called;
             });
 
             it('should process all normal messages directly', function () {