Stop exposing Websock queue length

Callers should be using rQwait() to ensure sufficient data is present, and not poke around in the internal buffering.
novnc · CendioOssman · Jun 30, 2023 · May 15, 2023 · May 11, 2023 · May 11, 2023
commit 55ffe8fc513b720a8bf84178d3c81af7a1ce314c
diff --git a/core/decoders/raw.js b/core/decoders/raw.js
@@ -24,38 +24,34 @@ export default class RawDecoder {
         const pixelSize = depth == 8 ? 1 : 4;
         const bytesPerLine = width * pixelSize;
 
-        if (sock.rQwait("RAW", bytesPerLine)) {
-            return false;
-        }
+        while (this._lines > 0) {
+            if (sock.rQwait("RAW", bytesPerLine)) {
+                return false;
+            }
+
+            const curY = y + (height - this._lines);
+
+            let data = sock.rQshiftBytes(bytesPerLine);
 
-        const curY = y + (height - this._lines);
-        const currHeight = Math.min(this._lines,
-                                    Math.floor(sock.rQlen / bytesPerLine));
-        const pixels = width * currHeight;
-
-        let data = sock.rQshiftBytes(currHeight * bytesPerLine);
-
-        // Convert data if needed
-        if (depth == 8) {
-            const newdata = new Uint8Array(pixels * 4);
-            for (let i = 0; i < pixels; i++) {
-                newdata[i * 4 + 0] = ((data[i] >> 0) & 0x3) * 255 / 3;
-                newdata[i * 4 + 1] = ((data[i] >> 2) & 0x3) * 255 / 3;
-                newdata[i * 4 + 2] = ((data[i] >> 4) & 0x3) * 255 / 3;
-                newdata[i * 4 + 3] = 255;
+            // Convert data if needed
+            if (depth == 8) {
+                const newdata = new Uint8Array(width * 4);
+                for (let i = 0; i < width; i++) {
+                    newdata[i * 4 + 0] = ((data[i] >> 0) & 0x3) * 255 / 3;
+                    newdata[i * 4 + 1] = ((data[i] >> 2) & 0x3) * 255 / 3;
+                    newdata[i * 4 + 2] = ((data[i] >> 4) & 0x3) * 255 / 3;
+                    newdata[i * 4 + 3] = 255;
+                }
+                data = newdata;
             }
-            data = newdata;
-        }
 
-        // Max sure the image is fully opaque
-        for (let i = 0; i < pixels; i++) {
-            data[i * 4 + 3] = 255;
-        }
+            // Max sure the image is fully opaque
+            for (let i = 0; i < width; i++) {
+                data[i * 4 + 3] = 255;
+            }
 
-        display.blitImage(x, curY, width, currHeight, data, 0);
-        this._lines -= currHeight;
-        if (this._lines > 0) {
-            return false;
+            display.blitImage(x, curY, width, 1, data, 0);
+            this._lines--;
         }
 
         return true;

diff --git a/core/rfb.js b/core/rfb.js
@@ -958,7 +958,7 @@ export default class RFB extends EventTargetMixin {
     }
 
     _handleMessage() {
-        if (this._sock.rQlen === 0) {
+        if (this._sock.rQwait("message", 1)) {
             Log.Warn("handleMessage called on an empty receive queue");
             return;
         }
@@ -975,7 +975,7 @@ export default class RFB extends EventTargetMixin {
                     if (!this._normalMsg()) {
                         break;
                     }
-                    if (this._sock.rQlen === 0) {
+                    if (this._sock.rQwait("message", 1)) {
                         break;
                     }
                 }
@@ -2473,7 +2473,7 @@ export default class RFB extends EventTargetMixin {
                     .then(() => {
                         this._flushing = false;
                         // Resume processing
-                        if (this._sock.rQlen > 0) {
+                        if (!this._sock.rQwait("message", 1)) {
                             this._handleMessage();
                         }
                     });

diff --git a/core/websock.js b/core/websock.js
@@ -95,10 +95,6 @@ export default class Websock {
     }
 
     // Receive Queue
-    get rQlen() {
-        return this._rQlen - this._rQi;
-    }
-
     rQpeek8() {
         return this._rQ[this._rQi];
     }
@@ -129,7 +125,7 @@ export default class Websock {
     }
 
     rQshiftStr(len) {
-        if (typeof(len) === 'undefined') { len = this.rQlen; }
+        if (typeof(len) === 'undefined') { len = this._rQlen - this._rQi; }
         let str = "";
         // Handle large arrays in steps to avoid long strings on the stack
         for (let i = 0; i < len; i += 4096) {
@@ -140,28 +136,28 @@ export default class Websock {
     }
 
     rQshiftBytes(len) {
-        if (typeof(len) === 'undefined') { len = this.rQlen; }
+        if (typeof(len) === 'undefined') { len = this._rQlen - this._rQi; }
         this._rQi += len;
         return new Uint8Array(this._rQ.buffer, this._rQi - len, len);
     }
 
     rQshiftTo(target, len) {
-        if (len === undefined) { len = this.rQlen; }
+        if (len === undefined) { len = this._rQlen - this._rQi; }
         // TODO: make this just use set with views when using a ArrayBuffer to store the rQ
         target.set(new Uint8Array(this._rQ.buffer, this._rQi, len));
         this._rQi += len;
     }
 
     rQpeekBytes(len) {
-        if (typeof(len) === 'undefined') { len = this.rQlen; }
+        if (typeof(len) === 'undefined') { len = this._rQlen - this._rQi; }
         return new Uint8Array(this._rQ.buffer, this._rQi, len);
     }
 
     // Check to see if we must wait for 'num' bytes (default to FBU.bytes)
     // to be available in the receive queue. Return true if we need to
     // wait (and possibly print a debug message), otherwise false.
     rQwait(msg, num, goback) {
-        if (this.rQlen < num) {
+        if (this._rQlen - this._rQi < num) {
             if (goback) {
                 if (this._rQi < goback) {
                     throw new Error("rQwait cannot backup " + goback + " bytes");
@@ -294,7 +290,7 @@ export default class Websock {
         // we don't want to grow unboundedly
         if (this._rQbufferSize > MAX_RQ_GROW_SIZE) {
             this._rQbufferSize = MAX_RQ_GROW_SIZE;
-            if (this._rQbufferSize - this.rQlen < minFit) {
+            if (this._rQbufferSize - (this._rQlen - this._rQi) < minFit) {
                 throw new Error("Receive Queue buffer exceeded " + MAX_RQ_GROW_SIZE + " bytes, and the new message could not fit");
             }
         }
@@ -323,7 +319,7 @@ export default class Websock {
 
     _recvMessage(e) {
         this._DecodeMessage(e.data);
-        if (this.rQlen > 0) {
+        if (this._rQlen - this._rQi > 0) {
             this._eventHandlers.message();
             if (this._rQlen == this._rQi) {
                 // All data has now been processed, this means we

diff --git a/tests/test.websock.js b/tests/test.websock.js
@@ -17,56 +17,43 @@ describe('Websock', function () {
             sock._rQ.set(RQ_TEMPLATE);
             sock._rQlen = RQ_TEMPLATE.length;
         });
-        describe('rQlen', function () {
-            it('should return the length of the receive queue', function () {
-                sock._rQi = 0;
-
-                expect(sock.rQlen).to.equal(RQ_TEMPLATE.length);
-            });
-
-            it("should return the proper length if we read some from the receive queue", function () {
-                sock._rQi = 1;
-
-                expect(sock.rQlen).to.equal(RQ_TEMPLATE.length - 1);
-            });
-        });
 
         describe('rQpeek8', function () {
             it('should peek at the next byte without poping it off the queue', function () {
-                const befLen = sock.rQlen;
+                const befLen = sock._rQlen - sock._rQi;
                 const peek = sock.rQpeek8();
                 expect(sock.rQpeek8()).to.equal(peek);
-                expect(sock.rQlen).to.equal(befLen);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen);
             });
         });
 
         describe('rQshift8()', function () {
             it('should pop a single byte from the receive queue', function () {
                 const peek = sock.rQpeek8();
-                const befLen = sock.rQlen;
+                const befLen = sock._rQlen - sock._rQi;
                 expect(sock.rQshift8()).to.equal(peek);
-                expect(sock.rQlen).to.equal(befLen - 1);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen - 1);
             });
         });
 
         describe('rQshift16()', function () {
             it('should pop two bytes from the receive queue and return a single number', function () {
-                const befLen = sock.rQlen;
+                const befLen = sock._rQlen - sock._rQi;
                 const expected = (RQ_TEMPLATE[0] << 8) + RQ_TEMPLATE[1];
                 expect(sock.rQshift16()).to.equal(expected);
-                expect(sock.rQlen).to.equal(befLen - 2);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen - 2);
             });
         });
 
         describe('rQshift32()', function () {
             it('should pop four bytes from the receive queue and return a single number', function () {
-                const befLen = sock.rQlen;
+                const befLen = sock._rQlen - sock._rQi;
                 const expected = (RQ_TEMPLATE[0] << 24) +
                                (RQ_TEMPLATE[1] << 16) +
                                (RQ_TEMPLATE[2] << 8) +
                                RQ_TEMPLATE[3];
                 expect(sock.rQshift32()).to.equal(expected);
-                expect(sock.rQlen).to.equal(befLen - 4);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen - 4);
             });
         });
 
@@ -77,12 +64,12 @@ describe('Websock', function () {
                 const shifted = sock.rQshiftStr(3);
                 expect(shifted).to.be.a('string');
                 expect(shifted).to.equal(String.fromCharCode.apply(null, Array.prototype.slice.call(new Uint8Array(RQ_TEMPLATE.buffer, befRQi, 3))));
-                expect(sock.rQlen).to.equal(befLen - 3);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen - 3);
             });
 
             it('should shift the entire rest of the queue off if no length is given', function () {
                 sock.rQshiftStr();
-                expect(sock.rQlen).to.equal(0);
+                expect(sock._rQlen - sock._rQi).to.equal(0);
             });
 
             it('should be able to handle very large strings', function () {
@@ -106,7 +93,7 @@ describe('Websock', function () {
                 const shifted = sock.rQshiftStr();
 
                 expect(shifted).to.be.equal(expected);
-                expect(sock.rQlen).to.equal(0);
+                expect(sock._rQlen - sock._rQi).to.equal(0);
             });
         });
 
@@ -117,12 +104,12 @@ describe('Websock', function () {
                 const shifted = sock.rQshiftBytes(3);
                 expect(shifted).to.be.an.instanceof(Uint8Array);
                 expect(shifted).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, befRQi, 3));
-                expect(sock.rQlen).to.equal(befLen - 3);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen - 3);
             });
 
             it('should shift the entire rest of the queue off if no length is given', function () {
                 sock.rQshiftBytes();
-                expect(sock.rQlen).to.equal(0);
+                expect(sock._rQlen - sock._rQi).to.equal(0);
             });
         });
 
@@ -132,9 +119,9 @@ describe('Websock', function () {
             });
 
             it('should not modify the receive queue', function () {
-                const befLen = sock.rQlen;
+                const befLen = sock._rQlen - sock._rQi;
                 sock.rQpeekBytes(2);
-                expect(sock.rQlen).to.equal(befLen);
+                expect(sock._rQlen - sock._rQi).to.equal(befLen);
             });
 
             it('should return an array containing the requested bytes of the receive queue', function () {