From 62b6df93ea8f3a5961da3cd2f29c4b390b72679f Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 11 Mar 2024 08:13:52 +0100 Subject: [PATCH] perf: dump immediatly if known size exceeds limit (#2882) * perf: dump immediatly if known size exceeds limit * fixup --- lib/api/api-request.js | 3 ++- lib/api/readable.js | 11 +++++++++-- test/client-request.js | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/api/api-request.js b/lib/api/api-request.js index 2537b264ee3..92c8c84228e 100644 --- a/lib/api/api-request.js +++ b/lib/api/api-request.js @@ -91,7 +91,8 @@ class RequestHandler extends AsyncResource { const parsedHeaders = responseHeaders === 'raw' ? util.parseHeaders(rawHeaders) : headers const contentType = parsedHeaders['content-type'] - const body = new Readable({ resume, abort, contentType, highWaterMark }) + const contentLength = parsedHeaders['content-length'] + const body = new Readable({ resume, abort, contentType, contentLength, highWaterMark }) this.callback = null this.res = body diff --git a/lib/api/readable.js b/lib/api/readable.js index f4d935c5bf6..796c237e889 100644 --- a/lib/api/readable.js +++ b/lib/api/readable.js @@ -11,8 +11,9 @@ const { ReadableStreamFrom } = require('../core/util') const kConsume = Symbol('kConsume') const kReading = Symbol('kReading') const kBody = Symbol('kBody') -const kAbort = Symbol('abort') +const kAbort = Symbol('kAbort') const kContentType = Symbol('kContentType') +const kContentLength = Symbol('kContentLength') const noop = () => {} @@ -21,6 +22,7 @@ class BodyReadable extends Readable { resume, abort, contentType = '', + contentLength, highWaterMark = 64 * 1024 // Same as nodejs fs streams. }) { super({ @@ -35,6 +37,7 @@ class BodyReadable extends Readable { this[kConsume] = null this[kBody] = null this[kContentType] = contentType + this[kContentLength] = contentLength // Is stream being consumed through Readable API? // This is an optimization so that we avoid checking @@ -146,7 +149,7 @@ class BodyReadable extends Readable { } async dump (opts) { - let limit = Number.isFinite(opts?.limit) ? opts.limit : 262144 + let limit = Number.isFinite(opts?.limit) ? opts.limit : 128 * 1024 const signal = opts?.signal if (signal != null && (typeof signal !== 'object' || !('aborted' in signal))) { @@ -160,6 +163,10 @@ class BodyReadable extends Readable { } return await new Promise((resolve, reject) => { + if (this[kContentLength] > limit) { + this.destroy(new AbortError()) + } + const onAbort = () => { this.destroy(signal.reason ?? new AbortError()) } diff --git a/test/client-request.js b/test/client-request.js index 2b73d3b01d0..a0f2d43ba9d 100644 --- a/test/client-request.js +++ b/test/client-request.js @@ -14,6 +14,41 @@ const { promisify } = require('node:util') const { NotSupportedError } = require('../lib/core/errors') const { parseFormDataString } = require('./utils/formdata') +test('request dump big', async (t) => { + t = tspl(t, { plan: 3 }) + + const server = createServer((req, res) => { + res.setHeader('content-length', 999999999) + while (res.write('asd')) { + // Do nothing... + } + res.on('drain', () => t.fail()) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + after(() => client.destroy()) + + let dumped = false + client.on('disconnect', () => { + t.strictEqual(dumped, true) + }) + client.request({ + path: '/', + method: 'GET' + }, (err, { body }) => { + t.ifError(err) + body.dump().then(() => { + dumped = true + t.ok(true, 'pass') + }) + }) + }) + + await t.completed +}) + test('request dump', async (t) => { t = tspl(t, { plan: 3 })