From ce240da2601a986278cb312e24ed7f8fdb6f2225 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 22 Jun 2024 00:16:16 +0000 Subject: [PATCH 1/7] build(deps): bump node from `075a5cc` to `9af472b` in /build (#3355) Bumps node from `075a5cc` to `9af472b`. --- updated-dependencies: - dependency-name: node dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Dockerfile b/build/Dockerfile index 8bcab469b60..8997f0e6397 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -1,4 +1,4 @@ -FROM node:22-alpine3.19@sha256:075a5cc188c3c9a49acacd481a9e8a3c9abf4223f02c658e37fdb8e9fe2c4664 +FROM node:22-alpine3.19@sha256:9af472b2578996eb3d6affbcb82fdee6f086da2c43121e75038a4a70317f784f ARG UID=1000 ARG GID=1000 From a5eac88b6c9cb760b30367993f65f330782f2283 Mon Sep 17 00:00:00 2001 From: Gianluigi Oliva Date: Sat, 22 Jun 2024 08:50:42 +0200 Subject: [PATCH 2/7] fix: post request signal (#3354) --- lib/api/api-request.js | 5 +++++ test/request-signal.js | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index ced5590d210..396b430f50e 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -154,6 +154,11 @@ class RequestHandler extends AsyncResource { } onComplete (trailers) { + if (this.removeAbortListener) { + this.removeAbortListener() + this.removeAbortListener = null + } + util.parseHeaders(trailers, this.trailers) this.res.push(null) } diff --git a/test/request-signal.js b/test/request-signal.js index fd4d2f885a5..94ba8ec796b 100644 --- a/test/request-signal.js +++ b/test/request-signal.js @@ -36,9 +36,11 @@ test('post abort signal', async (t) => { server.listen(0, async () => { const ac = new AbortController() - const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) + const uresPromise = request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) ac.abort() + try { + const ures = await uresPromise /* eslint-disable-next-line no-unused-vars */ for await (const chunk of ures.body) { // Do nothing... @@ -61,9 +63,11 @@ test('post abort signal w/ reason', async (t) => { server.listen(0, async () => { const ac = new AbortController() const _err = new Error() - const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) + const uresPromise = request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) ac.abort(_err) + try { + const ures = await uresPromise /* eslint-disable-next-line no-unused-vars */ for await (const chunk of ures.body) { // Do nothing... @@ -74,3 +78,20 @@ test('post abort signal w/ reason', async (t) => { }) await t.completed }) + +test('post abort signal after request completed', async (t) => { + t = tspl(t, { plan: 1 }) + + const server = createServer((req, res) => { + res.end('asd') + }) + after(() => server.close()) + + server.listen(0, async () => { + const ac = new AbortController() + const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) + ac.abort() + t.equal(await ures.body.text(), 'asd') + }) + await t.completed +}) From 02c3a6790cf85d2ec7f5ceb8471222d250f2205f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 22 Jun 2024 14:05:56 +0200 Subject: [PATCH 3/7] Revert "fix: post request signal (#3354)" (#3359) --- lib/api/api-request.js | 5 ----- test/request-signal.js | 25 ++----------------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 396b430f50e..ced5590d210 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -154,11 +154,6 @@ class RequestHandler extends AsyncResource { } onComplete (trailers) { - if (this.removeAbortListener) { - this.removeAbortListener() - this.removeAbortListener = null - } - util.parseHeaders(trailers, this.trailers) this.res.push(null) } diff --git a/test/request-signal.js b/test/request-signal.js index 94ba8ec796b..fd4d2f885a5 100644 --- a/test/request-signal.js +++ b/test/request-signal.js @@ -36,11 +36,9 @@ test('post abort signal', async (t) => { server.listen(0, async () => { const ac = new AbortController() - const uresPromise = request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) + const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) ac.abort() - try { - const ures = await uresPromise /* eslint-disable-next-line no-unused-vars */ for await (const chunk of ures.body) { // Do nothing... @@ -63,11 +61,9 @@ test('post abort signal w/ reason', async (t) => { server.listen(0, async () => { const ac = new AbortController() const _err = new Error() - const uresPromise = request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) + const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) ac.abort(_err) - try { - const ures = await uresPromise /* eslint-disable-next-line no-unused-vars */ for await (const chunk of ures.body) { // Do nothing... @@ -78,20 +74,3 @@ test('post abort signal w/ reason', async (t) => { }) await t.completed }) - -test('post abort signal after request completed', async (t) => { - t = tspl(t, { plan: 1 }) - - const server = createServer((req, res) => { - res.end('asd') - }) - after(() => server.close()) - - server.listen(0, async () => { - const ac = new AbortController() - const ures = await request(`http://0.0.0.0:${server.address().port}`, { signal: ac.signal }) - ac.abort() - t.equal(await ures.body.text(), 'asd') - }) - await t.completed -}) From 27b25897b3589ddfac525c524fcebc3ff4b94c07 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Sun, 23 Jun 2024 15:23:21 +0900 Subject: [PATCH 4/7] websocket: don't use pooled buffer in mask pool (#3357) * websocket: don't use pooled buffer in mask pool * add test * fixup * fixup --- lib/web/websocket/frame.js | 2 +- test/websocket/frame.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/web/websocket/frame.js b/lib/web/websocket/frame.js index b062ffde8ec..fe54646cc7d 100644 --- a/lib/web/websocket/frame.js +++ b/lib/web/websocket/frame.js @@ -27,7 +27,7 @@ try { function generateMask () { if (bufIdx === BUFFER_SIZE) { bufIdx = 0 - crypto.randomFillSync((buffer ??= Buffer.allocUnsafe(BUFFER_SIZE)), 0, BUFFER_SIZE) + crypto.randomFillSync((buffer ??= Buffer.allocUnsafeSlow(BUFFER_SIZE)), 0, BUFFER_SIZE) } return [buffer[bufIdx++], buffer[bufIdx++], buffer[bufIdx++], buffer[bufIdx++]] } diff --git a/test/websocket/frame.js b/test/websocket/frame.js index 0f6a7d4a751..38022f88bc1 100644 --- a/test/websocket/frame.js +++ b/test/websocket/frame.js @@ -5,6 +5,23 @@ const assert = require('node:assert') const { WebsocketFrameSend } = require('../../lib/web/websocket/frame') const { opcodes } = require('../../lib/web/websocket/constants') +// Always be above all tests. +test('Don not use pooled buffer in mask pool', () => { + const allocUnsafe = Buffer.allocUnsafe + let counter = 0 + try { + Buffer.allocUnsafe = (n) => { + counter++ + return allocUnsafe(n) + } + // create mask pool + new WebsocketFrameSend(Buffer.alloc(0)).createFrame(opcodes.BINARY) + assert.strictEqual(counter, 1) + } finally { + Buffer.allocUnsafe = allocUnsafe + } +}) + test('Writing 16-bit frame length value at correct offset when buffer has a non-zero byteOffset', () => { /* When writing 16-bit frame lengths, a `DataView` was being used without setting a `byteOffset` into the buffer: From 0455e06c2a2d418361c264d7ef0d77953c27e0eb Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 23 Jun 2024 10:52:02 +0200 Subject: [PATCH 5/7] fix: consider bytes read when dumping (#3360) --- lib/api/readable.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/api/readable.js b/lib/api/readable.js index a65a7fcb557..452384a9a11 100644 --- a/lib/api/readable.js +++ b/lib/api/readable.js @@ -14,6 +14,7 @@ const kBody = Symbol('kBody') const kAbort = Symbol('kAbort') const kContentType = Symbol('kContentType') const kContentLength = Symbol('kContentLength') +const kBytesRead = Symbol('kBytesRead') const noop = () => {} @@ -35,9 +36,10 @@ class BodyReadable extends Readable { this[kAbort] = abort this[kConsume] = null + this[kBytesRead] = 0 this[kBody] = null this[kContentType] = contentType - this[kContentLength] = contentLength + this[kContentLength] = Number.isFinite(contentLength) ? contentLength : null // Is stream being consumed through Readable API? // This is an optimization so that we avoid checking @@ -99,6 +101,8 @@ class BodyReadable extends Readable { } push (chunk) { + this[kBytesRead] += chunk ? chunk.length : 0 + if (this[kConsume] && chunk !== null) { consumePush(this[kConsume], chunk) return this[kReading] ? super.push(chunk) : true @@ -151,7 +155,7 @@ class BodyReadable extends Readable { } async dump (opts) { - let limit = Number.isFinite(opts?.limit) ? opts.limit : 128 * 1024 + const limit = Number.isFinite(opts?.limit) ? opts.limit : 128 * 1024 const signal = opts?.signal if (signal != null && (typeof signal !== 'object' || !('aborted' in signal))) { @@ -165,7 +169,7 @@ class BodyReadable extends Readable { } return await new Promise((resolve, reject) => { - if (this[kContentLength] > limit) { + if (this[kContentLength] > limit || this[kBytesRead] > limit) { this.destroy(new AbortError()) } @@ -185,8 +189,7 @@ class BodyReadable extends Readable { }) .on('error', noop) .on('data', function (chunk) { - limit -= chunk.length - if (limit <= 0) { + if (this[kBytesRead] > limit) { this.destroy() } }) From 3f4ef9a27f3d33e0dadf72bfe8d8d3604b7a37e6 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 23 Jun 2024 10:52:21 +0200 Subject: [PATCH 6/7] refactor: simplify signal handling (#3362) --- lib/api/api-request.js | 8 +------- lib/core/util.js | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index ced5590d210..ca0343d606c 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -77,12 +77,6 @@ class RequestHandler extends AsyncResource { } else if (this.abort) { this.abort(this.reason) } - - if (this.removeAbortListener) { - this.res?.off('close', this.removeAbortListener) - this.removeAbortListener() - this.removeAbortListener = null - } }) } } @@ -127,6 +121,7 @@ class RequestHandler extends AsyncResource { if (this.removeAbortListener) { res.on('close', this.removeAbortListener) + this.removeAbortListener = null } this.callback = null @@ -183,7 +178,6 @@ class RequestHandler extends AsyncResource { } if (this.removeAbortListener) { - res?.off('close', this.removeAbortListener) this.removeAbortListener() this.removeAbortListener = null } diff --git a/lib/core/util.js b/lib/core/util.js index 00f8a9b200a..9b566ef2962 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -518,7 +518,7 @@ function addAbortListener (signal, listener) { signal.addEventListener('abort', listener, { once: true }) return () => signal.removeEventListener('abort', listener) } - signal.addListener('abort', listener) + signal.once('abort', listener) return () => signal.removeListener('abort', listener) } From 59fc6d5518723d3bde28a14eedfb806a21ed59bc Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 23 Jun 2024 10:53:20 +0200 Subject: [PATCH 7/7] fix: use explicit flag for when use has interacted with stream (#3361) --- lib/api/readable.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/api/readable.js b/lib/api/readable.js index 452384a9a11..a04ca8edca2 100644 --- a/lib/api/readable.js +++ b/lib/api/readable.js @@ -14,6 +14,7 @@ const kBody = Symbol('kBody') const kAbort = Symbol('kAbort') const kContentType = Symbol('kContentType') const kContentLength = Symbol('kContentLength') +const kUsed = Symbol('kUsed') const kBytesRead = Symbol('kBytesRead') const noop = () => {} @@ -38,6 +39,7 @@ class BodyReadable extends Readable { this[kConsume] = null this[kBytesRead] = 0 this[kBody] = null + this[kUsed] = false this[kContentType] = contentType this[kContentLength] = Number.isFinite(contentLength) ? contentLength : null @@ -48,7 +50,7 @@ class BodyReadable extends Readable { this[kReading] = false } - destroy (err) { + _destroy (err, callback) { if (!err && !this._readableState.endEmitted) { err = new RequestAbortedError() } @@ -57,15 +59,11 @@ class BodyReadable extends Readable { this[kAbort]() } - return super.destroy(err) - } - - _destroy (err, callback) { // Workaround for Node "bug". If the stream is destroyed in same // tick as it is created, then a user who is waiting for a // promise (i.e micro tick) for installing a 'error' listener will // never get a chance and will always encounter an unhandled exception. - if (!this[kReading]) { + if (!this[kUsed]) { setImmediate(() => { callback(err) }) @@ -77,6 +75,7 @@ class BodyReadable extends Readable { on (ev, ...args) { if (ev === 'data' || ev === 'readable') { this[kReading] = true + this[kUsed] = true } return super.on(ev, ...args) }