Skip to content

Commit

Permalink
perf: dump immediatly if known size exceeds limit (nodejs#2882)
Browse files Browse the repository at this point in the history
* perf: dump immediatly if known size exceeds limit

* fixup
  • Loading branch information
ronag authored and KhafraDev committed Mar 14, 2024
1 parent 3a93e4f commit 62b6df9
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 3 deletions.
3 changes: 2 additions & 1 deletion lib/api/api-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions lib/api/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {}

Expand All @@ -21,6 +22,7 @@ class BodyReadable extends Readable {
resume,
abort,
contentType = '',
contentLength,
highWaterMark = 64 * 1024 // Same as nodejs fs streams.
}) {
super({
Expand All @@ -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
Expand Down Expand Up @@ -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))) {
Expand All @@ -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())
}
Expand Down
35 changes: 35 additions & 0 deletions test/client-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

Expand Down

0 comments on commit 62b6df9

Please sign in to comment.