diff --git a/TRACE.md b/TRACE.md new file mode 100644 index 0000000..87e3f34 --- /dev/null +++ b/TRACE.md @@ -0,0 +1,27 @@ +# How `maxResponseSize` travels to the code that enforces it + +## 1. `new Agent({ maxResponseSize })` → `request()`, plain HTTP/1.1, default settings + +1. `index.js:74,205`: `request()` (via `makeDispatcher`) calls `api.request` with `opts.dispatcher` as `this`. The option is already stored on the Agent at this point. +2. `lib/api/api-request.js:233`: calls `this.dispatch(opts, handler)` on the Agent. +3. `lib/dispatcher/agent.js:25,48`: the constructor puts `maxResponseSize` into the `...options` rest object and keeps a deep clone in `this[kOptions]`. +4. `lib/dispatcher/dispatcher-base.js:185`: `dispatch()` forwards to `Agent[kDispatch]`. +5. `lib/dispatcher/agent.js:94-96,18-21`: when the origin is first used, `defaultFactory(origin, this[kOptions])` creates a `Pool`, because `connections` is not 1. The options, including `maxResponseSize`, are passed through unchanged. +6. `lib/dispatcher/pool.js:57-70,101`: `maxResponseSize` is not one of the named parameters, so it stays in `...options` and is stored in `this[kOptions]`. +7. `lib/dispatcher/pool.js:40-41`: `createClient` calls `pool[kFactory](pool[kUrl], pool[kOptions])`, which is `new Client(origin, opts)` (`pool.js:30-31`). +8. `lib/dispatcher/client.js:130,215-217,323`: destructures `maxResponseSize`, checks that it is an integer ≥ -1, and stores it as `this[kMaxResponseSize]` (-1 means no limit). +9. `lib/dispatcher/client.js:557-559`: once the socket connects and ALPN is not `h2`, calls `connectH1(client, socket)`. +10. `lib/dispatcher/client-h1.js:907-909`: Milo is off by default (`UNDICI_USE_MILO` unset and no `useMilo`), so it creates the llhttp `Parser`. +11. `lib/dispatcher/client-h1.js:261`: the `Parser` copies `client[kMaxResponseSize]` into `this.maxResponseSize`. +12. `lib/dispatcher/client-h1.js:770` (`Parser.onBody`): **enforcement**. If the bytes received so far plus the current chunk exceed the limit, it destroys the socket with `ResponseExceededMaxSizeError`. `bytesRead` is reset per response in `onMessageComplete` (`client-h1.js:806`). + - **Bug (now fixed):** this line used to compare only `buf.length > maxResponseSize`, so it checked each chunk alone and never the running total. A large body that arrived in small chunks got through. The check is now `this.bytesRead + buf.length > maxResponseSize`, which matches the Milo and HTTP/2 paths. + +## 2. Other ways of setting it + +| Setup | Passed down? | Enforced on the whole body? | Evidence | +|---|---|---|---| +| **BalancedPool** `new BalancedPool(upstreams, { maxResponseSize })` | Yes | Yes (HTTP/1.1 only after this fix) | `lib/dispatcher/balanced-pool.js:52,62` stores it in `this[kOptions]`. `:91` calls `this[kFactory](upstreamOrigin, this[kOptions])` → `defaultFactory` → `new Pool(origin, opts)` (`:47-48`). From there it follows steps 6–12 above. | +| **RetryAgent wrapping an Agent** `new RetryAgent(new Agent({ maxResponseSize }))` | Yes, because the wrapped Agent owns it | Yes, per attempt (HTTP/1.1 only after this fix) | `lib/dispatcher/retry-agent.js:26` dispatches through the wrapped Agent, which then follows step 3 onward. **But** `new RetryAgent(agent, { maxResponseSize })` does **not** apply a limit: those options are used only as `retryOptions` for `RetryHandler` (`retry-agent.js:10-13,19-22`), and `lib/handler/` never reads `maxResponseSize`. Two other points: `UND_ERR_RES_EXCEEDED_MAX_SIZE` is not a default retryable error code (`lib/handler/retry-handler.js:135-144`), so hitting the limit is not retried. And because `bytesRead` is per response (`client-h1.js:255,806`), a retry that resumes with `Range` after e.g. `ECONNRESET` restarts the count, so the total body delivered across attempts can exceed the limit. | +| **ProxyAgent** `new ProxyAgent({ uri, maxResponseSize })` | Only on the tunnelled path | Tunnelled: yes. Forward-proxied plain HTTP: **no** | `lib/dispatcher/proxy-agent.js:200-202` builds the inner `Agent` with `...opts`. Its factory gets those options and, for tunnelled requests, calls `agentFactory(origin, options)` (`:189`) → Pool/Client, so the limit is enforced. A request is tunnelled for `https:` targets or when `proxyTunnel: true` (`:41-43`). For an `http:` target with the default `proxyTunnel`, the factory returns `Http1ProxyWrapper` (`:170-187`). That wrapper creates `new Client(proxyUrl, { connect })` / `factory(proxyUrl, { connect })` (`:59,61`) and never receives `maxResponseSize`, so **no limit is applied**. The SOCKS5 branch (`:157-167` → `lib/dispatcher/socks5-proxy-agent.js:235-237`) likewise builds its Pool without `maxResponseSize`. This is from reading the code; I did not run a proxy to confirm it. | +| **H2CClient** `new H2CClient(origin, { maxResponseSize })` | Yes | Yes (already cumulative) | `lib/dispatcher/h2c-client.js:18,41-42` passes `...opts` to `Client` (`client.js:130,323`). `client.js:558` → `connectH2`. `lib/dispatcher/client-h2.js:1137,1141` creates per-stream state with `bytesRead: 0` and `maxResponseSize`. `client-h2.js:1455` checks `state.bytesRead + chunk.length > maxResponseSize` and resets only that stream. `:1463` adds to the total. Covered by the existing tests in `test/max-response-size.js`. | +| **Client with Milo** `new Client(origin, { maxResponseSize, useMilo: true })` (or `UNDICI_USE_MILO=1`) | Yes | Yes (already cumulative) | `lib/dispatcher/client.js:140,340` stores `kUseMilo`. `lib/dispatcher/client-h1.js:907-908` creates `H1Parser`. `lib/dispatcher/parser-h1.js:173` copies the limit. Two checks use `this.bytesRead + len > maxResponseSize`: `parser-h1.js:932` (the `EVENT_DATA` path, adding at `:940`) and `parser-h1.js:433` (the direct Content-Length body fast path, adding at `:437`). `bytesRead` is reset per message at `:648`. The new regression test runs on both parsers. | 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..cc4a470 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,41 @@ describe('max response size', async (t) => { await t.completed }) + for (const useMilo of [false, true]) { + test(`should throw an error if a body streamed in small chunks is too big (useMilo: ${useMilo})`, async (t) => { + t = tspl(t, { plan: 2 }) + + const server = createServer({ joinDuplicateHeaders: true }) + after(() => { + server.closeAllConnections?.() + server.close() + }) + + // Every chunk is below the limit, but together they exceed it. + server.on('request', async (req, res) => { + res.on('error', () => {}) + res.writeHead(200) + for (let i = 0; i < 10 && !res.destroyed; i++) { + res.write('x'.repeat(4)) + await new Promise((resolve) => setTimeout(resolve, 10)) + } + res.end() + }) + + server.listen(0) + await once(server, 'listening') + + const agent = new Agent({ maxResponseSize: 10, 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 throw an error if the response is too big over h2c', async (t) => { t = tspl(t, { plan: 1 })