fix: stream proxy body, forward Range/conditional and custom headers (#10)

[herbie-bot]

Fixes #10.

createBackendProxyHandler buffered the entire request body via req.arrayBuffer() and forwarded only Content-Type/Accept. That made the proxy unusable for large binary uploads (memory proportional to file size) and broke browser media seeking (no Range forwarding).

Changes

• Stream the body with req.body + duplex: "half" (undici requirement) instead of buffering — arbitrary-sized uploads now pass through with constant memory.
• Always forward range/conditional headers: Range, If-Range, If-Match, If-None-Match, If-Modified-Since, If-Unmodified-Since, alongside Content-Type/Accept. Media seeking (206 Partial Content) and conditional caching now work.
• Optional forwardRequestHeaders allowlist (case-insensitive) for application headers like idempotency keys and checksums. Hard deny-set that is never forwarded regardless of the list: Cookie, Host, Authorization (proxy sets its own bearer), Content-Length, Connection.
• Tests (src/server/__tests__/route-handlers.test.ts, 9 cases): streaming (duplex: "half", stream body, body not consumed), GET has no body, range/conditional forwarding, 206 status+body passthrough, custom-header allowlisting, Cookie/Host exclusion regression, and spoofed-Authorization override.
• README documents streaming, the always-forwarded header set, and forwardRequestHeaders with security exclusions.

Acceptance criteria

• Arbitrary-sized request bodies pass through with constant memory
• Range requests produce 206 Partial Content responses
• Custom headers configurable without security regression
• Regression test verifies Cookie exclusion

Verification

pnpm typecheck, pnpm lint, pnpm format:check, pnpm build all clean; pnpm test → 69 passed (10 files), including the 9 new tests.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The change is well-scoped, preserves a strict security posture for headers, and includes targeted tests that cover the new streaming and forwarding behavior.

Pull request overview

This PR fixes the backend proxy route handler so it can handle large binary uploads efficiently and supports browser media seeking by streaming request bodies and forwarding range/conditional headers, while keeping a security-focused request-header allowlist.

Changes:

• Stream proxied request bodies via req.body (using duplex: "half") instead of buffering with arrayBuffer().
• Always forward Range + conditional headers needed for partial content and caching, and add an optional forwardRequestHeaders allowlist with a hard deny-set.
• Add Vitest coverage for streaming behavior, header forwarding/denial, and 206 Partial Content passthrough; update README with the new behavior and config.

File summaries

File	Description
src/server/route-handlers.ts	Switch to streaming request proxying and introduce safe, configurable request-header forwarding.
src/server/tests/route-handlers.test.ts	Add regression tests covering streaming, range/conditional forwarding, and header allow/deny behavior.
README.md	Document streaming semantics and the header forwarding/denial rules, including forwardRequestHeaders.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[danielmarv]

Looks good overall streaming the request body is the right fix, and the header filtering is handled safely.

i would suggest to consider;

1. Set redirect: "manual" on the upstream fetch. fetch follows redirects by default, and 307/308 responses require it to repeat the request with the same method and body. That worked while the body was buffered as an ArrayBuffer, but req.body is now a single-use stream and cannot be replayed after the first request starts consuming it. The redirected fetch can therefore fail. Using manual redirect handling lets the proxy return the backend’s 3xx response and Location header to the client instead.

2. Pass signal: req.signal so cancelling an upload also aborts the upstream request. Otherwise, the backend connection can remain open after the client disconnects.

[herbie-bot]

Pushed 51630ee — both review points plus two defects that surfaced while verifying the change against a real HTTP server.

@danielmarv — both adopted

redirect: "manual" — correct, and worth recording why it is safe here: the browser Fetch spec answers a non-navigate redirect: "manual" with an opaque redirect (status 0, empty headers), which new Response(body, { status: 0 }) rejects with a RangeError. undici does not do that. Measured against a real server:

redirect:follow -> status=200 type=basic location=null
redirect:manual -> status=307 type=basic location="/target"
   new Response(...) ok, status=307

So the real 3xx and its Location are relayed. A test pins it, and the code comment names the divergence so nobody "corrects" it later.

signal: req.signal — adopted as suggested. It matters more than it looks for open-transcript: its upload endpoint streams into an S3 multipart upload and deletes the PENDING row when the write fails, so propagating the abort is what lets a cancelled upload roll back instead of leaving the backend draining a connection nobody is on.

Content-Length moved out of the deny-set

The deny-set rationale was "hop-by-hop / managed by fetch". That is browser semantics — Node's fetch does not enforce the forbidden-header list. Measured, with a ReadableStream body and duplex: "half":

server saw headers: { ..., "content-length": "24" }
server saw body: {"marks":["chunk:8@126ms","chunk:8@246ms","chunk:8@370ms"]}

The header arrives and the body still streams. It has to arrive: without a declared length the upstream request is framed as Transfer-Encoding: chunked, so a backend asking for the length gets -1. open-transcript uses exactly that value to reject an over-sized upload before reading a byte:

long declaredLength = request.getContentLengthLong();
if (contentLength != null && contentLength > maxBytes) throw new PayloadTooLargeException(maxBytes);

Dropping the header does not break anything visibly — it silently disables the check and turns a cheap rejection into a full 512 MB transfer. The body is relayed byte for byte, so the incoming length stays accurate.

Response headers were relayed verbatim — a pre-existing defect

This one predates the PR but lives in the lines it touches. fetch adds Accept-Encoding: gzip, deflate by itself and transparently decodes the response, while leaving the encoding headers describing the compressed bytes. The proxy's new Response(response.body, { headers: response.headers }) then relays a contradiction:

	upstream	after the proxy
Content-Encoding	gzip	gzip ← wrong
Content-Length	49	49 ← wrong
body	49 B gzip	226 B plain

The client is handed plain bytes labelled gzip with a length that does not match → ERR_CONTENT_DECODING_FAILED. It is invisible today only because Spring's server.compression.enabled defaults to false; turning it on — an ordinary production tuning step — would break every proxied response in every app at once.

relayedResponseHeaders() drops Content-Encoding and Content-Length only when the upstream response was encoded, so an uncompressed 206 keeps its Content-Length and Content-Range. Hop-by-hop headers are always dropped: a chunked upstream response does carry them, and they contradict the framing Next.js applies to the response it actually sends —

chunked resp headers: [["connection","keep-alive"], ..., ["transfer-encoding","chunked"]]

Tests: 69 → 79

The existing suite mocks fetch, so it can assert the shape of init.body but proves nothing about undici — and acceptance criterion "arbitrary-sized bodies pass through with memory staying flat" was ticked by a test that never moves a byte. Added route-handlers.streaming.test.ts (@vitest-environment node): the handler runs against a real node:http server with the runtime's real fetch. The streaming case withholds its second body chunk until the server confirms the first, so a buffering handler waits on a stream that waits on it, and the test fails on an explicit assertion rather than hanging:

AssertionError: expected false to be true
 ❯ Object.pull route-handlers.streaming.test.ts:109:28

That is the output with arrayBuffer() put back — verified, so the canary can actually die. It is also the regression test for a Node or Next upgrade breaking duplex: "half".

One necessary side effect

@types/node as a devDependency (^24, matching .nvmrc). The repository carried no Node types at all, and the integration suite needs them. Dev-only; dist/ stays free of tests.

typecheck, lint, format:check, test (79 passed), build — all clean.

Recorded, not built

docs/TODO.md gains the Expect: 100-continue extension: it would let a backend reject an upload before any body byte flows, which is what a 412-on-retry currently pays 512 MB for. Two prerequisites, both real: fetch cannot request it (undici.request({ expectContinue: true }) can, so it means dropping fetch), and Tomcat answers it itself unless continueResponseTiming is set to ON_REQUEST_BODY_READ — its default is IMMEDIATELY, checked in the 10.1.54 bytecode.