perf: dump immediatly if known size exceeds limit (#2882)

* perf: dump immediatly if known size exceeds limit * fixup
nodejs · Mar 11, 2024 · 825e6e1 · 825e6e1
1 parent e81dc8d
commit 825e6e1
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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 })