Attachment upload buffers the entire body in memory before the size check

[cuibonobo]

Server design-review finding.

Problem

The upload handler enforces maxAttachmentBytes in two passes (src/routes/attachments.ts:17-25): a Content-Length pre-check, then a byte-length check on the fully-read body:

const data = new Uint8Array(await c.req.arrayBuffer());
if (data.byteLength > maxAttachmentBytes) { ... 413 }

The pre-check only constrains honest clients — Content-Length is optional (chunked transfer encoding has none) and unverified. The real check runs after arrayBuffer() has already read the whole request into memory. So:

• A client using chunked encoding can stream an arbitrarily large body; the process buffers all of it before deciding it's too big. A handful of concurrent oversized uploads exhausts memory — the JSON routes got bodyLimit for exactly this (Security: No rate limiting, no JSON body size limit, unbounded query limit parameter #14), but the attachment route's "own limit" is enforced too late to protect anything.
• Even legitimate traffic holds the full payload (default up to 50 MB) per in-flight upload, plus a second copy when putAttachment hashes/writes it.

Direction

Enforce the limit while reading: consume the body stream, count bytes, abort with 413 the moment the counter passes maxAttachmentBytes (Hono's bodyLimit middleware does exactly this and takes a per-route maxSize — the simplest fix is applying it to POST /attachments with config.maxAttachmentBytes and dropping the hand-rolled checks). Keep the Content-Length pre-check as a cheap fast-fail for honest clients.

Noted but out of scope: the storage API itself is bytes-in-memory (BlobAdapter.putAttachment(data: Uint8Array) — and downloads likewise materialize the full file), so true streaming to disk needs a core adapter-contract change. Worth raising with the owner if attachment sizes grow; at the current 50 MB ceiling, bounding the inbound buffer is the part that matters.

Work items

• Stream-limited body read (or bodyLimit with maxAttachmentBytes) on POST /attachments
• Test: chunked upload without Content-Length exceeding the limit → 413 without full buffering (assert via a limit small enough to prove early abort)
• 413 body conforms to Core sync: wire error contract — typed code vocabulary, structured bodies, 400/422 discipline (core #53) #33's error shape

Refs #33; kin to the closed #14 (this is the gap it left).

[cuibonobo]

The contract-level cause of the buffering (the BlobAdapter byte-array API) is now on record as haverstack/core#72, deliberately deferred with named wake-up triggers. This issue's fix — enforcing the limit while reading the inbound stream — stands on its own and is worth doing regardless; core#72 would later make the whole path streaming rather than bounded-buffering.

---

Generated by Claude Code