diff --git a/TRACE.md b/TRACE.md new file mode 100644 index 0000000..557dc42 --- /dev/null +++ b/TRACE.md @@ -0,0 +1,37 @@ +# How `maxResponseSize` travels + +## 1. `new Agent({ maxResponseSize })` → enforcement (plain HTTP/1.1, default settings) + +Defaults here mean: `http:` origin, `connections` not set (so a Pool is used), `useMilo` not set and +`UNDICI_USE_MILO` not set (so the llhttp parser is used). + +1. `lib/dispatcher/agent.js`: the constructor collects `maxResponseSize` into `...options` (line 25) and stores a copy in `this[kOptions]` (line 48). On the first request to an origin, `[kDispatch]` passes `this[kOptions]` to the factory (lines 95-97). `defaultFactory` creates `new Pool(origin, opts)` (lines 18-22). +2. `lib/dispatcher/pool.js`: the constructor leaves `maxResponseSize` in `...options` (line 70) and stores it in `this[kOptions]` (line 101). `createClient` calls `defaultFactory`, which runs `new Client(pool[kUrl], pool[kOptions])` (lines 30-32, 41). +3. `lib/dispatcher/pool-base.js`: routes the request to that Client through `[kGetDispatcher]()` / `dispatcher.dispatch(...)` (lines 243-250). It doesn't touch the option. +4. `lib/dispatcher/client.js`: checks the value (lines 215-216) and stores it on the client as `this[kMaxResponseSize]` (line 323; `-1` means no limit). `connect()` calls `connectH1(client, socket)` because a plain-http socket has no ALPN `h2` (lines 557-559). +5. `lib/core/symbols.js`: defines the `kMaxResponseSize` symbol (line 60) that all of these files share. +6. `lib/dispatcher/client-h1.js`: `connectH1` creates the llhttp `Parser` because Milo is off (lines 907-909). The `Parser` constructor copies `client[kMaxResponseSize]` into `this.maxResponseSize` (line 261). llhttp's `wasm_on_body` callback (lines 184-187) calls `Parser.onBody`, which **enforces the limit** (lines 770-773): it destroys the socket with `ResponseExceededMaxSizeError`. `bytesRead` goes back to 0 for each response in `onMessageComplete` (line 806). + +On the `request()` side, `index.js` (`makeDispatcher`, lines 74-108 and 205) and `lib/api/api-request.js` (line 233) only pass the `dispatcher` along. `lib/dispatcher/dispatcher-base.js` `dispatch` (lines 163-185) forwards to `[kDispatch]`. None of these read `maxResponseSize`. + +**The bug:** before the fix, line 770 compared only the current chunk against the limit +(`buf.length > maxResponseSize`). It never added the bytes already received (`this.bytesRead`). A body +that arrived in one large chunk was rejected, but a body that arrived in many chunks, each under the +limit, was accepted no matter how big it was. The check is now +`this.bytesRead + buf.length > maxResponseSize`, which matches the Milo parser and the HTTP/2 code. + +## 2. Other ways people set it + +| Setup | Passed down? | Limit on the whole body? | Evidence | +|---|---|---|---| +| `new BalancedPool(upstreams, { maxResponseSize })` | Yes | Yes for llhttp after this fix (before it, only per chunk, as in §1) | `lib/dispatcher/balanced-pool.js:52` (stays in `...opts`), `:62` (`this[kOptions]`), `:91` → `defaultFactory` `:47-48` `new Pool(origin, opts)` → same path as §1 steps 2-6 | +| `new RetryAgent(new Agent({ maxResponseSize }))` | Yes, because it is set on the inner Agent | Yes for llhttp after this fix | `lib/dispatcher/retry-agent.js:18-27`: `dispatch` calls `this.#agent.dispatch(opts, retry)`, so the inner Agent's Clients enforce it as in §1. The resulting `UND_ERR_RES_EXCEEDED_MAX_SIZE` error is not in the default retryable `errorCodes` (`lib/handler/retry-handler.js:135-145`), so it isn't retried away. Note: `new RetryAgent(agent, { maxResponseSize })` does **not** work. The second argument is only used as `retryOptions` (`retry-agent.js:13, 21`) and never reaches a Client. | +| `new ProxyAgent({ uri, maxResponseSize })`, **https target or `proxyTunnel: true`** (CONNECT tunnel) | Yes | Yes for llhttp after this fix | `lib/dispatcher/proxy-agent.js:200-202` creates `new Agent({ ...opts, factory, ... })`. For tunnelled origins the factory returns `agentFactory(origin, options)` (`:189`) → `new Pool`/`new Client(origin, opts)` (`:34-39`) with `maxResponseSize` in `opts` | +| `new ProxyAgent({ uri, maxResponseSize })`, **plain `http:` target, `proxyTunnel` not set** (the default for http targets, `:41-43`) | **No** | **No, not enforced at all** | The factory returns `new Http1ProxyWrapper(this[kProxy].uri, { headers, connect, factory, proxyServername })` (`proxy-agent.js:170-187`), which builds the client from `{ connect }` only: `factory(proxyUrl, { connect })` / `new Client(proxyUrl, { connect })` (`:58-62`). `maxResponseSize` and the other client options are dropped. (The SOCKS path `:157-167` drops it too. `Socks5ProxyAgent` builds its own `new Pool(origin, {...})` at `lib/dispatcher/socks5-proxy-agent.js:235`.) This comes from reading the code; I couldn't run a check script in this session. Not fixed here because it is a separate bug from the one reported. | +| `new H2CClient(origin, { maxResponseSize })` | Yes | Yes, it was already enforced on the whole body | `lib/dispatcher/h2c-client.js:18, 41-47` passes `...opts` to `Client` → `lib/dispatcher/client.js:323`. The session is HTTP/2, so `lib/dispatcher/client-h2.js` copies it into the per-stream state (`:1141`, `bytesRead: 0` at `:1136`) and checks the running total in the data handler `state.bytesRead + chunk.length > maxResponseSize` (`:1455-1459`, `bytesRead +=` at `:1463`) | +| `new Client(origin, { maxResponseSize, useMilo: true })` (or `UNDICI_USE_MILO=1`) | Yes | Yes, it was already enforced on the whole body | `lib/dispatcher/client.js:340` stores `kUseMilo`. `lib/dispatcher/client-h1.js:907-908` creates `H1Parser`. `lib/dispatcher/parser-h1.js:173` copies the limit. It is checked as a running total in both body paths: `EVENT_DATA` `:932-940` and `consumeDirectBody` `:433-437`. It resets per response at `:648`. | + +Regression test: `test/max-response-size.js`, "should enforce the limit on the whole body when it +arrives in many small chunks". It uses `new Agent({ maxResponseSize: 1024 })` with `request()` and a server +that streams 64 × 64-byte chunks. It runs once with llhttp and once with Milo. Without the fix the llhttp +variant fails ("Missing expected rejection"); with the fix both pass. 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..b98a115 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,53 @@ 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 many small chunks (useMilo: ${useMilo})`, async (t) => { + t = tspl(t, { plan: 2 }) + + const chunkSize = 64 + const chunks = 64 // 4 KiB in total, against a 1 KiB limit + let written = 0 + + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + server.on('request', (req, res) => { + res.on('error', () => {}) + res.writeHead(200) + const write = () => { + if (res.destroyed || written === chunks) { + res.end() + return + } + written++ + // Each chunk is flushed separately so the client parses it as its + // own body callback, below the limit on its own. + res.write(Buffer.alloc(chunkSize, 'a'), () => setTimeout(write, 1)) + } + write() + }) + + server.listen(0) + await once(server, 'listening') + + const agent = new Agent({ maxResponseSize: 1024, useMilo }) + after(() => agent.close()) + + await t.rejects( + request(`http://localhost:${server.address().port}`, { dispatcher: agent }) + .then(({ body }) => body.text()), + errors.ResponseExceededMaxSizeError + ) + t.ok(written < chunks, `body should be cut off before the end (wrote ${written}/${chunks} chunks)`) + + await t.completed + }) + } + test('invalid max response size should throw an error', async (t) => { t = tspl(t, { plan: 2 })