Always return copy of data from socket

We don't know how long the caller will hang on to this data, so we need to be safe by default and assume it will kept indefinitely. That means we can't return a reference to the internal buffer, as that will get overwritten with future messages. We want to avoid unnecessary copying in performance critical code, though. So allow code to explicitly ask for a shared buffer, assuming they know the data needs to be consumed immediately.
novnc · CendioOssman · Jun 30, 2023 · May 15, 2023 · May 11, 2023 · May 11, 2023
commit d0203a5995ef27c4f49c7d22da89438bdaee9a68
diff --git a/core/decoders/hextile.js b/core/decoders/hextile.js
@@ -86,7 +86,7 @@ export default class HextileDecoder {
                 }
             } else if (subencoding & 0x01) {  // Raw
                 let pixels = tw * th;
-                let data = sock.rQshiftBytes(pixels * 4);
+                let data = sock.rQshiftBytes(pixels * 4, false);
                 // Max sure the image is fully opaque
                 for (let i = 0;i <  pixels;i++) {
                     data[i * 4 + 3] = 255;

diff --git a/core/decoders/jpeg.js b/core/decoders/jpeg.js
@@ -124,7 +124,7 @@ export default class JPEGDecoder {
                 if (sock.rQwait("JPEG", length-2+extra, 4)) {
                     return null;
                 }
-                let data = sock.rQpeekBytes(length-2+extra);
+                let data = sock.rQpeekBytes(length-2+extra, false);
                 if (data.at(-2) === 0xFF && data.at(-1) !== 0x00 &&
                     !(data.at(-1) >= 0xD0 && data.at(-1) <= 0xD7)) {
                     extra -= 2;
@@ -139,7 +139,7 @@ export default class JPEGDecoder {
         segment[1] = type;
         segment[2] = length >> 8;
         segment[3] = length;
-        segment.set(sock.rQshiftBytes(length-2+extra), 4);
+        segment.set(sock.rQshiftBytes(length-2+extra, false), 4);
 
         return segment;
     }

diff --git a/core/decoders/raw.js b/core/decoders/raw.js
@@ -31,7 +31,7 @@ export default class RawDecoder {
 
             const curY = y + (height - this._lines);
 
-            let data = sock.rQshiftBytes(bytesPerLine);
+            let data = sock.rQshiftBytes(bytesPerLine, false);
 
             // Convert data if needed
             if (depth == 8) {

diff --git a/core/decoders/tight.js b/core/decoders/tight.js
@@ -312,7 +312,7 @@ export default class TightDecoder {
             return null;
         }
 
-        let data = sock.rQshiftBytes(this._len);
+        let data = sock.rQshiftBytes(this._len, false);
         this._len = 0;
 
         return data;

diff --git a/core/decoders/zrle.js b/core/decoders/zrle.js
@@ -32,7 +32,7 @@ export default class ZRLEDecoder {
             return false;
         }
 
-        const data = sock.rQshiftBytes(this._length);
+        const data = sock.rQshiftBytes(this._length, false);
 
         this._inflator.setInput(data);
 

diff --git a/core/websock.js b/core/websock.js
@@ -128,15 +128,19 @@ export default class Websock {
         let str = "";
         // Handle large arrays in steps to avoid long strings on the stack
         for (let i = 0; i < len; i += 4096) {
-            let part = this.rQshiftBytes(Math.min(4096, len - i));
+            let part = this.rQshiftBytes(Math.min(4096, len - i), false);
             str += String.fromCharCode.apply(null, part);
         }
         return str;
     }
 
-    rQshiftBytes(len) {
+    rQshiftBytes(len, copy=true) {
         this._rQi += len;
-        return new Uint8Array(this._rQ.buffer, this._rQi - len, len);
+        if (copy) {
+            return this._rQ.slice(this._rQi - len, this._rQi);
+        } else {
+            return this._rQ.subarray(this._rQi - len, this._rQi);
+        }
     }
 
     rQshiftTo(target, len) {
@@ -145,8 +149,12 @@ export default class Websock {
         this._rQi += len;
     }
 
-    rQpeekBytes(len) {
-        return new Uint8Array(this._rQ.buffer, this._rQi, len);
+    rQpeekBytes(len, copy=true) {
+        if (copy) {
+            return this._rQ.slice(this._rQi, this._rQi + len);
+        } else {
+            return this._rQ.subarray(this._rQi, this._rQi + len);
+        }
     }
 
     // Check to see if we must wait for 'num' bytes (default to FBU.bytes)

diff --git a/tests/test.websock.js b/tests/test.websock.js
@@ -101,6 +101,12 @@ describe('Websock', function () {
                 expect(shifted).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, befRQi, 3));
                 expect(sock._rQlen - sock._rQi).to.equal(befLen - 3);
             });
+            it('should return a shared array if requested', function () {
+                const befRQi = sock._rQi;
+                const shifted = sock.rQshiftBytes(3, false);
+                expect(shifted).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, befRQi, 3));
+                expect(shifted.buffer.byteLength).to.not.equal(shifted.length);
+            });
         });
 
         describe('rQpeekBytes', function () {
@@ -124,6 +130,12 @@ describe('Websock', function () {
                 sock._rQi = 1;
                 expect(sock.rQpeekBytes(2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2));
             });
+
+            it('should return a shared array if requested', function () {
+                const sl = sock.rQpeekBytes(2, false);
+                expect(sl).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 0, 2));
+                expect(sl.buffer.byteLength).to.not.equal(sl.length);
+            });
         });
 
         describe('rQwait', function () {