diff --git a/TRACE.md b/TRACE.md new file mode 100644 index 0000000..75a11c4 --- /dev/null +++ b/TRACE.md @@ -0,0 +1,22 @@ +# How `maxResponseSize` reaches the code that enforces it + +Line numbers refer to the tree after the fix in this change. + +## 1. `new Agent({ maxResponseSize })` → `request()` over plain HTTP/1.1, default settings + +1. `lib/dispatcher/agent.js` — the constructor keeps `maxResponseSize` in `...options` and saves a copy in `this[kOptions]` (L25, L48). On the first dispatch to an origin, `[kDispatch]` calls the factory with those options (L95). The default factory makes a `Pool` because `connections` is not 1 (L18-21). +2. `lib/dispatcher/pool.js` — the constructor keeps it in `...options` and stores it in `this[kOptions]` (L70, L101). `createClient` passes `pool[kOptions]` to the factory (L41), and the default factory does `new Client(origin, opts)` (L30-31). +3. `lib/dispatcher/client.js` — destructures `maxResponseSize` (L130), checks that it is an integer ≥ -1 (L215-216), and stores it as `this[kMaxResponseSize]` (L323; a missing value becomes `-1`, meaning no limit). Once the socket connects, it picks `connectH1` for an HTTP/1.1 socket (L558-559). The `kMaxResponseSize` symbol comes from `lib/core/symbols.js`. +4. `lib/dispatcher/client-h1.js` — `connectH1` builds the llhttp `Parser`, because Milo is off by default (L907-909; the default is `UNDICI_USE_MILO`, `lib/dispatcher/parser-h1.js` L70-71). The `Parser` constructor copies `client[kMaxResponseSize]` to `this.maxResponseSize` (L261). The WASM callback `wasm_on_body` (L184-187) calls `Parser.onBody`, **which enforces the limit (L770)** by destroying the socket with `ResponseExceededMaxSizeError`. `bytesRead` goes back to 0 for each message in `onMessageComplete` (L806). + +**The bug:** `onBody` compared only the current chunk with the limit (`buf.length > maxResponseSize`) and ignored `this.bytesRead`. So a body sent in many chunks, each under the limit, was never rejected. It now checks `this.bytesRead + buf.length > maxResponseSize`, like the Milo parser and HTTP/2 already do. + +## 2. Other ways to set it + +| Setup | Passed down? | Whole body enforced? | Evidence | +|---|---|---|---| +| `BalancedPool(upstreams, { maxResponseSize })` | Yes | Yes (after the fix; before it, chunked bodies got through, same as Agent) | `lib/dispatcher/balanced-pool.js` L52 + L62 keep it in `this[kOptions]`. `addUpstream` L91 calls the factory, whose default is `new Pool(origin, opts)` (L47-48). From there it follows the Pool → Client → `client-h1.js` path from section 1. | +| `RetryAgent(new Agent({ maxResponseSize }))` | Yes, through the wrapped Agent | Yes (after the fix), for each response attempt | `lib/dispatcher/retry-agent.js` L18-27 just hands `dispatch` to the wrapped agent, so the Agent path above applies. If you pass `maxResponseSize` to the RetryAgent itself, it ends up only in the retry options (L13, L21) and does nothing. `UND_ERR_RES_EXCEEDED_MAX_SIZE` is not in the default retryable `errorCodes` (`lib/handler/retry-handler.js` L135-144), so going over the limit is not retried. | +| `ProxyAgent({ uri, maxResponseSize })` | **Only for tunnelled requests** (HTTPS targets, or `proxyTunnel: true`) | Tunnelled: yes (after the fix). Plain `http:` target with the default `proxyTunnel` (unset): **no** | `lib/dispatcher/proxy-agent.js` L200-202: `new Agent({ ...opts, factory })` keeps the option. The tunnel branch of `factory` returns `agentFactory(origin, options)` (L189), which makes Pool → Client. But `shouldProxyTunnel` (L41-43) is false for `http:` targets unless `proxyTunnel === true`. In that case `factory` returns `Http1ProxyWrapper` (L180-188), which creates its proxy client with only `{ connect }` (L58-61), so `maxResponseSize` is dropped. SOCKS proxies drop it too: `Socks5ProxyAgent` gets an explicit list of options (L158-167), and its own `new Pool` passes only `pipelining`/`connections`/`connect` (`lib/dispatcher/socks5-proxy-agent.js` L235-238). | +| `H2CClient(origin, { maxResponseSize })` | Yes | Yes (this path was already correct) | `lib/dispatcher/h2c-client.js` L18 + L41-42 hand `...opts` to `Client`, which stores it (`client.js` L323). `lib/dispatcher/client-h2.js` L1141 copies it into each stream's state, and L1455 checks `state.bytesRead + chunk.length > maxResponseSize`, then resets only that stream. | +| `Client(origin, { maxResponseSize, useMilo: true })` | Yes | Yes (this path was already correct) | `client.js` L140/L340 store `useMilo`, and `client-h1.js` L907-908 then builds `H1Parser`. `lib/dispatcher/parser-h1.js` L173 reads `client[kMaxResponseSize]`. It checks the running total on both body paths: `consumeDirectBody` L433 and `EVENT_DATA` L932. `bytesRead` goes back to 0 for each message at L648. | 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..61806b7 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,59 @@ describe('max response size', async (t) => { await t.completed }) + for (const useMilo of [false, true]) { + test(`should count the whole body when it arrives in many small chunks (useMilo: ${useMilo})`, async (t) => { + t = tspl(t, { plan: 2 }) + + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + server.on('request', (req, res) => { + res.on('error', () => {}) + if (req.url === '/small') { + res.write('ab') + setImmediate(() => res.end('cd')) + return + } + // Every chunk is well under the limit, but the body as a whole is not. + let sent = 0 + const write = () => { + if (res.destroyed) return + if (sent++ === 20) return res.end() + res.write('hello') + setImmediate(write) + } + write() + }) + + server.listen(0) + await once(server, 'listening') + + const agent = new Agent({ maxResponseSize: 10, useMilo }) + after(() => agent.close()) + const origin = `http://localhost:${server.address().port}` + + // A small chunked body must still pass and must not count against later ones. + const small = await request(`${origin}/small`, { dispatcher: agent }) + t.strictEqual(await small.body.text(), 'abcd') + + let error = null + try { + const { body } = await request(origin, { dispatcher: agent }) + await body.text() + } catch (err) { + error = err + } + + t.ok(error instanceof errors.ResponseExceededMaxSizeError) + + await t.completed + }) + } + test('should throw an error if the response is too big over h2c', async (t) => { t = tspl(t, { plan: 1 })