From d0203a5995ef27c4f49c7d22da89438bdaee9a68 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 16 May 2023 19:06:10 +0200 Subject: [PATCH] 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. --- core/decoders/hextile.js | 2 +- core/decoders/jpeg.js | 4 ++-- core/decoders/raw.js | 2 +- core/decoders/tight.js | 2 +- core/decoders/zrle.js | 2 +- core/websock.js | 18 +++++++++++++----- tests/test.websock.js | 12 ++++++++++++ 7 files changed, 31 insertions(+), 11 deletions(-) diff --git a/core/decoders/hextile.js b/core/decoders/hextile.js index 6d96927f..cc33e0e1 100644 --- 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 index 5f5cc265..feb2aeb6 100644 --- 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 index 488ac1af..3c166142 100644 --- 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 index 5eaed545..45622080 100644 --- 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 index 97fbd58e..49128e79 100644 --- 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 index b6891329..7cd87091 100644 --- 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 index 2034bd6e..d501021a 100644 --- 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 () {