diff --git a/TRACE.md b/TRACE.md new file mode 100644 index 0000000..06341c5 --- /dev/null +++ b/TRACE.md @@ -0,0 +1,37 @@ +# How `maxResponseSize` is propagated and enforced + +## 1. `new Agent({ maxResponseSize })` → enforcement (plain HTTP/1.1, defaults) + +1. `lib/dispatcher/agent.js:25,48` – the constructor collects everything except `factory`/`maxOrigins`/`connect`/`tls` into `options`, so `maxResponseSize` is kept in `this[kOptions]`. +2. `lib/dispatcher/agent.js:95` – on first dispatch to an origin, `kDispatch` calls `defaultFactory` (lines 20-24) with `this[kOptions]`. +3. `lib/dispatcher/pool.js:25-27,88,101,41` – `defaultFactory` returns a `Pool` (unless `connections === 1`, which returns a `Client`). The `Pool` keeps the option in `this[kOptions]` (line 101) and passes it to `new Client(...)` through `createClient` (line 41). +4. `lib/dispatcher/client.js:130,215-216,323` – the `Client` constructor validates the value (integer, ≥ -1) and stores it as `this[kMaxResponseSize]`, with `-1` meaning unlimited. +5. `lib/dispatcher/client-h1.js:907-909` – on connect, the socket gets a parser. With the defaults (Milo off, `UNDICI_USE_MILO` unset, `parser-h1.js:70`) this is the llhttp `Parser`. +6. `lib/dispatcher/client-h1.js:261` – the `Parser` constructor copies `client[kMaxResponseSize]` to `this.maxResponseSize`. +7. `lib/dispatcher/client-h1.js:752-775` (`onBody`) – this is where the limit is enforced for each body chunk. It throws `ResponseExceededMaxSizeError` by destroying the socket. `bytesRead` is accumulated at line 775 and reset per message at line 806. + +## 2. Other ways of setting it + +| Dispatcher | Passed down? | Enforced on whole body? | Evidence | +|---|---|---|---| +| `BalancedPool` | Yes. Its options are kept in `kOptions` and given to each upstream `Pool` (then `Client`). | Yes, by the same parser code as in section 1. | `lib/dispatcher/balanced-pool.js:52,62,91` | +| `RetryAgent` wrapping an `Agent` | `RetryAgent` takes no `maxResponseSize` and does not forward one. Its options are only retry options. The limit comes from the wrapped `Agent`, so set it there. | Yes, per attempt, by the wrapped dispatcher. The counter resets on each retry, so a body resumed by retry is limited per request, not as a cumulative total. | `lib/dispatcher/retry-agent.js:10-13,18-26` (`this.#options` goes only to `retryOptions`; the dispatch goes to `#agent`) | +| `ProxyAgent` | Partly. The option reaches the inner `Agent` through `...opts` (`proxy-agent.js:200-201`). For tunnelled requests (HTTPS by default, or `proxyTunnel: true`), `factory` calls `agentFactory(origin, options)` (line 189) and the limit is enforced. For non-tunnelled HTTP requests, `Http1ProxyWrapper` is built (lines 180-186) and builds its client with `factory(proxyUrl, { connect })` or `new Client(proxyUrl, { connect })` (lines 58-61). That call drops `options`. | Tunnelled: yes. Non-tunnelled HTTP via proxy: **no, the limit is silently ignored**. This is a separate bug that I did not fix. | `lib/dispatcher/proxy-agent.js:58-61,180-186,189,200-201` | +| `H2CClient` | Yes. `...opts` is spread into `super(...)`, so `Client` receives `maxResponseSize`. | Yes. The HTTP/2 stream state carries `maxResponseSize` and compares `state.bytesRead + chunk.length` (the running total). | `lib/dispatcher/h2c-client.js:18,41-47`; `lib/dispatcher/client-h2.js:1141,1444,1455,1463` | +| `Client` with Milo (`useMilo: true`) | Yes. `client.js:340` stores `kUseMilo` and `client-h1.js:907-908` picks `H1Parser`. `parser-h1.js:173` copies the limit. | Yes. It checks `this.bytesRead + len` in both the direct-body path (`consumeDirectBody`, `parser-h1.js:433-435`) and the event path (`EVENT_DATA`, `parser-h1.js:932`), and increments `bytesRead` (lines 437, 940). | `lib/dispatcher/parser-h1.js:173,433,437,932,940` | + +## The bug (fixed) + +The llhttp parser's `onBody` compared only the current chunk with the limit: + +```js +if (maxResponseSize > -1 && buf.length > maxResponseSize) { +``` + +`this.bytesRead` was updated but never included in the comparison. A body sent in many chunks that were each smaller than the limit was never rejected. The Milo parser and the HTTP/2 path already used `bytesRead + chunk length`. The fix is in `lib/dispatcher/client-h1.js:770`: + +```js +if (maxResponseSize > -1 && this.bytesRead + buf.length > maxResponseSize) { +``` + +Regression test: `test/max-response-size.js`, "should throw an error if a response streamed in small chunks exceeds the limit". It uses `request()` with an `Agent({ maxResponseSize: 25 })` and a server that sends four 10-byte chunks. Before the fix it hangs, because the body is never rejected. diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 295facf..a26b9c0 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -767,7 +767,7 @@ class Parser { assert(statusCode >= 200) - if (maxResponseSize > -1 && buf.length > maxResponseSize) { + if (maxResponseSize > -1 && this.bytesRead + buf.length > maxResponseSize) { util.destroy(socket, new ResponseExceededMaxSizeError()) return -1 } diff --git a/test/max-response-size.js b/test/max-response-size.js index cb5d120..894e08b 100644 --- a/test/max-response-size.js +++ b/test/max-response-size.js @@ -3,7 +3,7 @@ const { tspl } = require('@matteo.collina/tspl') const { test, after, describe } = require('node:test') const { once } = require('node:events') -const { Client, H2CClient, errors } = require('..') +const { Agent, Client, H2CClient, errors, request } = require('..') const { createServer } = require('node:http') const { createServer: createH2CServer } = require('node:http2') @@ -197,6 +197,39 @@ describe('max response size', async (t) => { await t.completed }) + test('should throw an error if a response streamed in small chunks exceeds the limit', async (t) => { + t = tspl(t, { plan: 3 }) + + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + server.on('request', (req, res) => { + res.write('a'.repeat(10)) + setTimeout(() => res.write('b'.repeat(10)), 20) + setTimeout(() => res.write('c'.repeat(10)), 40) + setTimeout(() => res.end('d'.repeat(10)), 60) + }) + + server.listen(0, () => { + const agent = new Agent({ maxResponseSize: 25 }) + after(() => agent.close()) + + request(`http://localhost:${server.address().port}`, { dispatcher: agent }).then(({ body }) => { + t.ok(true) + body.on('error', (err) => { + t.ok(err) + t.ok(err instanceof errors.ResponseExceededMaxSizeError) + }) + body.resume() + }, t.ifError) + }) + + await t.completed + }) + test('invalid max response size should throw an error', async (t) => { t = tspl(t, { plan: 2 })