From 87143b361e78dda79b2f58b59d82d4696ba66163 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 15 May 2023 12:57:59 +0200 Subject: [PATCH 01/18] Reduce kept state in JPEG decoder We don't have to keep track of this much data between rects, so restructure things to make it more simple. This allows the JPEG parsing code to be a pure function which only depends on the input. --- core/decoders/jpeg.js | 231 +++++++++++++++++++++--------------------- 1 file changed, 118 insertions(+), 113 deletions(-) diff --git a/core/decoders/jpeg.js b/core/decoders/jpeg.js index e1f2bdf8..157bf4f3 100644 --- a/core/decoders/jpeg.js +++ b/core/decoders/jpeg.js @@ -11,131 +11,136 @@ export default class JPEGDecoder { constructor() { // RealVNC will reuse the quantization tables // and Huffman tables, so we need to cache them. - this._quantTables = []; - this._huffmanTables = []; this._cachedQuantTables = []; this._cachedHuffmanTables = []; - this._jpegLength = 0; this._segments = []; } decodeRect(x, y, width, height, sock, display, depth) { // A rect of JPEG encodings is simply a JPEG file - if (!this._parseJPEG(sock.rQslice(0))) { - return false; - } - const data = sock.rQshiftBytes(this._jpegLength); - if (this._quantTables.length != 0 && this._huffmanTables.length != 0) { - // If there are quantization tables and Huffman tables in the JPEG - // image, we can directly render it. - display.imageRect(x, y, width, height, "image/jpeg", data); - return true; - } else { - // Otherwise we need to insert cached tables. - const sofIndex = this._segments.findIndex( - x => x[1] == 0xC0 || x[1] == 0xC2 - ); - if (sofIndex == -1) { - throw new Error("Illegal JPEG image without SOF"); - } - let segments = this._segments.slice(0, sofIndex); - segments = segments.concat(this._quantTables.length ? - this._quantTables : - this._cachedQuantTables); - segments.push(this._segments[sofIndex]); - segments = segments.concat(this._huffmanTables.length ? - this._huffmanTables : - this._cachedHuffmanTables, - this._segments.slice(sofIndex + 1)); - let length = 0; - for (let i = 0; i < segments.length; i++) { - length += segments[i].length; - } - const data = new Uint8Array(length); - length = 0; - for (let i = 0; i < segments.length; i++) { - data.set(segments[i], length); - length += segments[i].length; - } - display.imageRect(x, y, width, height, "image/jpeg", data); - return true; - } - } - - _parseJPEG(buffer) { - if (this._quantTables.length != 0) { - this._cachedQuantTables = this._quantTables; - } - if (this._huffmanTables.length != 0) { - this._cachedHuffmanTables = this._huffmanTables; - } - this._quantTables = []; - this._huffmanTables = []; - this._segments = []; - let i = 0; - let bufferLength = buffer.length; while (true) { - let j = i; - if (j + 2 > bufferLength) { + let segment = this._readSegment(sock); + if (segment === null) { return false; } - if (buffer[j] != 0xFF) { - throw new Error("Illegal JPEG marker received (byte: " + - buffer[j] + ")"); - } - const type = buffer[j+1]; - j += 2; - if (type == 0xD9) { - this._jpegLength = j; - this._segments.push(buffer.slice(i, j)); - return true; - } else if (type == 0xDA) { - // start of scan - let hasFoundEndOfScan = false; - for (let k = j + 3; k + 1 < bufferLength; k++) { - if (buffer[k] == 0xFF && buffer[k+1] != 0x00 && - !(buffer[k+1] >= 0xD0 && buffer[k+1] <= 0xD7)) { - j = k; - hasFoundEndOfScan = true; - break; - } - } - if (!hasFoundEndOfScan) { - return false; - } - this._segments.push(buffer.slice(i, j)); - i = j; - continue; - } else if (type >= 0xD0 && type < 0xD9 || type == 0x01) { - // No length after marker - this._segments.push(buffer.slice(i, j)); - i = j; - continue; - } - if (j + 2 > bufferLength) { - return false; - } - const length = (buffer[j] << 8) + buffer[j+1] - 2; - if (length < 0) { - throw new Error("Illegal JPEG length received (length: " + - length + ")"); - } - j += 2; - if (j + length > bufferLength) { - return false; - } - j += length; - const segment = buffer.slice(i, j); - if (type == 0xC4) { - // Huffman tables - this._huffmanTables.push(segment); - } else if (type == 0xDB) { - // Quantization tables - this._quantTables.push(segment); - } this._segments.push(segment); - i = j; + // End of image? + if (segment[1] === 0xD9) { + break; + } } + + let huffmanTables = []; + let quantTables = []; + for (let segment of this._segments) { + let type = segment[1]; + if (type === 0xC4) { + // Huffman tables + huffmanTables.push(segment); + } else if (type === 0xDB) { + // Quantization tables + quantTables.push(segment); + } + } + + const sofIndex = this._segments.findIndex( + x => x[1] == 0xC0 || x[1] == 0xC2 + ); + if (sofIndex == -1) { + throw new Error("Illegal JPEG image without SOF"); + } + + if (quantTables.length === 0) { + this._segments.splice(sofIndex+1, 0, + ...this._cachedQuantTables); + } + if (huffmanTables.length === 0) { + this._segments.splice(sofIndex+1, 0, + ...this._cachedHuffmanTables); + } + + let length = 0; + for (let segment of this._segments) { + length += segment.length; + } + + let data = new Uint8Array(length); + length = 0; + for (let segment of this._segments) { + data.set(segment, length); + length += segment.length; + } + + display.imageRect(x, y, width, height, "image/jpeg", data); + + if (huffmanTables.length !== 0) { + this._cachedHuffmanTables = huffmanTables; + } + if (quantTables.length !== 0) { + this._cachedQuantTables = quantTables; + } + + this._segments = []; + + return true; + } + + _readSegment(sock) { + if (sock.rQwait("JPEG", 2)) { + return null; + } + + let marker = sock.rQshift8(); + if (marker != 0xFF) { + throw new Error("Illegal JPEG marker received (byte: " + + marker + ")"); + } + let type = sock.rQshift8(); + if (type >= 0xD0 && type <= 0xD9 || type == 0x01) { + // No length after marker + return new Uint8Array([marker, type]); + } + + if (sock.rQwait("JPEG", 2, 2)) { + return null; + } + + let length = sock.rQshift16(); + if (length < 2) { + throw new Error("Illegal JPEG length received (length: " + + length + ")"); + } + + if (sock.rQwait("JPEG", length-2, 4)) { + return null; + } + + let extra = 0; + if (type === 0xDA) { + // start of scan + extra += 2; + while (true) { + if (sock.rQwait("JPEG", length-2+extra, 4)) { + return null; + } + let data = sock.rQslice(0, length-2+extra); + if (data.at(-2) === 0xFF && data.at(-1) !== 0x00 && + !(data.at(-1) >= 0xD0 && data.at(-1) <= 0xD7)) { + extra -= 2; + break; + } + extra++; + } + } + + let segment = new Uint8Array(2 + length + extra); + segment[0] = marker; + segment[1] = type; + segment[2] = length >> 8; + segment[3] = length; + segment.set(sock.rQshiftBytes(length-2+extra), 4); + + return segment; } } From ae9b042df1c17bb702ca78cda2aa2ce1a30002f0 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 11 May 2023 12:33:22 +0200 Subject: [PATCH 02/18] Change rQslice() to rQpeekBytes() We don't need any full slice functionality, so let's change this to better march rQpeek8() and rQshiftBytes(). --- core/decoders/jpeg.js | 2 +- core/ra2.js | 2 +- core/rfb.js | 2 +- core/websock.js | 5 +++-- tests/test.websock.js | 16 ++++++++-------- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/core/decoders/jpeg.js b/core/decoders/jpeg.js index 157bf4f3..5f5cc265 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.rQslice(0, length-2+extra); + let data = sock.rQpeekBytes(length-2+extra); if (data.at(-2) === 0xFF && data.at(-1) !== 0x00 && !(data.at(-1) >= 0xD0 && data.at(-1) <= 0xD7)) { extra -= 2; diff --git a/core/ra2.js b/core/ra2.js index 9557054f..b2bfb50a 100644 --- a/core/ra2.js +++ b/core/ra2.js @@ -139,7 +139,7 @@ export default class RSAAESAuthenticationState extends EventTargetMixin { this._hasStarted = true; // 1: Receive server public key await this._waitSockAsync(4); - const serverKeyLengthBuffer = this._sock.rQslice(0, 4); + const serverKeyLengthBuffer = this._sock.rQpeekBytes(4); const serverKeyLength = this._sock.rQshift32(); if (serverKeyLength < 1024) { throw new Error("RA2: server public key is too short: " + serverKeyLength); diff --git a/core/rfb.js b/core/rfb.js index ea8dc0ff..eba2e1da 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -2455,7 +2455,7 @@ export default class RFB extends EventTargetMixin { default: this._fail("Unexpected server message (type " + msgType + ")"); - Log.Debug("sock.rQslice(0, 30): " + this._sock.rQslice(0, 30)); + Log.Debug("sock.rQpeekBytes(30): " + this._sock.rQpeekBytes(30)); return true; } } diff --git a/core/websock.js b/core/websock.js index e07e31ba..3813af1c 100644 --- a/core/websock.js +++ b/core/websock.js @@ -168,8 +168,9 @@ export default class Websock { this._rQi += len; } - rQslice(start, end = this.rQlen) { - return new Uint8Array(this._rQ.buffer, this._rQi + start, end - start); + rQpeekBytes(len) { + if (typeof(len) === 'undefined') { len = this.rQlen; } + return new Uint8Array(this._rQ.buffer, 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 1a0ba233..d2e20e0d 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -126,32 +126,32 @@ describe('Websock', function () { }); }); - describe('rQslice', function () { + describe('rQpeekBytes', function () { beforeEach(function () { sock.rQi = 0; }); it('should not modify the receive queue', function () { const befLen = sock.rQlen; - sock.rQslice(0, 2); + sock.rQpeekBytes(2); expect(sock.rQlen).to.equal(befLen); }); - it('should return an array containing the given slice of the receive queue', function () { - const sl = sock.rQslice(0, 2); + it('should return an array containing the requested bytes of the receive queue', function () { + const sl = sock.rQpeekBytes(2); expect(sl).to.be.an.instanceof(Uint8Array); expect(sl).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 0, 2)); }); it('should use the rest of the receive queue if no end is given', function () { - const sl = sock.rQslice(1); - expect(sl).to.have.length(RQ_TEMPLATE.length - 1); - expect(sl).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1)); + const sl = sock.rQpeekBytes(); + expect(sl).to.have.length(RQ_TEMPLATE.length); + expect(sl).to.array.equal(RQ_TEMPLATE); }); it('should take the current rQi in to account', function () { sock.rQi = 1; - expect(sock.rQslice(0, 2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2)); + expect(sock.rQpeekBytes(2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2)); }); }); From fb3c8f64e99674c48a1f317633d337b14e1e3471 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 11 May 2023 22:32:13 +0200 Subject: [PATCH 03/18] Switch Display.flush() to use a promise That is the modern way to handle operations that cannot complete immediately. --- core/display.js | 23 +++++++++++++---------- core/rfb.js | 18 ++++++++---------- docs/API-internal.md | 6 ------ tests/playback.js | 19 ++++++------------- tests/test.display.js | 18 +++++++----------- tests/test.jpeg.js | 18 ++++++------------ tests/test.tight.js | 9 +++------ tests/test.tightpng.js | 9 +++------ 8 files changed, 46 insertions(+), 74 deletions(-) diff --git a/core/display.js b/core/display.js index bf8d5fab..fcd62699 100644 --- a/core/display.js +++ b/core/display.js @@ -15,7 +15,7 @@ export default class Display { this._drawCtx = null; this._renderQ = []; // queue drawing actions for in-oder rendering - this._flushing = false; + this._flushPromise = null; // the full frame buffer (logical canvas) size this._fbWidth = 0; @@ -61,10 +61,6 @@ export default class Display { this._scale = 1.0; this._clipViewport = false; - - // ===== EVENT HANDLERS ===== - - this.onflush = () => {}; // A flush request has finished } // ===== PROPERTIES ===== @@ -306,9 +302,14 @@ export default class Display { flush() { if (this._renderQ.length === 0) { - this.onflush(); + return Promise.resolve(); } else { - this._flushing = true; + if (this._flushPromise === null) { + this._flushPromise = new Promise((resolve) => { + this._flushResolve = resolve; + }); + } + return this._flushPromise; } } @@ -517,9 +518,11 @@ export default class Display { } } - if (this._renderQ.length === 0 && this._flushing) { - this._flushing = false; - this.onflush(); + if (this._renderQ.length === 0 && + this._flushPromise !== null) { + this._flushResolve(); + this._flushPromise = null; + this._flushResolve = null; } } } diff --git a/core/rfb.js b/core/rfb.js index eba2e1da..c47f9ce4 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -257,7 +257,6 @@ export default class RFB extends EventTargetMixin { Log.Error("Display exception: " + exc); throw exc; } - this._display.onflush = this._onFlush.bind(this); this._keyboard = new Keyboard(this._canvas); this._keyboard.onkeyevent = this._handleKeyEvent.bind(this); @@ -2460,14 +2459,6 @@ export default class RFB extends EventTargetMixin { } } - _onFlush() { - this._flushing = false; - // Resume processing - if (this._sock.rQlen > 0) { - this._handleMessage(); - } - } - _framebufferUpdate() { if (this._FBU.rects === 0) { if (this._sock.rQwait("FBU header", 3, 1)) { return false; } @@ -2478,7 +2469,14 @@ export default class RFB extends EventTargetMixin { // to avoid building up an excessive queue if (this._display.pending()) { this._flushing = true; - this._display.flush(); + this._display.flush() + .then(() => { + this._flushing = false; + // Resume processing + if (this._sock.rQlen > 0) { + this._handleMessage(); + } + }); return false; } } diff --git a/docs/API-internal.md b/docs/API-internal.md index 2cac8ddf..c41e0f32 100644 --- a/docs/API-internal.md +++ b/docs/API-internal.md @@ -81,9 +81,3 @@ None | blitImage | (x, y, width, height, arr, offset, from_queue) | Blit pixels (of R,G,B,A) to the display | drawImage | (img, x, y) | Draw image and track damage | autoscale | (containerWidth, containerHeight) | Scale the display - -### 2.2.3 Callbacks - -| name | parameters | description -| ------- | ---------- | ------------ -| onflush | () | A display flush has been requested and we are now ready to resume FBU processing diff --git a/tests/playback.js b/tests/playback.js index 19ab2c34..955df0ee 100644 --- a/tests/playback.js +++ b/tests/playback.js @@ -131,12 +131,10 @@ export default class RecordingPlayer { _doPacket() { // Avoid having excessive queue buildup in non-realtime mode if (this._trafficManagement && this._rfb._flushing) { - const orig = this._rfb._display.onflush; - this._rfb._display.onflush = () => { - this._rfb._display.onflush = orig; - this._rfb._onFlush(); - this._doPacket(); - }; + this._rfb.flush() + .then(() => { + this._doPacket(); + }); return; } @@ -150,13 +148,8 @@ export default class RecordingPlayer { _finish() { if (this._rfb._display.pending()) { - this._rfb._display.onflush = () => { - if (this._rfb._flushing) { - this._rfb._onFlush(); - } - this._finish(); - }; - this._rfb._display.flush(); + this._rfb._display.flush() + .then(() => { this._finish(); }); } else { this._running = false; this._ws.onclose({code: 1000, reason: ""}); diff --git a/tests/test.display.js b/tests/test.display.js index 0604997c..e6c0406f 100644 --- a/tests/test.display.js +++ b/tests/test.display.js @@ -298,14 +298,11 @@ describe('Display/Canvas Helper', function () { expect(display).to.have.displayed(checkedData); }); - it('should support drawing images via #imageRect', function (done) { + it('should support drawing images via #imageRect', async function () { display.imageRect(0, 0, 4, 4, "image/png", makeImagePng(checkedData, 4, 4)); display.flip(); - display.onflush = () => { - expect(display).to.have.displayed(checkedData); - done(); - }; - display.flush(); + await display.flush(); + expect(display).to.have.displayed(checkedData); }); it('should support blit images with true color via #blitImage', function () { @@ -360,12 +357,11 @@ describe('Display/Canvas Helper', function () { expect(img.addEventListener).to.have.been.calledOnce; }); - it('should call callback when queue is flushed', function () { - display.onflush = sinon.spy(); + it('should resolve promise when queue is flushed', async function () { display.fillRect(0, 0, 4, 4, [0, 0xff, 0]); - expect(display.onflush).to.not.have.been.called; - display.flush(); - expect(display.onflush).to.have.been.calledOnce; + let promise = display.flush(); + expect(promise).to.be.an.instanceOf(Promise); + await promise; }); it('should draw a blit image on type "blit"', function () { diff --git a/tests/test.jpeg.js b/tests/test.jpeg.js index 5211cc7c..8dee4891 100644 --- a/tests/test.jpeg.js +++ b/tests/test.jpeg.js @@ -44,7 +44,7 @@ describe('JPEG Decoder', function () { display.resize(4, 4); }); - it('should handle JPEG rects', function (done) { + it('should handle JPEG rects', async function () { let data = [ // JPEG data 0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, @@ -151,14 +151,11 @@ describe('JPEG Decoder', function () { return diff < 5; } - display.onflush = () => { - expect(display).to.have.displayed(targetData, almost); - done(); - }; - display.flush(); + await display.flush(); + expect(display).to.have.displayed(targetData, almost); }); - it('should handle JPEG rects without Huffman and quantification tables', function (done) { + it('should handle JPEG rects without Huffman and quantification tables', async function () { let data1 = [ // JPEG data 0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, @@ -289,10 +286,7 @@ describe('JPEG Decoder', function () { return diff < 5; } - display.onflush = () => { - expect(display).to.have.displayed(targetData, almost); - done(); - }; - display.flush(); + await display.flush(); + expect(display).to.have.displayed(targetData, almost); }); }); diff --git a/tests/test.tight.js b/tests/test.tight.js index cc92c1a2..b3457a88 100644 --- a/tests/test.tight.js +++ b/tests/test.tight.js @@ -295,7 +295,7 @@ describe('Tight Decoder', function () { expect(display).to.have.displayed(targetData); }); - it('should handle JPEG rects', function (done) { + it('should handle JPEG rects', async function () { let data = [ // Control bytes 0x90, 0xd6, 0x05, @@ -410,10 +410,7 @@ describe('Tight Decoder', function () { return diff < 5; } - display.onflush = () => { - expect(display).to.have.displayed(targetData, almost); - done(); - }; - display.flush(); + await display.flush(); + expect(display).to.have.displayed(targetData, almost); }); }); diff --git a/tests/test.tightpng.js b/tests/test.tightpng.js index c72c20d7..02c66d93 100644 --- a/tests/test.tightpng.js +++ b/tests/test.tightpng.js @@ -44,7 +44,7 @@ describe('TightPng Decoder', function () { display.resize(4, 4); }); - it('should handle the TightPng encoding', function (done) { + it('should handle the TightPng encoding', async function () { let data = [ // Control bytes 0xa0, 0xb4, 0x04, @@ -139,10 +139,7 @@ describe('TightPng Decoder', function () { return diff < 30; } - display.onflush = () => { - expect(display).to.have.displayed(targetData, almost); - done(); - }; - display.flush(); + await display.flush(); + expect(display).to.have.displayed(targetData, almost); }); }); From 0180bc81c14bb3b9a23eae679c9fad075f53069c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 11 May 2023 23:06:34 +0200 Subject: [PATCH 04/18] Stop direct access to socket buffer Use proper accessor functions instead of poking around in internal buffers. --- core/decoders/hextile.js | 34 +++++++++++------------------ core/decoders/raw.js | 15 ++++++------- core/decoders/tight.js | 8 ++----- core/websock.js | 16 -------------- tests/test.websock.js | 46 ++++++++++++++++++++-------------------- 5 files changed, 43 insertions(+), 76 deletions(-) diff --git a/core/decoders/hextile.js b/core/decoders/hextile.js index ac21eff0..6d96927f 100644 --- a/core/decoders/hextile.js +++ b/core/decoders/hextile.js @@ -31,10 +31,7 @@ export default class HextileDecoder { return false; } - let rQ = sock.rQ; - let rQi = sock.rQi; - - let subencoding = rQ[rQi]; // Peek + let subencoding = sock.rQpeek8(); if (subencoding > 30) { // Raw throw new Error("Illegal hextile subencoding (subencoding: " + subencoding + ")"); @@ -65,7 +62,7 @@ export default class HextileDecoder { return false; } - let subrects = rQ[rQi + bytes - 1]; // Peek + let subrects = sock.rQpeekBytes(bytes).at(-1); if (subencoding & 0x10) { // SubrectsColoured bytes += subrects * (4 + 2); } else { @@ -79,7 +76,7 @@ export default class HextileDecoder { } // We know the encoding and have a whole tile - rQi++; + sock.rQshift8(); if (subencoding === 0) { if (this._lastsubencoding & 0x01) { // Weird: ignore blanks are RAW @@ -89,42 +86,36 @@ export default class HextileDecoder { } } else if (subencoding & 0x01) { // Raw let pixels = tw * th; + let data = sock.rQshiftBytes(pixels * 4); // Max sure the image is fully opaque for (let i = 0;i < pixels;i++) { - rQ[rQi + i * 4 + 3] = 255; + data[i * 4 + 3] = 255; } - display.blitImage(tx, ty, tw, th, rQ, rQi); - rQi += bytes - 1; + display.blitImage(tx, ty, tw, th, data, 0); } else { if (subencoding & 0x02) { // Background - this._background = [rQ[rQi], rQ[rQi + 1], rQ[rQi + 2], rQ[rQi + 3]]; - rQi += 4; + this._background = new Uint8Array(sock.rQshiftBytes(4)); } if (subencoding & 0x04) { // Foreground - this._foreground = [rQ[rQi], rQ[rQi + 1], rQ[rQi + 2], rQ[rQi + 3]]; - rQi += 4; + this._foreground = new Uint8Array(sock.rQshiftBytes(4)); } this._startTile(tx, ty, tw, th, this._background); if (subencoding & 0x08) { // AnySubrects - let subrects = rQ[rQi]; - rQi++; + let subrects = sock.rQshift8(); for (let s = 0; s < subrects; s++) { let color; if (subencoding & 0x10) { // SubrectsColoured - color = [rQ[rQi], rQ[rQi + 1], rQ[rQi + 2], rQ[rQi + 3]]; - rQi += 4; + color = sock.rQshiftBytes(4); } else { color = this._foreground; } - const xy = rQ[rQi]; - rQi++; + const xy = sock.rQshift8(); const sx = (xy >> 4); const sy = (xy & 0x0f); - const wh = rQ[rQi]; - rQi++; + const wh = sock.rQshift8(); const sw = (wh >> 4) + 1; const sh = (wh & 0x0f) + 1; @@ -133,7 +124,6 @@ export default class HextileDecoder { } this._finishTile(display); } - sock.rQi = rQi; this._lastsubencoding = subencoding; this._tiles--; } diff --git a/core/decoders/raw.js b/core/decoders/raw.js index d08f7ba9..c69ec7ac 100644 --- a/core/decoders/raw.js +++ b/core/decoders/raw.js @@ -33,29 +33,26 @@ export default class RawDecoder { Math.floor(sock.rQlen / bytesPerLine)); const pixels = width * currHeight; - let data = sock.rQ; - let index = sock.rQi; + 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[index + i] >> 0) & 0x3) * 255 / 3; - newdata[i * 4 + 1] = ((data[index + i] >> 2) & 0x3) * 255 / 3; - newdata[i * 4 + 2] = ((data[index + i] >> 4) & 0x3) * 255 / 3; + 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; - index = 0; } // Max sure the image is fully opaque for (let i = 0; i < pixels; i++) { - data[index + i * 4 + 3] = 255; + data[i * 4 + 3] = 255; } - display.blitImage(x, curY, width, currHeight, data, index); - sock.rQskipBytes(currHeight * bytesPerLine); + display.blitImage(x, curY, width, currHeight, data, 0); this._lines -= currHeight; if (this._lines > 0) { return false; diff --git a/core/decoders/tight.js b/core/decoders/tight.js index 7952707c..5eaed545 100644 --- a/core/decoders/tight.js +++ b/core/decoders/tight.js @@ -76,12 +76,8 @@ export default class TightDecoder { return false; } - const rQi = sock.rQi; - const rQ = sock.rQ; - - display.fillRect(x, y, width, height, - [rQ[rQi], rQ[rQi + 1], rQ[rQi + 2]], false); - sock.rQskipBytes(3); + let pixel = sock.rQshiftBytes(3); + display.fillRect(x, y, width, height, pixel, false); return true; } diff --git a/core/websock.js b/core/websock.js index 3813af1c..d5f03d28 100644 --- a/core/websock.js +++ b/core/websock.js @@ -94,22 +94,6 @@ export default class Websock { return "unknown"; } - get sQ() { - return this._sQ; - } - - get rQ() { - return this._rQ; - } - - get rQi() { - return this._rQi; - } - - set rQi(val) { - this._rQi = val; - } - // Receive Queue get rQlen() { return this._rQlen - this._rQi; diff --git a/tests/test.websock.js b/tests/test.websock.js index d2e20e0d..4b6fe230 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -19,13 +19,13 @@ describe('Websock', function () { }); describe('rQlen', function () { it('should return the length of the receive queue', function () { - sock.rQi = 0; + 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; + sock._rQi = 1; expect(sock.rQlen).to.equal(RQ_TEMPLATE.length - 1); }); @@ -72,8 +72,8 @@ describe('Websock', function () { describe('rQshiftStr', function () { it('should shift the given number of bytes off of the receive queue and return a string', function () { - const befLen = sock.rQlen; - const befRQi = sock.rQi; + const befLen = sock._rQlen; + const befRQi = sock._rQi; 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)))); @@ -112,8 +112,8 @@ describe('Websock', function () { describe('rQshiftBytes', function () { it('should shift the given number of bytes of the receive queue and return an array', function () { - const befLen = sock.rQlen; - const befRQi = sock.rQi; + const befLen = sock._rQlen; + const befRQi = sock._rQi; const shifted = sock.rQshiftBytes(3); expect(shifted).to.be.an.instanceof(Uint8Array); expect(shifted).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, befRQi, 3)); @@ -128,7 +128,7 @@ describe('Websock', function () { describe('rQpeekBytes', function () { beforeEach(function () { - sock.rQi = 0; + sock._rQi = 0; }); it('should not modify the receive queue', function () { @@ -150,14 +150,14 @@ describe('Websock', function () { }); it('should take the current rQi in to account', function () { - sock.rQi = 1; + sock._rQi = 1; expect(sock.rQpeekBytes(2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2)); }); }); describe('rQwait', function () { beforeEach(function () { - sock.rQi = 0; + sock._rQi = 0; }); it('should return true if there are not enough bytes in the receive queue', function () { @@ -169,20 +169,20 @@ describe('Websock', function () { }); it('should return true and reduce rQi by "goback" if there are not enough bytes', function () { - sock.rQi = 5; + sock._rQi = 5; expect(sock.rQwait('hi', RQ_TEMPLATE.length, 4)).to.be.true; - expect(sock.rQi).to.equal(1); + expect(sock._rQi).to.equal(1); }); it('should raise an error if we try to go back more than possible', function () { - sock.rQi = 5; + sock._rQi = 5; expect(() => sock.rQwait('hi', RQ_TEMPLATE.length, 6)).to.throw(Error); }); it('should not reduce rQi if there are enough bytes', function () { - sock.rQi = 5; + sock._rQi = 5; sock.rQwait('hi', 1, 6); - expect(sock.rQi).to.equal(5); + expect(sock._rQi).to.equal(5); }); }); }); @@ -461,52 +461,52 @@ describe('Websock', function () { }); it('should compact the receive queue when a message handler empties it', function () { - sock._eventHandlers.message = () => { sock.rQi = sock._rQlen; }; + sock._eventHandlers.message = () => { sock._rQi = sock._rQlen; }; sock._rQ = new Uint8Array([0, 1, 2, 3, 4, 5, 0, 0, 0, 0]); sock._rQlen = 6; - sock.rQi = 6; + sock._rQi = 6; const msg = { data: new Uint8Array([1, 2, 3]).buffer }; sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(0); - expect(sock.rQi).to.equal(0); + expect(sock._rQi).to.equal(0); }); it('should compact the receive queue when we reach the end of the buffer', function () { sock._rQ = new Uint8Array(20); sock._rQbufferSize = 20; sock._rQlen = 20; - sock.rQi = 10; + sock._rQi = 10; const msg = { data: new Uint8Array([1, 2]).buffer }; sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(12); - expect(sock.rQi).to.equal(0); + expect(sock._rQi).to.equal(0); }); it('should automatically resize the receive queue if the incoming message is larger than the buffer', function () { sock._rQ = new Uint8Array(20); sock._rQlen = 0; - sock.rQi = 0; + sock._rQi = 0; sock._rQbufferSize = 20; const msg = { data: new Uint8Array(30).buffer }; sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(30); - expect(sock.rQi).to.equal(0); + expect(sock._rQi).to.equal(0); expect(sock._rQ.length).to.equal(240); // keep the invariant that rQbufferSize / 8 >= rQlen }); it('should automatically resize the receive queue if the incoming message is larger than 1/8th of the buffer and we reach the end of the buffer', function () { sock._rQ = new Uint8Array(20); sock._rQlen = 16; - sock.rQi = 16; + sock._rQi = 16; sock._rQbufferSize = 20; const msg = { data: new Uint8Array(6).buffer }; sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(6); - expect(sock.rQi).to.equal(0); + expect(sock._rQi).to.equal(0); expect(sock._rQ.length).to.equal(48); }); }); From 55ffe8fc513b720a8bf84178d3c81af7a1ce314c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 14 May 2023 18:56:19 +0200 Subject: [PATCH 05/18] Stop exposing Websock queue length Callers should be using rQwait() to ensure sufficient data is present, and not poke around in the internal buffering. --- core/decoders/raw.js | 54 ++++++++++++++++++++----------------------- core/rfb.js | 6 ++--- core/websock.js | 18 ++++++--------- tests/test.websock.js | 43 ++++++++++++---------------------- 4 files changed, 50 insertions(+), 71 deletions(-) diff --git a/core/decoders/raw.js b/core/decoders/raw.js index c69ec7ac..488ac1af 100644 --- 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; - } - - 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; + while (this._lines > 0) { + if (sock.rQwait("RAW", bytesPerLine)) { + return false; } - data = newdata; - } - // Max sure the image is fully opaque - for (let i = 0; i < pixels; i++) { - data[i * 4 + 3] = 255; - } + const curY = y + (height - this._lines); - display.blitImage(x, curY, width, currHeight, data, 0); - this._lines -= currHeight; - if (this._lines > 0) { - return false; + let data = sock.rQshiftBytes(bytesPerLine); + + // 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; + } + + // Max sure the image is fully opaque + for (let i = 0; i < width; i++) { + data[i * 4 + 3] = 255; + } + + display.blitImage(x, curY, width, 1, data, 0); + this._lines--; } return true; diff --git a/core/rfb.js b/core/rfb.js index c47f9ce4..0f33057d 100644 --- 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 index d5f03d28..7b770d4b 100644 --- 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,20 +136,20 @@ 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); } @@ -161,7 +157,7 @@ export default class Websock { // 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 index 4b6fe230..91f69865 100644 --- 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 () { From e01dd27be49940e3a49e6cc90bd6ae7d0d1d7abd Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 14 May 2023 20:15:12 +0200 Subject: [PATCH 06/18] Remove Websock implicit read length Callers should be properly aware of how much data they need, as they need to call rQwait() first to ensure the data is present. --- core/websock.js | 4 ---- tests/test.websock.js | 18 +----------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/core/websock.js b/core/websock.js index 7b770d4b..b6891329 100644 --- a/core/websock.js +++ b/core/websock.js @@ -125,7 +125,6 @@ export default class Websock { } rQshiftStr(len) { - 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) { @@ -136,20 +135,17 @@ export default class Websock { } rQshiftBytes(len) { - 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 - 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 - this._rQi; } return new Uint8Array(this._rQ.buffer, this._rQi, len); } diff --git a/tests/test.websock.js b/tests/test.websock.js index 91f69865..2034bd6e 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -67,11 +67,6 @@ describe('Websock', function () { 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 - sock._rQi).to.equal(0); - }); - it('should be able to handle very large strings', function () { const BIG_LEN = 500000; const RQ_BIG = new Uint8Array(BIG_LEN); @@ -90,7 +85,7 @@ describe('Websock', function () { sock._rQ.set(RQ_BIG); sock._rQlen = RQ_BIG.length; - const shifted = sock.rQshiftStr(); + const shifted = sock.rQshiftStr(BIG_LEN); expect(shifted).to.be.equal(expected); expect(sock._rQlen - sock._rQi).to.equal(0); @@ -106,11 +101,6 @@ 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 shift the entire rest of the queue off if no length is given', function () { - sock.rQshiftBytes(); - expect(sock._rQlen - sock._rQi).to.equal(0); - }); }); describe('rQpeekBytes', function () { @@ -130,12 +120,6 @@ describe('Websock', function () { expect(sl).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 0, 2)); }); - it('should use the rest of the receive queue if no end is given', function () { - const sl = sock.rQpeekBytes(); - expect(sl).to.have.length(RQ_TEMPLATE.length); - expect(sl).to.array.equal(RQ_TEMPLATE); - }); - it('should take the current rQi in to account', function () { sock._rQi = 1; expect(sock.rQpeekBytes(2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2)); From aaa4eb8c3c358feb08c52991c331684ccd63b34c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 18:19:02 +0200 Subject: [PATCH 07/18] Use proper socket helpers for FBU header Let's not duplicate this stuff when we have convenience functions. --- core/rfb.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 0f33057d..8ac38aab 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -2486,13 +2486,11 @@ export default class RFB extends EventTargetMixin { if (this._sock.rQwait("rect header", 12)) { return false; } /* New FramebufferUpdate */ - const hdr = this._sock.rQshiftBytes(12); - this._FBU.x = (hdr[0] << 8) + hdr[1]; - this._FBU.y = (hdr[2] << 8) + hdr[3]; - this._FBU.width = (hdr[4] << 8) + hdr[5]; - this._FBU.height = (hdr[6] << 8) + hdr[7]; - this._FBU.encoding = parseInt((hdr[8] << 24) + (hdr[9] << 16) + - (hdr[10] << 8) + hdr[11], 10); + this._FBU.x = this._sock.rQshift16(); + this._FBU.y = this._sock.rQshift16(); + this._FBU.width = this._sock.rQshift16(); + this._FBU.height = this._sock.rQshift16(); + this._FBU.encoding = this._sock.rQshift32(); } if (!this._handleRect()) { From d0203a5995ef27c4f49c7d22da89438bdaee9a68 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 16 May 2023 19:06:10 +0200 Subject: [PATCH 08/18] 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 () { From 0ccc679d322947a05f2fc169da22d915930a75a6 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 14:50:35 +0200 Subject: [PATCH 09/18] Return unsigned values from rQshift32() This is what we almost always want, and this makes it consistent with rQshift8() and rQshift16(). --- core/rfb.js | 2 ++ core/websock.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/core/rfb.js b/core/rfb.js index 8ac38aab..b4045e52 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -2491,6 +2491,8 @@ export default class RFB extends EventTargetMixin { this._FBU.width = this._sock.rQshift16(); this._FBU.height = this._sock.rQshift16(); this._FBU.encoding = this._sock.rQshift32(); + /* Encodings are signed */ + this._FBU.encoding >>= 0; } if (!this._handleRect()) { diff --git a/core/websock.js b/core/websock.js index 7cd87091..2b9b519a 100644 --- a/core/websock.js +++ b/core/websock.js @@ -121,7 +121,7 @@ export default class Websock { for (let byte = bytes - 1; byte >= 0; byte--) { res += this._rQ[this._rQi++] << (byte * 8); } - return res; + return res >>> 0; } rQshiftStr(len) { From 12d2e7832d15d10e6c105c709b86d4adc040548d Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 14:54:40 +0200 Subject: [PATCH 10/18] Properly decode ExtendedDesktopSize fields We are expected to preserve these and use them in our requests back to the server. We can't do that if we don't actually decode them correctly. --- core/rfb.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index b4045e52..d89f029a 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -2740,12 +2740,12 @@ export default class RFB extends EventTargetMixin { for (let i = 0; i < numberOfScreens; i += 1) { // Save the id and flags of the first screen if (i === 0) { - this._screenID = this._sock.rQshiftBytes(4); // id - this._sock.rQskipBytes(2); // x-position - this._sock.rQskipBytes(2); // y-position - this._sock.rQskipBytes(2); // width - this._sock.rQskipBytes(2); // height - this._screenFlags = this._sock.rQshiftBytes(4); // flags + this._screenID = this._sock.rQshift32(); // id + this._sock.rQskipBytes(2); // x-position + this._sock.rQskipBytes(2); // y-position + this._sock.rQskipBytes(2); // width + this._sock.rQskipBytes(2); // height + this._screenFlags = this._sock.rQshift32(); // flags } else { this._sock.rQskipBytes(16); } From 45cedea78fc5f05b32c8514132349710707081b7 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 14:55:35 +0200 Subject: [PATCH 11/18] Don't send SetDesktopSize too early We don't know the server layout yet, so we can't preserve the screen id or flags yet at this point. Move it until after we've parsed everything. --- core/rfb.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index d89f029a..6857ed71 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -2726,14 +2726,6 @@ export default class RFB extends EventTargetMixin { const firstUpdate = !this._supportsSetDesktopSize; this._supportsSetDesktopSize = true; - // Normally we only apply the current resize mode after a - // window resize event. However there is no such trigger on the - // initial connect. And we don't know if the server supports - // resizing until we've gotten here. - if (firstUpdate) { - this._requestRemoteResize(); - } - this._sock.rQskipBytes(1); // number-of-screens this._sock.rQskipBytes(3); // padding @@ -2783,6 +2775,14 @@ export default class RFB extends EventTargetMixin { this._resize(this._FBU.width, this._FBU.height); } + // Normally we only apply the current resize mode after a + // window resize event. However there is no such trigger on the + // initial connect. And we don't know if the server supports + // resizing until we've gotten here. + if (firstUpdate) { + this._requestRemoteResize(); + } + return true; } From 2a7db6f647954ebb0dc2d5734b2cb7d44f323db6 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 14:56:48 +0200 Subject: [PATCH 12/18] Make ExtendedDesktopSize tests more realistic Send real messages and avoid poking around in internals, as we weren't testing things correctly that way. --- tests/test.rfb.js | 60 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/tests/test.rfb.js b/tests/test.rfb.js index 01169cad..bf12a460 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -814,10 +814,32 @@ describe('Remote Frame Buffer Protocol Client', function () { let client; beforeEach(function () { client = makeRFB(); - client._supportsSetDesktopSize = true; client.resizeSession = true; container.style.width = '70px'; container.style.height = '80px'; + + const incoming = [ 0x00, // msg-type=FBU + 0x00, // padding + 0x00, 0x01, // number of rects = 1 + 0x00, 0x00, // reason = server initialized + 0x00, 0x00, // status = no error + 0x00, 0x04, // new width = 4 + 0x00, 0x04, // new height = 4 + 0xff, 0xff, + 0xfe, 0xcc, // enc = (-308) ExtendedDesktopSize + 0x01, // number of screens = 1 + 0x00, 0x00, + 0x00, // padding + 0x78, 0x90, + 0xab, 0xcd, // screen id = 0 + 0x00, 0x00, // screen x = 0 + 0x00, 0x00, // screen y = 0 + 0x00, 0x04, // screen width = 4 + 0x00, 0x04, // screen height = 4 + 0x12, 0x34, + 0x56, 0x78]; // screen flags + client._sock._websocket._receiveData(new Uint8Array(incoming)); + sinon.spy(RFB.messages, "setDesktopSize"); }); @@ -833,6 +855,13 @@ describe('Remote Frame Buffer Protocol Client', function () { }); it('should request a resize when initially connecting', function () { + // Create a new object that hasn't yet seen a + // ExtendedDesktopSize rect + client = makeRFB(); + client.resizeSession = true; + container.style.width = '70px'; + container.style.height = '80px'; + // Simple ExtendedDesktopSize FBU message const incoming = [ 0x00, // msg-type=FBU 0x00, // padding @@ -846,17 +875,14 @@ describe('Remote Frame Buffer Protocol Client', function () { 0x01, // number of screens = 1 0x00, 0x00, 0x00, // padding - 0x00, 0x00, - 0x00, 0x00, // screen id = 0 + 0x78, 0x90, + 0xab, 0xcd, // screen id = 0 0x00, 0x00, // screen x = 0 0x00, 0x00, // screen y = 0 0x00, 0x04, // screen width = 4 0x00, 0x04, // screen height = 4 - 0x00, 0x00, - 0x00, 0x00]; // screen flags - - // This property is indirectly used as a marker for the first update - client._supportsSetDesktopSize = false; + 0x12, 0x34, + 0x56, 0x78]; // screen flags // First message should trigger a resize @@ -866,7 +892,7 @@ describe('Remote Frame Buffer Protocol Client', function () { // not the reported size from the server expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; expect(RFB.messages.setDesktopSize).to.have.been.calledWith( - sinon.match.object, 70, 80, 0, 0); + sinon.match.object, 70, 80, 0x7890abcd, 0x12345678); RFB.messages.setDesktopSize.resetHistory(); @@ -884,7 +910,8 @@ describe('Remote Frame Buffer Protocol Client', function () { clock.tick(1000); expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; - expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 40, 50, 0, 0); + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 40, 50, 0x7890abcd, 0x12345678); }); it('should not request the same size twice', function () { @@ -895,7 +922,7 @@ describe('Remote Frame Buffer Protocol Client', function () { expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; expect(RFB.messages.setDesktopSize).to.have.been.calledWith( - sinon.match.object, 40, 50, 0, 0); + sinon.match.object, 40, 50, 0x7890abcd, 0x12345678); // Server responds with the requested size 40x50 const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, @@ -934,7 +961,8 @@ describe('Remote Frame Buffer Protocol Client', function () { clock.tick(200); expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; - expect(RFB.messages.setDesktopSize).to.have.been.calledWith(sinon.match.object, 40, 50, 0, 0); + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 40, 50, 0x7890abcd, 0x12345678); }); it('should not resize when resize is disabled', function () { @@ -974,9 +1002,9 @@ describe('Remote Frame Buffer Protocol Client', function () { // Simple ExtendedDesktopSize FBU message, new size: 100x100 const incoming = [ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0x00, 0x64, 0xff, 0xff, 0xfe, 0xcc, - 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0xab, 0xab, 0xab, 0xab, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x04, - 0x00, 0x00, 0x00, 0x00 ]; + 0x11, 0x22, 0x33, 0x44 ]; // Note that this will cause the browser to display scrollbars // since the framebuffer is 100x100 and the container is 70x80. @@ -996,8 +1024,8 @@ describe('Remote Frame Buffer Protocol Client', function () { clock.tick(1000); expect(RFB.messages.setDesktopSize).to.have.been.calledOnce; - expect(RFB.messages.setDesktopSize.firstCall.args[1]).to.equal(120); - expect(RFB.messages.setDesktopSize.firstCall.args[2]).to.equal(130); + expect(RFB.messages.setDesktopSize).to.have.been.calledWith( + sinon.match.object, 120, 130, 0xabababab, 0x11223344); }); }); From b298bf9e901ea4264a83abd498415da49388709e Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 22:31:27 +0200 Subject: [PATCH 13/18] Don't split large WebSocket data in tests It takes too much time and can make the tests fail. --- tests/fake.websocket.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/fake.websocket.js b/tests/fake.websocket.js index a929a71f..d273fe07 100644 --- a/tests/fake.websocket.js +++ b/tests/fake.websocket.js @@ -55,11 +55,15 @@ export default class FakeWebSocket { } _receiveData(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.slice(i, i+1); - this.onmessage(new MessageEvent("message", { 'data': buf.buffer })); + if (data.length < 4096) { + // 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.slice(i, i+1); + this.onmessage(new MessageEvent("message", { 'data': buf.buffer })); + } + } else { + this.onmessage(new MessageEvent("message", { 'data': data.buffer })); } } } From b146de6d69b65588a74a7142bd332bf3e08f9202 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 4 Jun 2023 14:52:36 +0200 Subject: [PATCH 14/18] Avoid internal variables in recv queue tests Makes for more robust and realistic tests. --- tests/test.websock.js | 133 +++++++++++++++++------------------------- 1 file changed, 55 insertions(+), 78 deletions(-) diff --git a/tests/test.websock.js b/tests/test.websock.js index d501021a..e72ba272 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -7,73 +7,63 @@ describe('Websock', function () { "use strict"; describe('Receive queue methods', function () { - let sock; - const RQ_TEMPLATE = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]); + let sock, websock; beforeEach(function () { sock = new Websock(); - // skip init - sock._allocateBuffers(); - sock._rQ.set(RQ_TEMPLATE); - sock._rQlen = RQ_TEMPLATE.length; + websock = new FakeWebSocket(); + websock._open(); + sock.attach(websock); }); describe('rQpeek8', function () { it('should peek at the next byte without poping it off the queue', function () { - const befLen = sock._rQlen - sock._rQi; - const peek = sock.rQpeek8(); - expect(sock.rQpeek8()).to.equal(peek); - expect(sock._rQlen - sock._rQi).to.equal(befLen); + websock._receiveData(new Uint8Array([0xab, 0xcd])); + expect(sock.rQpeek8()).to.equal(0xab); + expect(sock.rQpeek8()).to.equal(0xab); }); }); describe('rQshift8()', function () { it('should pop a single byte from the receive queue', function () { - const peek = sock.rQpeek8(); - const befLen = sock._rQlen - sock._rQi; - expect(sock.rQshift8()).to.equal(peek); - expect(sock._rQlen - sock._rQi).to.equal(befLen - 1); + websock._receiveData(new Uint8Array([0xab, 0xcd])); + expect(sock.rQshift8()).to.equal(0xab); + expect(sock.rQshift8()).to.equal(0xcd); }); }); describe('rQshift16()', function () { it('should pop two bytes from the receive queue and return a single number', function () { - const befLen = sock._rQlen - sock._rQi; - const expected = (RQ_TEMPLATE[0] << 8) + RQ_TEMPLATE[1]; - expect(sock.rQshift16()).to.equal(expected); - expect(sock._rQlen - sock._rQi).to.equal(befLen - 2); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); + expect(sock.rQshift16()).to.equal(0xabcd); + expect(sock.rQshift16()).to.equal(0x1234); }); }); describe('rQshift32()', function () { it('should pop four bytes from the receive queue and return a single number', function () { - 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 - sock._rQi).to.equal(befLen - 4); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + expect(sock.rQshift32()).to.equal(0xabcd1234); + expect(sock.rQshift32()).to.equal(0x88ee1133); }); }); describe('rQshiftStr', function () { it('should shift the given number of bytes off of the receive queue and return a string', function () { - const befLen = sock._rQlen; - const befRQi = sock._rQi; - 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 - sock._rQi).to.equal(befLen - 3); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + expect(sock.rQshiftStr(4)).to.equal('\xab\xcd\x12\x34'); + expect(sock.rQshiftStr(4)).to.equal('\x88\xee\x11\x33'); }); it('should be able to handle very large strings', function () { const BIG_LEN = 500000; - const RQ_BIG = new Uint8Array(BIG_LEN); + const incoming = new Uint8Array(BIG_LEN); let expected = ""; let letterCode = 'a'.charCodeAt(0); for (let i = 0; i < BIG_LEN; i++) { - RQ_BIG[i] = letterCode; + incoming[i] = letterCode; expected += String.fromCharCode(letterCode); if (letterCode < 'z'.charCodeAt(0)) { @@ -82,90 +72,77 @@ describe('Websock', function () { letterCode = 'a'.charCodeAt(0); } } - sock._rQ.set(RQ_BIG); - sock._rQlen = RQ_BIG.length; + websock._receiveData(incoming); const shifted = sock.rQshiftStr(BIG_LEN); expect(shifted).to.be.equal(expected); - expect(sock._rQlen - sock._rQi).to.equal(0); }); }); describe('rQshiftBytes', function () { it('should shift the given number of bytes of the receive queue and return an array', function () { - const befLen = sock._rQlen; - const befRQi = sock._rQi; - 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 - sock._rQi).to.equal(befLen - 3); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + expect(sock.rQshiftBytes(4)).to.array.equal(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); + expect(sock.rQshiftBytes(4)).to.array.equal(new Uint8Array([0x88, 0xee, 0x11, 0x33])); }); + 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); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + const bytes = sock.rQshiftBytes(4, false); + expect(bytes).to.array.equal(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); + expect(bytes.buffer.byteLength).to.not.equal(bytes.length); }); }); describe('rQpeekBytes', function () { - beforeEach(function () { - sock._rQi = 0; - }); - it('should not modify the receive queue', function () { - const befLen = sock._rQlen - sock._rQi; - sock.rQpeekBytes(2); - expect(sock._rQlen - sock._rQi).to.equal(befLen); - }); - - it('should return an array containing the requested bytes of the receive queue', function () { - const sl = sock.rQpeekBytes(2); - expect(sl).to.be.an.instanceof(Uint8Array); - expect(sl).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 0, 2)); - }); - - it('should take the current rQi in to account', function () { - sock._rQi = 1; - expect(sock.rQpeekBytes(2)).to.array.equal(new Uint8Array(RQ_TEMPLATE.buffer, 1, 2)); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + expect(sock.rQpeekBytes(4)).to.array.equal(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); + expect(sock.rQpeekBytes(4)).to.array.equal(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); }); 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); + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); + const bytes = sock.rQpeekBytes(4, false); + expect(bytes).to.array.equal(new Uint8Array([0xab, 0xcd, 0x12, 0x34])); + expect(bytes.buffer.byteLength).to.not.equal(bytes.length); }); }); describe('rQwait', function () { beforeEach(function () { - sock._rQi = 0; + websock._receiveData(new Uint8Array([0xab, 0xcd, 0x12, 0x34, + 0x88, 0xee, 0x11, 0x33])); }); it('should return true if there are not enough bytes in the receive queue', function () { - expect(sock.rQwait('hi', RQ_TEMPLATE.length + 1)).to.be.true; + expect(sock.rQwait('hi', 9)).to.be.true; }); it('should return false if there are enough bytes in the receive queue', function () { - expect(sock.rQwait('hi', RQ_TEMPLATE.length)).to.be.false; + expect(sock.rQwait('hi', 8)).to.be.false; }); it('should return true and reduce rQi by "goback" if there are not enough bytes', function () { - sock._rQi = 5; - expect(sock.rQwait('hi', RQ_TEMPLATE.length, 4)).to.be.true; - expect(sock._rQi).to.equal(1); + expect(sock.rQshift32()).to.equal(0xabcd1234); + expect(sock.rQwait('hi', 8, 2)).to.be.true; + expect(sock.rQshift32()).to.equal(0x123488ee); }); it('should raise an error if we try to go back more than possible', function () { - sock._rQi = 5; - expect(() => sock.rQwait('hi', RQ_TEMPLATE.length, 6)).to.throw(Error); + expect(sock.rQshift32()).to.equal(0xabcd1234); + expect(() => sock.rQwait('hi', 8, 6)).to.throw(Error); }); it('should not reduce rQi if there are enough bytes', function () { - sock._rQi = 5; - sock.rQwait('hi', 1, 6); - expect(sock._rQi).to.equal(5); + expect(sock.rQshift32()).to.equal(0xabcd1234); + expect(sock.rQwait('hi', 4, 2)).to.be.false; + expect(sock.rQshift32()).to.equal(0x88ee1133); }); }); }); From 3fc0cb0cb70ae1cc6e07976e630660f36c020085 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 15 May 2023 13:33:11 +0200 Subject: [PATCH 15/18] Remove Base64 WebSocket remnants There is no encoding/decoding in modern WebSockets, so let's clean up some of the old crud that no longer serves a purpose. --- core/websock.js | 14 +++----------- tests/test.websock.js | 36 ++---------------------------------- 2 files changed, 5 insertions(+), 45 deletions(-) diff --git a/core/websock.js b/core/websock.js index 2b9b519a..711ea02b 100644 --- a/core/websock.js +++ b/core/websock.js @@ -177,7 +177,7 @@ export default class Websock { flush() { if (this._sQlen > 0 && this.readyState === 'open') { - this._websocket.send(this._encodeMessage()); + this._websocket.send(new Uint8Array(this._sQ.buffer, 0, this._sQlen)); this._sQlen = 0; } } @@ -268,11 +268,6 @@ export default class Websock { } // private methods - _encodeMessage() { - // Put in a binary arraybuffer - // according to the spec, you can send ArrayBufferViews with the send method - return new Uint8Array(this._sQ.buffer, 0, this._sQlen); - } // We want to move all the unread data to the start of the queue, // e.g. compacting. @@ -312,17 +307,14 @@ export default class Websock { } // push arraybuffer values onto the end of the receive que - _DecodeMessage(data) { - const u8 = new Uint8Array(data); + _recvMessage(e) { + const u8 = new Uint8Array(e.data); if (u8.length > this._rQbufferSize - this._rQlen) { this._expandCompactRQ(u8.length); } this._rQ.set(u8, this._rQlen); this._rQlen += u8.length; - } - _recvMessage(e) { - this._DecodeMessage(e.data); if (this._rQlen - this._rQi > 0) { this._eventHandlers.message(); if (this._rQlen == this._rQi) { diff --git a/tests/test.websock.js b/tests/test.websock.js index e72ba272..de7fe0d1 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -161,10 +161,9 @@ describe('Websock', function () { it('should actually send on the websocket', function () { sock._sQ = new Uint8Array([1, 2, 3]); sock._sQlen = 3; - const encoded = sock._encodeMessage(); sock.flush(); - expect(sock).to.have.sent(encoded); + expect(sock).to.have.sent(new Uint8Array([1, 2, 3])); }); it('should not call send if we do not have anything queued up', function () { @@ -397,9 +396,8 @@ describe('Websock', function () { sock._allocateBuffers(); }); - it('should support adding binary Uint8Array data to the receive queue', function () { + it('should support adding data to the receive queue', function () { const msg = { data: new Uint8Array([1, 2, 3]) }; - sock._mode = 'binary'; sock._recvMessage(msg); expect(sock.rQshiftStr(3)).to.equal('\x01\x02\x03'); }); @@ -426,7 +424,6 @@ describe('Websock', function () { sock._rQlen = 6; sock._rQi = 6; const msg = { data: new Uint8Array([1, 2, 3]).buffer }; - sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(0); expect(sock._rQi).to.equal(0); @@ -438,7 +435,6 @@ describe('Websock', function () { sock._rQlen = 20; sock._rQi = 10; const msg = { data: new Uint8Array([1, 2]).buffer }; - sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(12); expect(sock._rQi).to.equal(0); @@ -450,7 +446,6 @@ describe('Websock', function () { sock._rQi = 0; sock._rQbufferSize = 20; const msg = { data: new Uint8Array(30).buffer }; - sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(30); expect(sock._rQi).to.equal(0); @@ -463,37 +458,10 @@ describe('Websock', function () { sock._rQi = 16; sock._rQbufferSize = 20; const msg = { data: new Uint8Array(6).buffer }; - sock._mode = 'binary'; sock._recvMessage(msg); expect(sock._rQlen).to.equal(6); expect(sock._rQi).to.equal(0); expect(sock._rQ.length).to.equal(48); }); }); - - describe('Data encoding', function () { - before(function () { FakeWebSocket.replace(); }); - after(function () { FakeWebSocket.restore(); }); - - describe('as binary data', function () { - let sock; - beforeEach(function () { - sock = new Websock(); - sock.open('ws://', 'binary'); - sock._websocket._open(); - }); - - it('should only send the send queue up to the send queue length', function () { - sock._sQ = new Uint8Array([1, 2, 3, 4, 5]); - sock._sQlen = 3; - const res = sock._encodeMessage(); - expect(res).to.array.equal(new Uint8Array([1, 2, 3])); - }); - - it('should properly pass the encoded data off to the actual WebSocket', function () { - sock.send([1, 2, 3]); - expect(sock._websocket._getSentData()).to.array.equal(new Uint8Array([1, 2, 3])); - }); - }); - }); }); From 7356d4e60b689fc098b55f02ae3d7688e166194f Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 21 May 2023 20:16:59 +0200 Subject: [PATCH 16/18] Move WebSocket queue index reset to receive It's more robust to do this just before we need the space, rather than assume when the queue will be read and adjust things right after. --- core/websock.js | 12 ++++++------ tests/test.websock.js | 11 +++++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/core/websock.js b/core/websock.js index 711ea02b..75be044a 100644 --- a/core/websock.js +++ b/core/websock.js @@ -308,6 +308,12 @@ export default class Websock { // push arraybuffer values onto the end of the receive que _recvMessage(e) { + if (this._rQlen == this._rQi) { + // All data has now been processed, this means we + // can reset the receive queue. + this._rQlen = 0; + this._rQi = 0; + } const u8 = new Uint8Array(e.data); if (u8.length > this._rQbufferSize - this._rQlen) { this._expandCompactRQ(u8.length); @@ -317,12 +323,6 @@ export default class Websock { if (this._rQlen - this._rQi > 0) { this._eventHandlers.message(); - if (this._rQlen == this._rQi) { - // All data has now been processed, this means we - // can reset the receive queue. - this._rQlen = 0; - this._rQi = 0; - } } else { Log.Debug("Ignoring empty message"); } diff --git a/tests/test.websock.js b/tests/test.websock.js index de7fe0d1..f3cc4224 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -418,14 +418,13 @@ describe('Websock', function () { expect(sock._eventHandlers.message).not.to.have.been.called; }); - it('should compact the receive queue when a message handler empties it', function () { - sock._eventHandlers.message = () => { sock._rQi = sock._rQlen; }; + it('should compact the receive queue when fully read', function () { sock._rQ = new Uint8Array([0, 1, 2, 3, 4, 5, 0, 0, 0, 0]); sock._rQlen = 6; sock._rQi = 6; const msg = { data: new Uint8Array([1, 2, 3]).buffer }; sock._recvMessage(msg); - expect(sock._rQlen).to.equal(0); + expect(sock._rQlen).to.equal(3); expect(sock._rQi).to.equal(0); }); @@ -455,13 +454,13 @@ describe('Websock', function () { it('should automatically resize the receive queue if the incoming message is larger than 1/8th of the buffer and we reach the end of the buffer', function () { sock._rQ = new Uint8Array(20); sock._rQlen = 16; - sock._rQi = 16; + sock._rQi = 15; sock._rQbufferSize = 20; const msg = { data: new Uint8Array(6).buffer }; sock._recvMessage(msg); - expect(sock._rQlen).to.equal(6); + expect(sock._rQlen).to.equal(7); expect(sock._rQi).to.equal(0); - expect(sock._rQ.length).to.equal(48); + expect(sock._rQ.length).to.equal(56); }); }); }); From f8b65f9fe140ef194e166ca4867d2ae49e8daab3 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 28 May 2023 16:40:09 +0200 Subject: [PATCH 17/18] Add Websock send queue helpers Callers shouldn't be poking around directly in to the send queue, but should use accessor functions like for the read queue. --- core/ra2.js | 12 +- core/rfb.js | 342 +++++++++++++++--------------------------- core/websock.js | 36 +++-- tests/test.websock.js | 74 +++++++-- 4 files changed, 216 insertions(+), 248 deletions(-) diff --git a/core/ra2.js b/core/ra2.js index b2bfb50a..d330b848 100644 --- a/core/ra2.js +++ b/core/ra2.js @@ -182,7 +182,8 @@ export default class RSAAESAuthenticationState extends EventTargetMixin { clientPublicKey[3] = clientKeyLength & 0xff; clientPublicKey.set(clientN, 4); clientPublicKey.set(clientE, 4 + clientKeyBytes); - this._sock.send(clientPublicKey); + this._sock.sQpushBytes(clientPublicKey); + this._sock.flush(); // 3: Send client random const clientRandom = new Uint8Array(16); @@ -193,7 +194,8 @@ export default class RSAAESAuthenticationState extends EventTargetMixin { clientRandomMessage[0] = (serverKeyBytes & 0xff00) >>> 8; clientRandomMessage[1] = serverKeyBytes & 0xff; clientRandomMessage.set(clientEncryptedRandom, 2); - this._sock.send(clientRandomMessage); + this._sock.sQpushBytes(clientRandomMessage); + this._sock.flush(); // 4: Receive server random await this._waitSockAsync(2); @@ -234,7 +236,8 @@ export default class RSAAESAuthenticationState extends EventTargetMixin { clientHash = await window.crypto.subtle.digest("SHA-1", clientHash); serverHash = new Uint8Array(serverHash); clientHash = new Uint8Array(clientHash); - this._sock.send(await clientCipher.makeMessage(clientHash)); + this._sock.sQpushBytes(await clientCipher.makeMessage(clientHash)); + this._sock.flush(); await this._waitSockAsync(2 + 20 + 16); if (this._sock.rQshift16() !== 20) { throw new Error("RA2: wrong server hash"); @@ -295,7 +298,8 @@ export default class RSAAESAuthenticationState extends EventTargetMixin { for (let i = 0; i < password.length; i++) { credentials[username.length + 2 + i] = password.charCodeAt(i); } - this._sock.send(await clientCipher.makeMessage(credentials)); + this._sock.sQpushBytes(await clientCipher.makeMessage(credentials)); + this._sock.flush(); } get hasStarted() { diff --git a/core/rfb.js b/core/rfb.js index 6857ed71..da95a386 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -1381,7 +1381,8 @@ export default class RFB extends EventTargetMixin { while (repeaterID.length < 250) { repeaterID += "\0"; } - this._sock.sendString(repeaterID); + this._sock.sQpushString(repeaterID); + this._sock.flush(); return true; } @@ -1391,7 +1392,8 @@ export default class RFB extends EventTargetMixin { const cversion = "00" + parseInt(this._rfbVersion, 10) + ".00" + ((this._rfbVersion * 10) % 10); - this._sock.sendString("RFB " + cversion + "\n"); + this._sock.sQpushString("RFB " + cversion + "\n"); + this._sock.flush(); Log.Debug('Sent ProtocolVersion: ' + cversion); this._rfbInitState = 'Security'; @@ -1443,7 +1445,8 @@ export default class RFB extends EventTargetMixin { return this._fail("Unsupported security types (types: " + types + ")"); } - this._sock.send([this._rfbAuthScheme]); + this._sock.sQpush8(this._rfbAuthScheme); + this._sock.flush(); } else { // Server decides if (this._sock.rQwait("security scheme", 4)) { return false; } @@ -1505,12 +1508,15 @@ export default class RFB extends EventTargetMixin { return false; } - const xvpAuthStr = String.fromCharCode(this._rfbCredentials.username.length) + - String.fromCharCode(this._rfbCredentials.target.length) + - this._rfbCredentials.username + - this._rfbCredentials.target; - this._sock.sendString(xvpAuthStr); + this._sock.sQpush8(this._rfbCredentials.username.length); + this._sock.sQpush8(this._rfbCredentials.target.length); + this._sock.sQpushString(this._rfbCredentials.username); + this._sock.sQpushString(this._rfbCredentials.target); + + this._sock.flush(); + this._rfbAuthScheme = securityTypeVNCAuth; + return this._negotiateAuthentication(); } @@ -1528,7 +1534,9 @@ export default class RFB extends EventTargetMixin { return this._fail("Unsupported VeNCrypt version " + major + "." + minor); } - this._sock.send([0, 2]); + this._sock.sQpush8(0); + this._sock.sQpush8(2); + this._sock.flush(); this._rfbVeNCryptState = 1; } @@ -1587,10 +1595,8 @@ export default class RFB extends EventTargetMixin { return this._fail("Unsupported security types (types: " + subtypes + ")"); } - this._sock.send([this._rfbAuthScheme >> 24, - this._rfbAuthScheme >> 16, - this._rfbAuthScheme >> 8, - this._rfbAuthScheme]); + this._sock.sQpush32(this._rfbAuthScheme); + this._sock.flush(); this._rfbVeNCryptState = 4; return true; @@ -1609,20 +1615,11 @@ export default class RFB extends EventTargetMixin { const user = encodeUTF8(this._rfbCredentials.username); const pass = encodeUTF8(this._rfbCredentials.password); - 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); + this._sock.sQpush32(user.length); + this._sock.sQpush32(pass.length); + this._sock.sQpushString(user); + this._sock.sQpushString(pass); + this._sock.flush(); this._rfbInitState = "SecurityResult"; return true; @@ -1641,7 +1638,8 @@ export default class RFB extends EventTargetMixin { // TODO(directxman12): make genDES not require an Array const challenge = Array.prototype.slice.call(this._sock.rQshiftBytes(16)); const response = RFB.genDES(this._rfbCredentials.password, challenge); - this._sock.send(response); + this._sock.sQpushBytes(response); + this._sock.flush(); this._rfbInitState = "SecurityResult"; return true; } @@ -1659,8 +1657,9 @@ export default class RFB extends EventTargetMixin { if (this._rfbCredentials.ardPublicKey != undefined && this._rfbCredentials.ardCredentials != undefined) { // if the async web crypto is done return the results - this._sock.send(this._rfbCredentials.ardCredentials); - this._sock.send(this._rfbCredentials.ardPublicKey); + this._sock.sQpushBytes(this._rfbCredentials.ardCredentials); + this._sock.sQpushBytes(this._rfbCredentials.ardPublicKey); + this._sock.flush(); this._rfbCredentials.ardCredentials = null; this._rfbCredentials.ardPublicKey = null; this._rfbInitState = "SecurityResult"; @@ -1724,10 +1723,12 @@ export default class RFB extends EventTargetMixin { return false; } - this._sock.send([0, 0, 0, this._rfbCredentials.username.length]); - this._sock.send([0, 0, 0, this._rfbCredentials.password.length]); - this._sock.sendString(this._rfbCredentials.username); - this._sock.sendString(this._rfbCredentials.password); + this._sock.sQpush32(this._rfbCredentials.username.length); + this._sock.sQpush32(this._rfbCredentials.password.length); + this._sock.sQpushString(this._rfbCredentials.username); + this._sock.sQpushString(this._rfbCredentials.password); + this._sock.flush(); + this._rfbInitState = "SecurityResult"; return true; } @@ -1765,7 +1766,8 @@ export default class RFB extends EventTargetMixin { "vendor or signature"); } Log.Debug("Selected tunnel type: " + clientSupportedTunnelTypes[0]); - this._sock.send([0, 0, 0, 0]); // use NOTUNNEL + this._sock.sQpush32(0); // use NOTUNNEL + this._sock.flush(); return false; // wait until we receive the sub auth count to continue } else { return this._fail("Server wanted tunnels, but doesn't support " + @@ -1815,7 +1817,8 @@ export default class RFB extends EventTargetMixin { for (let authType in clientSupportedTypes) { if (serverSupportedTypes.indexOf(authType) != -1) { - this._sock.send([0, 0, 0, clientSupportedTypes[authType]]); + this._sock.sQpush32(clientSupportedTypes[authType]); + this._sock.flush(); Log.Debug("Selected authentication type: " + authType); switch (authType) { @@ -1911,9 +1914,10 @@ export default class RFB extends EventTargetMixin { passwordBytes[password.length] = 0; usernameBytes = legacyCrypto.encrypt({ name: "DES-CBC", iv: secret }, key, usernameBytes); passwordBytes = legacyCrypto.encrypt({ name: "DES-CBC", iv: secret }, key, passwordBytes); - this._sock.send(B); - this._sock.send(usernameBytes); - this._sock.send(passwordBytes); + this._sock.sQpushBytes(B); + this._sock.sQpushBytes(usernameBytes); + this._sock.sQpushBytes(passwordBytes); + this._sock.flush(); this._rfbInitState = "SecurityResult"; return true; } @@ -2141,7 +2145,8 @@ export default class RFB extends EventTargetMixin { return this._handleSecurityReason(); case 'ClientInitialisation': - this._sock.send([this._shared ? 1 : 0]); // ClientInitialisation + this._sock.sQpush8(this._shared ? 1 : 0); // ClientInitialisation + this._sock.flush(); this._rfbInitState = 'ServerInitialisation'; return true; @@ -2887,21 +2892,13 @@ export default class RFB extends EventTargetMixin { // Class Methods RFB.messages = { keyEvent(sock, keysym, down) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(4); // msg-type + sock.sQpush8(down); - buff[offset] = 4; // msg-type - buff[offset + 1] = down; + sock.sQpush16(0); - buff[offset + 2] = 0; - buff[offset + 3] = 0; + sock.sQpush32(keysym); - buff[offset + 4] = (keysym >> 24); - buff[offset + 5] = (keysym >> 16); - buff[offset + 6] = (keysym >> 8); - buff[offset + 7] = keysym; - - sock._sQlen += 8; sock.flush(); }, @@ -2915,46 +2912,28 @@ RFB.messages = { return xtScanCode; } - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(255); // msg-type + sock.sQpush8(0); // sub msg-type - buff[offset] = 255; // msg-type - buff[offset + 1] = 0; // sub msg-type + sock.sQpush16(down); - buff[offset + 2] = (down >> 8); - buff[offset + 3] = down; - - buff[offset + 4] = (keysym >> 24); - buff[offset + 5] = (keysym >> 16); - buff[offset + 6] = (keysym >> 8); - buff[offset + 7] = keysym; + sock.sQpush32(keysym); const RFBkeycode = getRFBkeycode(keycode); - buff[offset + 8] = (RFBkeycode >> 24); - buff[offset + 9] = (RFBkeycode >> 16); - buff[offset + 10] = (RFBkeycode >> 8); - buff[offset + 11] = RFBkeycode; + sock.sQpush32(RFBkeycode); - sock._sQlen += 12; sock.flush(); }, pointerEvent(sock, x, y, mask) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(5); // msg-type - buff[offset] = 5; // msg-type + sock.sQpush8(mask); - buff[offset + 1] = mask; + sock.sQpush16(x); + sock.sQpush16(y); - buff[offset + 2] = x >> 8; - buff[offset + 3] = x; - - buff[offset + 4] = y >> 8; - buff[offset + 5] = y; - - sock._sQlen += 6; sock.flush(); }, @@ -3054,14 +3033,11 @@ RFB.messages = { }, clientCutText(sock, data, extended = false) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(6); // msg-type - buff[offset] = 6; // msg-type - - buff[offset + 1] = 0; // padding - buff[offset + 2] = 0; // padding - buff[offset + 3] = 0; // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding let length; if (extended) { @@ -3070,12 +3046,7 @@ RFB.messages = { length = data.length; } - buff[offset + 4] = length >> 24; - buff[offset + 5] = length >> 16; - buff[offset + 6] = length >> 8; - buff[offset + 7] = length; - - sock._sQlen += 8; + sock.sQpush32(length); // We have to keep track of from where in the data we begin creating the // buffer for the flush in the next iteration. @@ -3085,11 +3056,8 @@ RFB.messages = { while (remaining > 0) { let flushSize = Math.min(remaining, (sock._sQbufferSize - sock._sQlen)); - for (let i = 0; i < flushSize; i++) { - buff[sock._sQlen + i] = data[dataOffset + i]; - } - sock._sQlen += flushSize; + sock.sQpushBytes(data.subarray(dataOffset, dataOffset + flushSize)); sock.flush(); remaining -= flushSize; @@ -3099,92 +3067,57 @@ RFB.messages = { }, setDesktopSize(sock, width, height, id, flags) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(251); // msg-type - buff[offset] = 251; // msg-type - buff[offset + 1] = 0; // padding - buff[offset + 2] = width >> 8; // width - buff[offset + 3] = width; - buff[offset + 4] = height >> 8; // height - buff[offset + 5] = height; + sock.sQpush8(0); // padding - buff[offset + 6] = 1; // number-of-screens - buff[offset + 7] = 0; // padding + sock.sQpush16(width); + sock.sQpush16(height); + + sock.sQpush8(1); // number-of-screens + + sock.sQpush8(0); // padding // screen array - buff[offset + 8] = id >> 24; // id - buff[offset + 9] = id >> 16; - buff[offset + 10] = id >> 8; - buff[offset + 11] = id; - buff[offset + 12] = 0; // x-position - buff[offset + 13] = 0; - buff[offset + 14] = 0; // y-position - buff[offset + 15] = 0; - buff[offset + 16] = width >> 8; // width - buff[offset + 17] = width; - buff[offset + 18] = height >> 8; // height - buff[offset + 19] = height; - buff[offset + 20] = flags >> 24; // flags - buff[offset + 21] = flags >> 16; - buff[offset + 22] = flags >> 8; - buff[offset + 23] = flags; + sock.sQpush32(id); + sock.sQpush16(0); // x-position + sock.sQpush16(0); // y-position + sock.sQpush16(width); + sock.sQpush16(height); + sock.sQpush32(flags); - sock._sQlen += 24; sock.flush(); }, clientFence(sock, flags, payload) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(248); // msg-type - buff[offset] = 248; // msg-type + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding - buff[offset + 1] = 0; // padding - buff[offset + 2] = 0; // padding - buff[offset + 3] = 0; // padding + sock.sQpush32(flags); - buff[offset + 4] = flags >> 24; // flags - buff[offset + 5] = flags >> 16; - buff[offset + 6] = flags >> 8; - buff[offset + 7] = flags; + sock.sQpush8(payload.length); + sock.sQpushString(payload); - const n = payload.length; - - buff[offset + 8] = n; // length - - for (let i = 0; i < n; i++) { - buff[offset + 9 + i] = payload.charCodeAt(i); - } - - sock._sQlen += 9 + n; sock.flush(); }, enableContinuousUpdates(sock, enable, x, y, width, height) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(150); // msg-type - buff[offset] = 150; // msg-type - buff[offset + 1] = enable; // enable-flag + sock.sQpush8(enable); - buff[offset + 2] = x >> 8; // x - buff[offset + 3] = x; - buff[offset + 4] = y >> 8; // y - buff[offset + 5] = y; - buff[offset + 6] = width >> 8; // width - buff[offset + 7] = width; - buff[offset + 8] = height >> 8; // height - buff[offset + 9] = height; + sock.sQpush16(x); + sock.sQpush16(y); + sock.sQpush16(width); + sock.sQpush16(height); - sock._sQlen += 10; sock.flush(); }, pixelFormat(sock, depth, trueColor) { - const buff = sock._sQ; - const offset = sock._sQlen; - let bpp; if (depth > 16) { @@ -3197,100 +3130,69 @@ RFB.messages = { const bits = Math.floor(depth/3); - buff[offset] = 0; // msg-type + sock.sQpush8(0); // msg-type - buff[offset + 1] = 0; // padding - buff[offset + 2] = 0; // padding - buff[offset + 3] = 0; // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding - buff[offset + 4] = bpp; // bits-per-pixel - buff[offset + 5] = depth; // depth - buff[offset + 6] = 0; // little-endian - buff[offset + 7] = trueColor ? 1 : 0; // true-color + sock.sQpush8(bpp); + sock.sQpush8(depth); + sock.sQpush8(0); // little-endian + sock.sQpush8(trueColor ? 1 : 0); - buff[offset + 8] = 0; // red-max - buff[offset + 9] = (1 << bits) - 1; // red-max + sock.sQpush16((1 << bits) - 1); // red-max + sock.sQpush16((1 << bits) - 1); // green-max + sock.sQpush16((1 << bits) - 1); // blue-max - buff[offset + 10] = 0; // green-max - buff[offset + 11] = (1 << bits) - 1; // green-max + sock.sQpush8(bits * 0); // red-shift + sock.sQpush8(bits * 1); // green-shift + sock.sQpush8(bits * 2); // blue-shift - buff[offset + 12] = 0; // blue-max - buff[offset + 13] = (1 << bits) - 1; // blue-max + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding + sock.sQpush8(0); // padding - buff[offset + 14] = bits * 0; // red-shift - buff[offset + 15] = bits * 1; // green-shift - buff[offset + 16] = bits * 2; // blue-shift - - buff[offset + 17] = 0; // padding - buff[offset + 18] = 0; // padding - buff[offset + 19] = 0; // padding - - sock._sQlen += 20; sock.flush(); }, clientEncodings(sock, encodings) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(2); // msg-type - buff[offset] = 2; // msg-type - buff[offset + 1] = 0; // padding + sock.sQpush8(0); // padding - buff[offset + 2] = encodings.length >> 8; - buff[offset + 3] = encodings.length; - - let j = offset + 4; + sock.sQpush16(encodings.length); for (let i = 0; i < encodings.length; i++) { - const enc = encodings[i]; - buff[j] = enc >> 24; - buff[j + 1] = enc >> 16; - buff[j + 2] = enc >> 8; - buff[j + 3] = enc; - - j += 4; + sock.sQpush32(encodings[i]); } - sock._sQlen += j - offset; sock.flush(); }, fbUpdateRequest(sock, incremental, x, y, w, h) { - const buff = sock._sQ; - const offset = sock._sQlen; - if (typeof(x) === "undefined") { x = 0; } if (typeof(y) === "undefined") { y = 0; } - buff[offset] = 3; // msg-type - buff[offset + 1] = incremental ? 1 : 0; + sock.sQpush8(3); // msg-type - buff[offset + 2] = (x >> 8) & 0xFF; - buff[offset + 3] = x & 0xFF; + sock.sQpush8(incremental ? 1 : 0); - buff[offset + 4] = (y >> 8) & 0xFF; - buff[offset + 5] = y & 0xFF; + sock.sQpush16(x); + sock.sQpush16(y); + sock.sQpush16(w); + sock.sQpush16(h); - buff[offset + 6] = (w >> 8) & 0xFF; - buff[offset + 7] = w & 0xFF; - - buff[offset + 8] = (h >> 8) & 0xFF; - buff[offset + 9] = h & 0xFF; - - sock._sQlen += 10; sock.flush(); }, xvpOp(sock, ver, op) { - const buff = sock._sQ; - const offset = sock._sQlen; + sock.sQpush8(250); // msg-type - buff[offset] = 250; // msg-type - buff[offset + 1] = 0; // padding + sock.sQpush8(0); // padding - buff[offset + 2] = ver; - buff[offset + 3] = op; + sock.sQpush8(ver); + sock.sQpush8(op); - sock._sQlen += 4; sock.flush(); } }; diff --git a/core/websock.js b/core/websock.js index 75be044a..e8e0390c 100644 --- a/core/websock.js +++ b/core/websock.js @@ -175,6 +175,32 @@ export default class Websock { // Send Queue + sQpush8(num) { + this._sQ[this._sQlen++] = num; + } + + sQpush16(num) { + this._sQ[this._sQlen++] = (num >> 8) & 0xff; + this._sQ[this._sQlen++] = (num >> 0) & 0xff; + } + + sQpush32(num) { + this._sQ[this._sQlen++] = (num >> 24) & 0xff; + this._sQ[this._sQlen++] = (num >> 16) & 0xff; + this._sQ[this._sQlen++] = (num >> 8) & 0xff; + this._sQ[this._sQlen++] = (num >> 0) & 0xff; + } + + sQpushString(str) { + let bytes = str.split('').map(chr => chr.charCodeAt(0)); + this.sQpushBytes(new Uint8Array(bytes)); + } + + sQpushBytes(bytes) { + this._sQ.set(bytes, this._sQlen); + this._sQlen += bytes.length; + } + flush() { if (this._sQlen > 0 && this.readyState === 'open') { this._websocket.send(new Uint8Array(this._sQ.buffer, 0, this._sQlen)); @@ -182,16 +208,6 @@ export default class Websock { } } - send(arr) { - this._sQ.set(arr, this._sQlen); - this._sQlen += arr.length; - this.flush(); - } - - sendString(str) { - this.send(str.split('').map(chr => chr.charCodeAt(0))); - } - // Event Handlers off(evt) { this._eventHandlers[evt] = () => {}; diff --git a/tests/test.websock.js b/tests/test.websock.js index f3cc4224..b7fdd7d6 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -157,6 +157,66 @@ describe('Websock', function () { sock.attach(websock); }); + describe('sQpush8()', function () { + it('should send a single byte', function () { + sock.sQpush8(42); + sock.flush(); + expect(sock).to.have.sent(new Uint8Array([42])); + }); + it('should not send any data until flushing', function () { + sock.sQpush8(42); + expect(sock).to.have.sent(new Uint8Array([])); + }); + }); + + describe('sQpush16()', function () { + it('should send a number as two bytes', function () { + sock.sQpush16(420); + sock.flush(); + expect(sock).to.have.sent(new Uint8Array([1, 164])); + }); + it('should not send any data until flushing', function () { + sock.sQpush16(420); + expect(sock).to.have.sent(new Uint8Array([])); + }); + }); + + describe('sQpush32()', function () { + it('should send a number as two bytes', function () { + sock.sQpush32(420420); + sock.flush(); + expect(sock).to.have.sent(new Uint8Array([0, 6, 106, 68])); + }); + it('should not send any data until flushing', function () { + sock.sQpush32(420420); + expect(sock).to.have.sent(new Uint8Array([])); + }); + }); + + describe('sQpushString()', function () { + it('should send a string buffer', function () { + sock.sQpushString('\x12\x34\x56\x78\x90'); + sock.flush(); + expect(sock).to.have.sent(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); + }); + it('should not send any data until flushing', function () { + sock.sQpushString('\x12\x34\x56\x78\x90'); + expect(sock).to.have.sent(new Uint8Array([])); + }); + }); + + describe('sQpushBytes()', function () { + it('should send a byte buffer', function () { + sock.sQpushBytes(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); + sock.flush(); + expect(sock).to.have.sent(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); + }); + it('should not send any data until flushing', function () { + sock.sQpushBytes(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); + expect(sock).to.have.sent(new Uint8Array([])); + }); + }); + describe('flush', function () { it('should actually send on the websocket', function () { sock._sQ = new Uint8Array([1, 2, 3]); @@ -174,20 +234,6 @@ describe('Websock', function () { expect(sock).to.have.sent(new Uint8Array([])); }); }); - - describe('send', function () { - it('should send the given data immediately', function () { - sock.send([1, 2, 3]); - expect(sock).to.have.sent(new Uint8Array([1, 2, 3])); - }); - }); - - describe('sendString', function () { - it('should send after converting the string to an array', function () { - sock.sendString("\x01\x02\x03"); - expect(sock).to.have.sent(new Uint8Array([1, 2, 3])); - }); - }); }); describe('lifecycle methods', function () { From ccef89f55651e083fc2e818a251d09ff0bf9e75a Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 30 May 2023 20:32:31 +0200 Subject: [PATCH 18/18] Implicitly flush Websock if needed Callers shouldn't have to deal with the internal buffering limits of Websock, so implicitly flush the buffer if more room is needed. --- core/rfb.js | 19 +------ core/websock.js | 23 ++++++++- tests/test.websock.js | 116 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 19 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index da95a386..fb9df0b9 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -3047,23 +3047,8 @@ RFB.messages = { } sock.sQpush32(length); - - // We have to keep track of from where in the data we begin creating the - // buffer for the flush in the next iteration. - let dataOffset = 0; - - let remaining = data.length; - while (remaining > 0) { - - let flushSize = Math.min(remaining, (sock._sQbufferSize - sock._sQlen)); - - sock.sQpushBytes(data.subarray(dataOffset, dataOffset + flushSize)); - sock.flush(); - - remaining -= flushSize; - dataOffset += flushSize; - } - + sock.sQpushBytes(data); + sock.flush(); }, setDesktopSize(sock, width, height, id, flags) { diff --git a/core/websock.js b/core/websock.js index e8e0390c..21327c31 100644 --- a/core/websock.js +++ b/core/websock.js @@ -176,15 +176,18 @@ export default class Websock { // Send Queue sQpush8(num) { + this._sQensureSpace(1); this._sQ[this._sQlen++] = num; } sQpush16(num) { + this._sQensureSpace(2); this._sQ[this._sQlen++] = (num >> 8) & 0xff; this._sQ[this._sQlen++] = (num >> 0) & 0xff; } sQpush32(num) { + this._sQensureSpace(4); this._sQ[this._sQlen++] = (num >> 24) & 0xff; this._sQ[this._sQlen++] = (num >> 16) & 0xff; this._sQ[this._sQlen++] = (num >> 8) & 0xff; @@ -197,8 +200,18 @@ export default class Websock { } sQpushBytes(bytes) { - this._sQ.set(bytes, this._sQlen); - this._sQlen += bytes.length; + for (let offset = 0;offset < bytes.length;) { + this._sQensureSpace(1); + + let chunkSize = this._sQbufferSize - this._sQlen; + if (chunkSize > bytes.length - offset) { + chunkSize = bytes.length - offset; + } + + this._sQ.set(bytes.subarray(offset, chunkSize), this._sQlen); + this._sQlen += chunkSize; + offset += chunkSize; + } } flush() { @@ -208,6 +221,12 @@ export default class Websock { } } + _sQensureSpace(bytes) { + if (this._sQbufferSize - this._sQlen < bytes) { + this.flush(); + } + } + // Event Handlers off(evt) { this._eventHandlers[evt] = () => {}; diff --git a/tests/test.websock.js b/tests/test.websock.js index b7fdd7d6..dc361b74 100644 --- a/tests/test.websock.js +++ b/tests/test.websock.js @@ -150,6 +150,8 @@ describe('Websock', function () { describe('Send queue methods', function () { let sock; + const bufferSize = 10 * 1024; + beforeEach(function () { let websock = new FakeWebSocket(); websock._open(); @@ -167,6 +169,18 @@ describe('Websock', function () { sock.sQpush8(42); expect(sock).to.have.sent(new Uint8Array([])); }); + it('should implicitly flush if the queue is full', function () { + for (let i = 0;i <= bufferSize;i++) { + sock.sQpush8(42); + } + + let expected = []; + for (let i = 0;i < bufferSize;i++) { + expected.push(42); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); }); describe('sQpush16()', function () { @@ -179,6 +193,19 @@ describe('Websock', function () { sock.sQpush16(420); expect(sock).to.have.sent(new Uint8Array([])); }); + it('should implicitly flush if the queue is full', function () { + for (let i = 0;i <= bufferSize/2;i++) { + sock.sQpush16(420); + } + + let expected = []; + for (let i = 0;i < bufferSize/2;i++) { + expected.push(1); + expected.push(164); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); }); describe('sQpush32()', function () { @@ -191,6 +218,21 @@ describe('Websock', function () { sock.sQpush32(420420); expect(sock).to.have.sent(new Uint8Array([])); }); + it('should implicitly flush if the queue is full', function () { + for (let i = 0;i <= bufferSize/4;i++) { + sock.sQpush32(420420); + } + + let expected = []; + for (let i = 0;i < bufferSize/4;i++) { + expected.push(0); + expected.push(6); + expected.push(106); + expected.push(68); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); }); describe('sQpushString()', function () { @@ -203,6 +245,41 @@ describe('Websock', function () { sock.sQpushString('\x12\x34\x56\x78\x90'); expect(sock).to.have.sent(new Uint8Array([])); }); + it('should implicitly flush if the queue is full', function () { + for (let i = 0;i <= bufferSize/5;i++) { + sock.sQpushString('\x12\x34\x56\x78\x90'); + } + + let expected = []; + for (let i = 0;i < bufferSize/5;i++) { + expected.push(0x12); + expected.push(0x34); + expected.push(0x56); + expected.push(0x78); + expected.push(0x90); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); + it('should implicitly split a large buffer', function () { + let str = ''; + for (let i = 0;i <= bufferSize/5;i++) { + str += '\x12\x34\x56\x78\x90'; + } + + sock.sQpushString(str); + + let expected = []; + for (let i = 0;i < bufferSize/5;i++) { + expected.push(0x12); + expected.push(0x34); + expected.push(0x56); + expected.push(0x78); + expected.push(0x90); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); }); describe('sQpushBytes()', function () { @@ -215,6 +292,45 @@ describe('Websock', function () { sock.sQpushBytes(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); expect(sock).to.have.sent(new Uint8Array([])); }); + it('should implicitly flush if the queue is full', function () { + for (let i = 0;i <= bufferSize/5;i++) { + sock.sQpushBytes(new Uint8Array([0x12, 0x34, 0x56, 0x78, 0x90])); + } + + let expected = []; + for (let i = 0;i < bufferSize/5;i++) { + expected.push(0x12); + expected.push(0x34); + expected.push(0x56); + expected.push(0x78); + expected.push(0x90); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); + it('should implicitly split a large buffer', function () { + let buffer = []; + for (let i = 0;i <= bufferSize/5;i++) { + buffer.push(0x12); + buffer.push(0x34); + buffer.push(0x56); + buffer.push(0x78); + buffer.push(0x90); + } + + sock.sQpushBytes(new Uint8Array(buffer)); + + let expected = []; + for (let i = 0;i < bufferSize/5;i++) { + expected.push(0x12); + expected.push(0x34); + expected.push(0x56); + expected.push(0x78); + expected.push(0x90); + } + + expect(sock).to.have.sent(new Uint8Array(expected)); + }); }); describe('flush', function () {