diff --git a/TRACE.md b/TRACE.md new file mode 100644 index 0000000..4e424f7 --- /dev/null +++ b/TRACE.md @@ -0,0 +1,22 @@ +# How `maxResponseSize` reaches its enforcement point + +## 1. `new Agent({ maxResponseSize })` → `request()` over plain HTTP/1.1 (default settings) + +1. `lib/dispatcher/agent.js` — the constructor collects it in `...options` (line 25) and copies it into `this[kOptions]` (line 48). On the first dispatch to an origin it passes `kOptions` to the factory (line 95). `defaultFactory` (lines 18–22) creates a `Pool`, because `connections` is not 1. +2. `lib/dispatcher/pool.js` — the constructor collects it in `...options` (line 70) and stores it in `this[kOptions]` (line 101). `createClient()` passes `kOptions` to the factory (line 41). The default factory (line 31) is `new Client(origin, opts)`. +3. `lib/dispatcher/client.js` — the constructor destructures `maxResponseSize` (line 130), validates it (lines 215–217) and stores it as `this[kMaxResponseSize]`, with -1 meaning no limit (line 323). Once the socket connects, `connectH1` is called because the socket's ALPN is not `h2` (lines 557–559). +4. `lib/dispatcher/client-h1.js` — `connectH1` creates the llhttp `Parser`, because `useMilo` is unset and `UNDICI_USE_MILO` is off by default (lines 907–909). `Parser`'s constructor copies `client[kMaxResponseSize]` to `this.maxResponseSize` (line 261). **Enforcement happens here:** `Parser.onBody()` (lines 751–782) checks every body chunk and destroys the socket with `ResponseExceededMaxSizeError` when the limit is exceeded. `bytesRead` goes back to 0 for each new response in `onMessageComplete` (line 806). + +(`lib/core/symbols.js` only defines the `kMaxResponseSize` symbol. `lib/api/api-request.js` and `lib/core/request.js` never see the option. They only get the data after `onBody` lets it through.) + +**Bug (now fixed):** `onBody` compared only the current chunk, `buf.length > maxResponseSize`, and ignored the running total in `this.bytesRead`. A body sent as many chunks, each under the limit, therefore never tripped the check. The fix changes it to `this.bytesRead + buf.length > maxResponseSize`, matching the Milo parser and HTTP/2. + +## 2. Other ways to set it + +| Setup | Passed down? | Enforced on the whole body? | Evidence | +|---|---|---|---| +| `new BalancedPool(upstreams, { maxResponseSize })` | Yes | Yes (after the fix; before it, same bug as above) | `lib/dispatcher/balanced-pool.js:52` collects it in `...opts`, `:62` stores it in `kOptions`, and `:91` passes it to `new Pool(origin, opts)` (`:48`). From there it follows the Pool → Client → `client-h1.js` path from section 1. | +| `new RetryAgent(new Agent({ maxResponseSize }))` | Yes, through the wrapped Agent | Yes, for each response (after the fix) | `lib/dispatcher/retry-agent.js:26` sends every dispatch to the wrapped agent, so the Agent's own limit applies (section 1). Caveats: (a) passing `maxResponseSize` in RetryAgent's *own* options does nothing. Those options are only used as `retryOptions` (`retry-agent.js:13,21`) and never reach a Client. (b) Each retry is a new response whose `bytesRead` starts at 0 (`client-h1.js:806`), so a body that gets resumed with `Range` across several retries is limited per attempt, not as a total. | +| `new ProxyAgent({ uri, maxResponseSize })` | **Only when tunnelling** | Tunnelled: yes (after the fix). Not tunnelled: **no limit** | `lib/dispatcher/proxy-agent.js:200–201` builds the inner `Agent` with `...opts`, so the option reaches Agent `kOptions`. For tunnelled requests (`proxyTunnel: true` or a non-`http:` target, `:41–43`), the factory calls `agentFactory(origin, options)` (`:189`), which creates a Pool/Client with the option. For the default forward-proxy path, used for `http:` targets because `proxyTunnel` has no default (`:132`), the factory returns an `Http1ProxyWrapper` (`:170–186`). Its Client is built with only `{ connect }` (`:59` / `:61`), so `maxResponseSize` is dropped. | +| `new H2CClient(origin, { maxResponseSize })` | Yes | Yes | `lib/dispatcher/h2c-client.js:18` collects it in `...opts` and `:41–47` forwards it to `Client`. `client.js:323` stores it. `useH2c` makes the connector set `alpnProtocol = 'h2'` (`lib/core/connect.js:135–136`), so `client.js:557–558` calls `connectH2`. `lib/dispatcher/client-h2.js:1141` copies it into the per-stream state, and `:1455` checks `state.bytesRead + chunk.length` (the cumulative total), then resets that stream. | +| `new Client(origin, { maxResponseSize, useMilo: true })` | Yes | Yes (was already correct) | `client.js:140,340` stores `useMilo`. `client-h1.js:907–908` creates the Milo `H1Parser`. `lib/dispatcher/parser-h1.js:173` copies the limit. Both body paths check the cumulative total: `consumeDirectBody` at `:433` (`this.bytesRead + len`) and the `EVENT_DATA` handler at `:932` (`this.bytesRead + len`). `bytesRead` goes back to 0 for each response at `:648`. | 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..d887d8a 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') @@ -118,6 +118,71 @@ describe('max response size', async (t) => { await t.completed }) + for (const useMilo of [false, true]) { + test(`should enforce the limit on the whole body when it arrives in small chunks (useMilo: ${useMilo})`, async (t) => { + t = tspl(t, { plan: 2 }) + + const chunk = Buffer.alloc(10, 'a') + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + server.on('request', async (req, res) => { + res.on('error', () => {}) + // Each chunk is below the limit, but together they exceed it. + for (let i = 0; i < 10 && !res.destroyed; i++) { + res.write(chunk) + await new Promise((resolve) => setTimeout(resolve, 5)) + } + res.end() + }) + + server.listen(0) + await once(server, 'listening') + + const agent = new Agent({ maxResponseSize: 25, useMilo }) + after(() => agent.close()) + + const { statusCode, body } = await request(`http://localhost:${server.address().port}`, { dispatcher: agent }) + t.strictEqual(statusCode, 200) + await t.rejects(body.text(), errors.ResponseExceededMaxSizeError) + + await t.completed + }) + + test(`should allow a chunked body whose total equals the limit (useMilo: ${useMilo})`, async (t) => { + t = tspl(t, { plan: 1 }) + + const chunk = Buffer.alloc(10, 'a') + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + server.on('request', async (req, res) => { + for (let i = 0; i < 3; i++) { + res.write(chunk) + await new Promise((resolve) => setTimeout(resolve, 5)) + } + res.end() + }) + + server.listen(0) + await once(server, 'listening') + + const agent = new Agent({ maxResponseSize: 30, useMilo }) + after(() => agent.close()) + + const { body } = await request(`http://localhost:${server.address().port}`, { dispatcher: agent }) + t.strictEqual((await body.text()).length, 30) + + await t.completed + }) + } + test('should throw an error if the response is too big over h2c', async (t) => { t = tspl(t, { plan: 1 })