Public: anyone can read this; the crew can change it.

Security: a request body has a cap but no deadline — a trickling client holds a handler open #87

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`readBody` in `src/http.ts` now has a clock as well as a cap. One wall-clock budget — `BODY_MS`, five seconds — covers the whole body, raced against every `reader.read()` and cleared however the read ends, so a client that trickles a byte at a time can no longer hold a handler open; the drain loop after an oversized body is inside the same clock. Out of time is the end of the request: the stream i…

shipped production is serving it deploy ab962da

opened this

readBody in src/http.ts now has a clock as well as a cap. One wall-clock budget — BODY_MS, five seconds — covers the whole body, raced against every reader.read() and cleared however the read ends, so a client that trickles a byte at a time can no longer hold a handler open; the drain loop after an oversized body is inside the same clock. Out of time is the end of the request: the stream is let go (the cancel is not awaited, since a stream that stalled is exactly the one that would stall there too) and the read returns the status that refuses it — 408, or 413 when the body was already past the cap.

The refusal used to be undefined from readBody/readText/readForm; it is now the status itself (Refused = 413 | 408, with isRefused and refusal for the message), which let the two duplicated "that body was too large" replies in src/server.ts become one refuse() that sends the right status. That response carries Connection: close: bytes of the abandoned body are still on the wire, and without it the next request on that connection reads them as its own — which is exactly what the first draft of the end-to-end test walked into.

Worth a look: src/http.ts:41-131 for the clock and the two loops, and src/server.ts:92-101 for the refusal. Tests are in src/server.test.ts beside the cap's own — the deadline unit-tested with a short budget (a stream that stalls, one that never sends a first byte, one that stalls mid-drain, one that finishes in time), plus an end-to-end case written on a raw socket, because fetch cannot say "and now nothing, forever": half a form body and silence comes back 408 with the security floor on it, in about five seconds. bun test is 136 pass / 0 fail, up from 131, and the suite is ~6s slower for that one real stall.

What I did not do: make the budget configurable per route, touch Bun's idleTimeout, or add a limit on concurrent in-flight bodies — the deadline is the other half of the cap the issue names, and that is all this changes. Bun does not close the socket for us when the handler sets Connection: close, so an abusive client that ignores the header still holds an idle socket until the server's 60s idle timeout; it holds no handler, which was the hole. Separately, bun test --randomize turned up a pre-existing order dependency in the audit paging test (it pages through rows other tests left behind) — filed as issue #86, untouched here.

🤖 Generated with Claude Code

Closes #54

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs