From 10a9a9bf8088600d2ca08bf329a1211aa85e5458 Mon Sep 17 00:00:00 2001 From: Hendrik Ebbers Date: Fri, 4 Sep 2026 00:15:14 +0200 Subject: [PATCH 1/2] fix: stream proxy body, forward Range/conditional and custom headers createBackendProxyHandler buffered the entire request body via req.arrayBuffer() and forwarded only Content-Type/Accept, which broke large binary uploads and browser media seeking. - Stream the request body with req.body + duplex: "half" so uploads of any size pass through with constant memory instead of the heap. - Always forward the range/conditional headers (Range, If-Range, If-Match, If-None-Match, If-Modified-Since, If-Unmodified-Since) so media seeking (206 Partial Content) and conditional caching work. - Add optional forwardRequestHeaders allowlist (case-insensitive) for application headers such as idempotency keys and checksums, with a hard deny-set that is never forwarded: Cookie, Host, Authorization, Content-Length, Connection. - Add route-handlers tests covering streaming, range forwarding, 206 passthrough, custom-header allowlisting, and Cookie/Host exclusion. - Document the new behavior and forwardRequestHeaders in the README. Fixes #10 Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 24 ++- src/server/__tests__/route-handlers.test.ts | 215 ++++++++++++++++++++ src/server/route-handlers.ts | 79 ++++++- 3 files changed, 307 insertions(+), 11 deletions(-) create mode 100644 src/server/__tests__/route-handlers.test.ts diff --git a/README.md b/README.md index 26ed82f..119a72d 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ pnpm add @open-elements/ui next next-auth react react-dom lucide-react ### `@open-elements/nextjs-app-layer/server` (server-only) - `createAppLayerAuth({ issuer, clientId, clientSecret, ...tuning })` -- `createBackendProxyHandler({ backendUrl, auth })` +- `createBackendProxyHandler({ backendUrl, auth, forwardRequestHeaders? })` - `createLogoutHandler({ auth, oidcIssuer, authUrl })` - `middlewareConfig` (reference value — see warning below) @@ -128,6 +128,28 @@ const handler = createBackendProxyHandler({ export { handler as GET, handler as POST, handler as PUT, handler as DELETE }; ``` +The proxy **streams** the request body to the backend (`duplex: "half"`), so +arbitrarily large uploads pass through with constant memory use instead of being +buffered on the heap. + +It always forwards `Content-Type`, `Accept`, and the range / conditional headers +`Range`, `If-Range`, `If-Match`, `If-None-Match`, `If-Modified-Since`, and +`If-Unmodified-Since`, so browser media seeking (`206 Partial Content`) and +conditional caching work out of the box. + +To forward extra application headers (e.g. idempotency keys or checksums), pass +`forwardRequestHeaders`. Matching is case-insensitive. Security-sensitive headers +are always excluded even if listed: `Cookie`, `Host`, `Authorization` (the proxy +sets its own bearer token), `Content-Length`, and `Connection`. + +```ts +const handler = createBackendProxyHandler({ + backendUrl: process.env.BACKEND_URL ?? "http://localhost:8080", + auth, + forwardRequestHeaders: ["Idempotency-Key", "X-Checksum-Sha256"], +}); +``` + ```ts // src/app/api/logout/route.ts import { auth, oidcIssuer } from "@/auth"; diff --git a/src/server/__tests__/route-handlers.test.ts b/src/server/__tests__/route-handlers.test.ts new file mode 100644 index 0000000..cce73c6 --- /dev/null +++ b/src/server/__tests__/route-handlers.test.ts @@ -0,0 +1,215 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import type { Session } from "next-auth"; +import type { NextRequest } from "next/server"; +import { createBackendProxyHandler } from "../route-handlers"; + +const BACKEND_URL = "http://backend.test"; + +/** Build a session whose access token is attached as a bearer token. */ +function sessionWithToken(accessToken: string): Session { + return { accessToken, expires: "" } as unknown as Session; +} + +/** + * Construct a request the handler can consume. A plain `Request` exposes the + * same `method` / `headers` / `body` / `url` surface the handler reads, so we + * cast it to `NextRequest` instead of pulling in the Next.js runtime. + */ +function makeRequest( + url: string, + init?: { method?: string; headers?: Record; body?: BodyInit }, +): NextRequest { + return new Request(url, init) as unknown as NextRequest; +} + +/** The `{ params }` context the App Router passes as the second argument. */ +function context(path: string[]) { + return { params: Promise.resolve({ path }) }; +} + +/** Capture the URL and init of the single upstream fetch a handler performs. */ +function mockFetch(response: Response) { + return vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>( + async () => response, + ); +} + +describe("createBackendProxyHandler", () => { + const realFetch = globalThis.fetch; + + beforeEach(() => { + vi.restoreAllMocks(); + }); + + afterEach(() => { + globalThis.fetch = realFetch; + }); + + it("should target the backend /api path and forward query params", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + await handler(makeRequest("http://app.test/api/files?limit=5&sort=name"), context(["files"])); + + const calledUrl = new URL(fetchMock.mock.calls[0][0] as string); + expect(calledUrl.origin + calledUrl.pathname).toBe(`${BACKEND_URL}/api/files`); + expect(calledUrl.searchParams.get("limit")).toBe("5"); + expect(calledUrl.searchParams.get("sort")).toBe("name"); + }); + + it("should attach the session access token as a bearer Authorization header", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => sessionWithToken("tok-123"), + }); + await handler(makeRequest("http://app.test/api/me"), context(["me"])); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + expect(headers.get("Authorization")).toBe("Bearer tok-123"); + }); + + it("should stream the request body without buffering and set duplex for undici", async () => { + const fetchMock = mockFetch(new Response(null, { status: 201 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const req = makeRequest("http://app.test/api/upload", { method: "POST", body: "payload" }); + await handler(req, context(["upload"])); + + const init = fetchMock.mock.calls[0][1]! as RequestInit & { duplex?: string }; + expect(init.duplex).toBe("half"); + expect(init.body).toBeInstanceOf(ReadableStream); + // The handler must not consume the body itself (no buffering). + expect(req.bodyUsed).toBe(false); + }); + + it("should not set a body or duplex for GET requests", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + await handler(makeRequest("http://app.test/api/files"), context(["files"])); + + const init = fetchMock.mock.calls[0][1]! as RequestInit & { duplex?: string }; + expect(init.body).toBeUndefined(); + expect(init.duplex).toBeUndefined(); + }); + + it("should forward Range and conditional headers for media seeking", async () => { + const fetchMock = mockFetch(new Response(null, { status: 206 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + await handler( + makeRequest("http://app.test/api/audio.mp3", { + headers: { + Range: "bytes=200-1000", + "If-Range": '"etag-1"', + "If-None-Match": '"etag-2"', + "If-Modified-Since": "Wed, 21 Oct 2015 07:28:00 GMT", + }, + }), + context(["audio.mp3"]), + ); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + expect(headers.get("Range")).toBe("bytes=200-1000"); + expect(headers.get("If-Range")).toBe('"etag-1"'); + expect(headers.get("If-None-Match")).toBe('"etag-2"'); + expect(headers.get("If-Modified-Since")).toBe("Wed, 21 Oct 2015 07:28:00 GMT"); + }); + + it("should pass through the upstream 206 Partial Content status and body", async () => { + const fetchMock = mockFetch(new Response("chunk", { status: 206 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const res = await handler( + makeRequest("http://app.test/api/audio.mp3", { headers: { Range: "bytes=0-4" } }), + context(["audio.mp3"]), + ); + + expect(res.status).toBe(206); + expect(await res.text()).toBe("chunk"); + }); + + it("should forward allow-listed custom headers case-insensitively", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + forwardRequestHeaders: ["Idempotency-Key", "X-Checksum-Sha256"], + }); + await handler( + makeRequest("http://app.test/api/upload", { + method: "POST", + headers: { "idempotency-key": "abc", "X-Checksum-Sha256": "deadbeef" }, + }), + context(["upload"]), + ); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + expect(headers.get("Idempotency-Key")).toBe("abc"); + expect(headers.get("X-Checksum-Sha256")).toBe("deadbeef"); + }); + + it("should not forward the Cookie header even when explicitly allow-listed", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + forwardRequestHeaders: ["Cookie", "Host"], + }); + await handler( + makeRequest("http://app.test/api/me", { + headers: { Cookie: "session=secret", Host: "evil.test" }, + }), + context(["me"]), + ); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + expect(headers.get("Cookie")).toBeNull(); + expect(headers.get("Host")).toBeNull(); + }); + + it("should ignore a client-supplied Authorization header and use the session token", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => sessionWithToken("real-token"), + forwardRequestHeaders: ["Authorization"], + }); + await handler( + makeRequest("http://app.test/api/me", { headers: { Authorization: "Bearer spoofed" } }), + context(["me"]), + ); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + expect(headers.get("Authorization")).toBe("Bearer real-token"); + }); +}); diff --git a/src/server/route-handlers.ts b/src/server/route-handlers.ts index ba65ac8..0527800 100644 --- a/src/server/route-handlers.ts +++ b/src/server/route-handlers.ts @@ -11,19 +11,71 @@ type RouteHandler = ( export interface BackendProxyConfig { readonly backendUrl: string; readonly auth: AuthFn; + /** + * Additional request headers to forward to the backend, matched + * case-insensitively (e.g. `["Idempotency-Key", "X-Checksum-Sha256"]`). + * + * Security-sensitive headers are always excluded regardless of this + * list: `Cookie`, `Host`, `Authorization` (the proxy sets its own + * bearer token), and the hop-by-hop / fetch-managed headers + * `Content-Length` and `Connection`. + */ + readonly forwardRequestHeaders?: readonly string[]; } +/** + * Request headers that are always forwarded to the backend. In addition to + * `Content-Type` and `Accept` these include the conditional / range headers + * that browsers rely on for media seeking and caching. + */ +const ALWAYS_FORWARDED_HEADERS = [ + "content-type", + "accept", + "range", + "if-range", + "if-none-match", + "if-match", + "if-modified-since", + "if-unmodified-since", +] as const; + +/** + * Headers that must never be forwarded to the backend, even when listed in + * `forwardRequestHeaders`. `Cookie` and `Host` would leak session state and + * confuse virtual-host routing; `Authorization` is set by the proxy itself; + * `Content-Length` and `Connection` are hop-by-hop / managed by `fetch`. + */ +const NEVER_FORWARDED_HEADERS = new Set([ + "cookie", + "host", + "authorization", + "content-length", + "connection", +]); + /** * Create a route handler that proxies an authenticated request to the * upstream backend, attaching `Authorization: Bearer ` from - * the session cookie and forwarding query params / body / Content-Type / - * Accept headers. + * the session cookie and forwarding query params, the (streamed) request + * body, and a curated set of request headers. + * + * The body is streamed rather than buffered, so arbitrarily large uploads + * pass through with constant memory use. Range and conditional headers are + * forwarded so that media seeking (`206 Partial Content`) and caching work. + * Extra application headers can be allow-listed via `forwardRequestHeaders`. * * Mount as `export { handler as GET, handler as POST, handler as PUT, handler as DELETE }` * in `frontend/src/app/api/[...path]/route.ts`. */ export function createBackendProxyHandler(config: BackendProxyConfig): RouteHandler { - const { backendUrl, auth } = config; + const { backendUrl, auth, forwardRequestHeaders = [] } = config; + const extraForwarded = new Set( + forwardRequestHeaders + .map((name) => name.toLowerCase()) + .filter((name) => !NEVER_FORWARDED_HEADERS.has(name)), + ); + const forwarded = new Set([...ALWAYS_FORWARDED_HEADERS, ...extraForwarded]); + return async function handler(req, { params }) { const session = await auth(); const { path } = await params; @@ -36,21 +88,28 @@ export function createBackendProxyHandler(config: BackendProxyConfig): RouteHand }); const headers = new Headers(); - const contentType = req.headers.get("Content-Type"); - if (contentType) headers.set("Content-Type", contentType); - const accept = req.headers.get("Accept"); - if (accept) headers.set("Accept", accept); + req.headers.forEach((value, key) => { + if (forwarded.has(key.toLowerCase())) headers.set(key, value); + }); if (session?.accessToken) { headers.set("Authorization", `Bearer ${session.accessToken}`); } const hasBody = req.method !== "GET" && req.method !== "HEAD"; - const response = await fetch(url.toString(), { + // Stream the body instead of buffering it into memory. `duplex: "half"` + // is required by undici (Node's fetch) whenever a ReadableStream body is + // sent; it is not yet in the DOM `RequestInit` type, hence the cast. + const init: RequestInit & { duplex?: "half" } = { method: req.method, headers, - body: hasBody ? await req.arrayBuffer() : undefined, - }); + }; + if (hasBody && req.body) { + init.body = req.body; + init.duplex = "half"; + } + + const response = await fetch(url.toString(), init); return new Response(response.body, { status: response.status, From 51630eed5206d5a053617b120c957ab1b47e8869 Mon Sep 17 00:00:00 2001 From: Hendrik Ebbers Date: Tue, 8 Sep 2026 09:31:49 +0200 Subject: [PATCH 2/2] fix: forward Content-Length, correct relayed response headers, honour abort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the streaming proxy fix, addressing the review of #11 and two defects found while verifying it against a real HTTP server. Review feedback (danielmarv): - `redirect: "manual"`: a streamed body is single-use, so following a 307/308 — which must repeat the request with the same method and body — fails once the stream has been consumed. The 3xx and its `Location` are relayed instead. Verified that undici returns the real response here rather than the browser Fetch spec's opaque redirect with status 0, which `new Response()` rejects. - `signal: req.signal`: a cancelled upload now aborts the upstream request, so the backend stops reading and can roll back its partial write. `Content-Length` is forwarded instead of denied. Node's fetch does not enforce the browser's forbidden-header list, so the header does reach the backend. It has to: with a streamed body and no declared length the upstream request is framed as `Transfer-Encoding: chunked`, and a backend asking for the length (Servlet `getContentLengthLong()`) gets `-1`. Backends use that value to reject an over-sized upload before reading the body, so dropping the header silently disabled the check and turned a cheap rejection into a full transfer. The body is relayed byte for byte, so the incoming length stays accurate. Response headers are corrected rather than relayed verbatim. Node's fetch adds `Accept-Encoding: gzip, deflate` on its own and transparently decodes the response, but leaves `Content-Encoding` and `Content-Length` describing the encoded bytes; relaying them hands the client plain bytes labelled `gzip`, which browsers report as ERR_CONTENT_DECODING_FAILED. Both are dropped when the upstream response was encoded — and only then, so an uncompressed 206 keeps its `Content-Length` and `Content-Range`. Hop-by-hop headers are never relayed: a chunked upstream response carries `Transfer-Encoding` and `Connection` in its headers, which contradict the framing Next.js applies to the response it sends. This defect predates #11 and is invisible today only because Spring's `server.compression.enabled` defaults to false. Tests: 69 -> 79. The existing suite mocks fetch and can only assert the shape of `init.body`; it proves nothing about undici. A new integration suite runs the handler 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 handler that buffers fails on an explicit assertion instead of hanging. Confirmed to fail when `arrayBuffer()` is put back. This is the canary for a Node or Next upgrade breaking `duplex: "half"`. `@types/node` is added as a devDependency (^24, matching .nvmrc) because the repository carried no Node types and the integration suite needs them. It is dev-only; `dist/` stays free of tests. docs/TODO.md records the `Expect: 100-continue` extension that would let a backend reject an upload before any body byte flows, with its two prerequisites (undici instead of fetch, and Tomcat's `continueResponseTiming`). Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 28 ++- docs/TODO.md | 32 +++ package.json | 1 + pnpm-lock.yaml | 38 +++- .../route-handlers.streaming.test.ts | 192 ++++++++++++++++++ src/server/__tests__/route-handlers.test.ts | 135 ++++++++++++ src/server/route-handlers.ts | 85 ++++++-- 7 files changed, 482 insertions(+), 29 deletions(-) create mode 100644 src/server/__tests__/route-handlers.streaming.test.ts diff --git a/README.md b/README.md index 119a72d..f2c4dd0 100644 --- a/README.md +++ b/README.md @@ -132,15 +132,33 @@ The proxy **streams** the request body to the backend (`duplex: "half"`), so arbitrarily large uploads pass through with constant memory use instead of being buffered on the heap. -It always forwards `Content-Type`, `Accept`, and the range / conditional headers -`Range`, `If-Range`, `If-Match`, `If-None-Match`, `If-Modified-Since`, and -`If-Unmodified-Since`, so browser media seeking (`206 Partial Content`) and -conditional caching work out of the box. +It always forwards `Content-Type`, `Content-Length`, `Accept`, and the range / +conditional headers `Range`, `If-Range`, `If-Match`, `If-None-Match`, +`If-Modified-Since`, and `If-Unmodified-Since`, so browser media seeking +(`206 Partial Content`) and conditional caching work out of the box. + +`Content-Length` is forwarded because the body is streamed: without it the +upstream request is framed as `Transfer-Encoding: chunked`, and a backend asking +for the declared length (e.g. Servlet `getContentLengthLong()`) gets `-1`. +Backends use that value to reject an over-sized upload *before* reading the +body, so dropping it would turn a cheap rejection into a full transfer. + +On the way back, headers that `fetch` invalidated are corrected rather than +relayed blindly: Node's `fetch` decompresses a `Content-Encoding` response +transparently but leaves the encoding and length describing the compressed +bytes, so both are dropped when the upstream response was encoded (an +uncompressed `206` keeps its `Content-Length`). Hop-by-hop headers +(`Connection`, `Keep-Alive`, `Transfer-Encoding`, …) are never relayed. + +The upstream request uses `redirect: "manual"`, because a streamed body is +single-use and cannot be replayed for a `307`/`308`; the `3xx` and its +`Location` are relayed to the client instead. The client's `AbortSignal` is +passed through, so cancelling an upload also aborts the upstream request. To forward extra application headers (e.g. idempotency keys or checksums), pass `forwardRequestHeaders`. Matching is case-insensitive. Security-sensitive headers are always excluded even if listed: `Cookie`, `Host`, `Authorization` (the proxy -sets its own bearer token), `Content-Length`, and `Connection`. +sets its own bearer token), and `Connection`. ```ts const handler = createBackendProxyHandler({ diff --git a/docs/TODO.md b/docs/TODO.md index ff6f8f1..0c636ab 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -13,3 +13,35 @@ were left untested (out of scope for the bug fix). Testing requires extracting the callback or mocking NextAuth, which is disproportionate for that spec. **Prerequisite:** None. + +## Let the backend reject an upload before the body flows (`Expect: 100-continue`) + +The proxy streams the request body and forwards `Content-Length`, so a backend +can answer `413` (or `400`/`412`) from the headers alone — but the bytes are +already on their way, and Tomcat has to drain the rest of the body to deliver +that status at all (`server.tomcat.max-swallow-size`). A rejected 500 MB upload +is therefore transferred in full to be discarded. + +`Expect: 100-continue` removes that: the request stops after the headers and the +body flows only after `100 Continue`. Browsers cannot use it (`Expect` is a +forbidden header for XHR/fetch), but the proxy leg can — it holds the client +body in backpressure, gets the header verdict, and forwards nothing on a +rejection. Two prerequisites: + +- **Node's `fetch` cannot request it.** `undici.request({ expectContinue: true })` + can, which means replacing `fetch` in `createBackendProxyHandler` with a direct + undici call — a real dependency and API change, not a flag. +- **The backend must not answer it itself.** Tomcat's `continueResponseTiming` + defaults to `IMMEDIATELY`, i.e. it sends `100 Continue` when the headers + arrive, before the application sees the request. It needs + `ON_REQUEST_BODY_READ` (a connector attribute, not a `server.tomcat.*` + property) so the application can reject first. + +**Context:** surfaced while reviewing the streaming proxy fix (#10 / #11) for +`open-transcript`, whose upload queue re-`PUT`s a recording after a lost +response and is answered `412` — today at the cost of re-uploading the whole +file. Deferred because it only pays off once a consumer measurably suffers from +it, and it touches both the library's HTTP client and the consumer's servlet +container. + +**Prerequisite:** #11 (streamed body, forwarded `Content-Length`). diff --git a/package.json b/package.json index 630d427..b6e655f 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,7 @@ "@open-elements/ui": "^0.9.0", "@testing-library/jest-dom": "^6.9.1", "@testing-library/react": "^16.3.2", + "@types/node": "^24.13.3", "@types/react": "^19.0.0", "@types/react-dom": "^19.0.0", "@vitejs/plugin-react": "^4.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f64149e..5803be3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ importers: '@testing-library/react': specifier: ^16.3.2 version: 16.3.2(@testing-library/dom@10.4.1)(@types/react-dom@19.2.3(@types/react@19.2.15))(@types/react@19.2.15)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) + '@types/node': + specifier: ^24.13.3 + version: 24.13.3 '@types/react': specifier: ^19.0.0 version: 19.2.15 @@ -28,7 +31,7 @@ importers: version: 19.2.3(@types/react@19.2.15) '@vitejs/plugin-react': specifier: ^4.0.0 - version: 4.7.0(vite@7.3.3) + version: 4.7.0(vite@7.3.3(@types/node@24.13.3)) eslint: specifier: ^9.0.0 version: 9.39.4 @@ -61,7 +64,7 @@ importers: version: 8.59.4(eslint@9.39.4)(typescript@5.9.3) vitest: specifier: ^4.1.2 - version: 4.1.7(jsdom@29.1.1)(vite@7.3.3) + version: 4.1.7(@types/node@24.13.3)(jsdom@29.1.1)(vite@7.3.3(@types/node@24.13.3)) packages: @@ -1798,6 +1801,9 @@ packages: '@types/mdurl@2.0.0': resolution: {integrity: sha512-RGdgjQUZba5p6QEFAVx2OGb8rQDL/cPRG7GiedRzMcJ1tYnUANBncjbSB1NRGwbvjcPeikRABz2nshyPk1bhWg==} + '@types/node@24.13.3': + resolution: {integrity: sha512-Dh8vAsV36ig5wa9OX4pXvMc9D3Veibfw2wix0CUwYODLD8nkj9UsLjASr49nPg+2eKzxhBV+v7L8pXvT4e639Q==} + '@types/react-dom@19.2.3': resolution: {integrity: sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==} peerDependencies: @@ -2113,6 +2119,7 @@ packages: eslint@9.39.4: resolution: {integrity: sha512-XoMjdBOwe/esVgEvLmNsD3IRHkm7fbKIUGvrleloJXUZgDHig2IPWNniv+GwjyJXzuNqVjlr5+4yVUZjycJwfQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} + deprecated: This version is no longer supported. Please see https://eslint.org/version-support for other options. hasBin: true peerDependencies: jiti: '*' @@ -2745,6 +2752,9 @@ packages: uc.micro@2.1.0: resolution: {integrity: sha512-ARDJmphmdvUk6Glw7y9DQ2bFkKBHwQHLi2lsaH6PPmz/Ka9sFOBsBluozhDltWmnv9u/cF6Rt87znRTPV+yp/A==} + undici-types@7.18.2: + resolution: {integrity: sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==} + undici@7.25.0: resolution: {integrity: sha512-xXnp4kTyor2Zq+J1FfPI6Eq3ew5h6Vl0F/8d9XU5zZQf1tX9s2Su1/3PiMmUANFULpmksxkClamIZcaUqryHsQ==} engines: {node: '>=20.18.1'} @@ -4536,6 +4546,10 @@ snapshots: '@types/mdurl@2.0.0': {} + '@types/node@24.13.3': + dependencies: + undici-types: 7.18.2 + '@types/react-dom@19.2.3(@types/react@19.2.15)': dependencies: '@types/react': 19.2.15 @@ -4637,7 +4651,7 @@ snapshots: '@typescript-eslint/types': 8.59.4 eslint-visitor-keys: 5.0.1 - '@vitejs/plugin-react@4.7.0(vite@7.3.3)': + '@vitejs/plugin-react@4.7.0(vite@7.3.3(@types/node@24.13.3))': dependencies: '@babel/core': 7.29.0 '@babel/plugin-transform-react-jsx-self': 7.27.1(@babel/core@7.29.0) @@ -4645,7 +4659,7 @@ snapshots: '@rolldown/pluginutils': 1.0.0-beta.27 '@types/babel__core': 7.20.5 react-refresh: 0.17.0 - vite: 7.3.3 + vite: 7.3.3(@types/node@24.13.3) transitivePeerDependencies: - supports-color @@ -4658,13 +4672,13 @@ snapshots: chai: 6.2.2 tinyrainbow: 3.1.0 - '@vitest/mocker@4.1.7(vite@7.3.3)': + '@vitest/mocker@4.1.7(vite@7.3.3(@types/node@24.13.3))': dependencies: '@vitest/spy': 4.1.7 estree-walker: 3.0.3 magic-string: 0.30.21 optionalDependencies: - vite: 7.3.3 + vite: 7.3.3(@types/node@24.13.3) '@vitest/pretty-format@4.1.7': dependencies: @@ -5592,6 +5606,8 @@ snapshots: uc.micro@2.1.0: {} + undici-types@7.18.2: {} + undici@7.25.0: {} update-browserslist-db@1.2.3(browserslist@4.28.2): @@ -5623,7 +5639,7 @@ snapshots: dependencies: react: 19.2.6 - vite@7.3.3: + vite@7.3.3(@types/node@24.13.3): dependencies: esbuild: 0.27.7 fdir: 6.5.0(picomatch@4.0.4) @@ -5632,12 +5648,13 @@ snapshots: rollup: 4.60.4 tinyglobby: 0.2.16 optionalDependencies: + '@types/node': 24.13.3 fsevents: 2.3.3 - vitest@4.1.7(jsdom@29.1.1)(vite@7.3.3): + vitest@4.1.7(@types/node@24.13.3)(jsdom@29.1.1)(vite@7.3.3(@types/node@24.13.3)): dependencies: '@vitest/expect': 4.1.7 - '@vitest/mocker': 4.1.7(vite@7.3.3) + '@vitest/mocker': 4.1.7(vite@7.3.3(@types/node@24.13.3)) '@vitest/pretty-format': 4.1.7 '@vitest/runner': 4.1.7 '@vitest/snapshot': 4.1.7 @@ -5654,9 +5671,10 @@ snapshots: tinyexec: 1.2.2 tinyglobby: 0.2.16 tinyrainbow: 3.1.0 - vite: 7.3.3 + vite: 7.3.3(@types/node@24.13.3) why-is-node-running: 2.3.0 optionalDependencies: + '@types/node': 24.13.3 jsdom: 29.1.1 transitivePeerDependencies: - msw diff --git a/src/server/__tests__/route-handlers.streaming.test.ts b/src/server/__tests__/route-handlers.streaming.test.ts new file mode 100644 index 0000000..682dd36 --- /dev/null +++ b/src/server/__tests__/route-handlers.streaming.test.ts @@ -0,0 +1,192 @@ +// @vitest-environment node +// +// Integration tests for the backend proxy against a real HTTP server and the +// real `fetch` of the runtime. The sibling suite mocks `fetch` and can only +// assert the *shape* of what the handler passes in — it cannot show that bytes +// actually move before the request body ends, nor what `fetch` does to an +// encoded response body. Both are properties of undici, so they need undici. +// +// This is the canary for a Node or Next upgrade breaking `duplex: "half"`. +import { describe, it, expect, afterAll, beforeAll } from "vitest"; +import http from "node:http"; +import { gzipSync } from "node:zlib"; +import type { AddressInfo } from "node:net"; +import type { NextRequest } from "next/server"; +import { createBackendProxyHandler } from "../route-handlers"; + +interface Received { + readonly method: string; + readonly url: string; + readonly headers: http.IncomingHttpHeaders; +} + +let server: http.Server; +let backendUrl: string; +/** Resolves as soon as the upstream server has seen the first body chunk. */ +let firstChunkSeen: Promise; +let announceFirstChunk: () => void; +let received: Received | undefined; + +beforeAll(async () => { + server = http.createServer((req, res) => { + received = { method: req.method!, url: req.url!, headers: req.headers }; + + if (req.url?.startsWith("/api/gzip")) { + const body = gzipSync(JSON.stringify({ hello: "world" })); + res.writeHead(200, { + "content-type": "application/json", + "content-encoding": "gzip", + "content-length": String(body.length), + etag: '"v1"', + }); + return res.end(body); + } + + let total = 0; + req.on("data", (chunk: Buffer) => { + total += chunk.length; + announceFirstChunk(); + }); + req.on("end", () => { + res.writeHead(201, { "content-type": "application/json" }); + res.end(JSON.stringify({ received: total })); + }); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + backendUrl = `http://127.0.0.1:${(server.address() as AddressInfo).port}`; +}); + +afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); +}); + +/** A request whose body is a stream, as Next.js hands it to a route handler. */ +function streamingRequest( + url: string, + body: ReadableStream, + headers: Record, +): NextRequest { + return new Request(url, { + method: "PUT", + headers, + body, + duplex: "half", + } as RequestInit & { duplex: "half" }) as unknown as NextRequest; +} + +function context(path: string[]) { + return { params: Promise.resolve({ path }) }; +} + +/** The request the upstream server last saw; fails loudly if there was none. */ +function lastUpstreamRequest(): Received { + if (!received) { + throw new Error("the upstream server recorded no request"); + } + return received; +} + +describe("createBackendProxyHandler (real fetch)", () => { + it("should deliver body chunks upstream before the request body is closed", async () => { + received = undefined; + firstChunkSeen = new Promise((resolve) => { + announceFirstChunk = resolve; + }); + + // The stream deliberately withholds its second chunk until the upstream + // server has confirmed the first one. A handler that buffers the body + // waits for a stream that is waiting for the handler: nothing arrives, and + // the assertion below fails on the timeout instead of hanging silently. + const chunk = new Uint8Array(8); + let emitted = 0; + const stream = new ReadableStream({ + async pull(controller) { + if (emitted === 0) { + emitted++; + return controller.enqueue(chunk); + } + if (emitted === 1) { + const streamed = await Promise.race([ + firstChunkSeen.then(() => true), + new Promise((resolve) => setTimeout(() => resolve(false), 2_000)), + ]); + expect(streamed).toBe(true); + emitted++; + return controller.enqueue(chunk); + } + controller.close(); + }, + }); + + const handler = createBackendProxyHandler({ backendUrl, auth: async () => null }); + const res = await handler( + streamingRequest( + "http://app.test/api/upload", + stream, + { "content-type": "audio/webm" }, // no content-length: length is unknown up front + ), + context(["upload"]), + ); + + expect(res.status).toBe(201); + expect(await res.json()).toEqual({ received: 16 }); + }); + + it("should forward a declared Content-Length to the backend for a streamed body", async () => { + received = undefined; + firstChunkSeen = new Promise((resolve) => { + announceFirstChunk = resolve; + }); + + const payload = new Uint8Array(24); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(payload); + controller.close(); + }, + }); + + const handler = createBackendProxyHandler({ backendUrl, auth: async () => null }); + const res = await handler( + streamingRequest("http://app.test/api/upload", stream, { + "content-type": "audio/webm", + "content-length": "24", + }), + context(["upload"]), + ); + + expect(res.status).toBe(201); + // The declared length reached the backend rather than being replaced by + // `Transfer-Encoding: chunked` — this is what lets a backend reject an + // over-sized upload before reading the body. + expect(lastUpstreamRequest().headers["content-length"]).toBe("24"); + expect(lastUpstreamRequest().headers["transfer-encoding"]).toBeUndefined(); + }); + + it("should relay a gzip-encoded response as decoded bytes without contradicting headers", async () => { + const handler = createBackendProxyHandler({ backendUrl, auth: async () => null }); + const res = await handler( + new Request("http://app.test/api/gzip") as unknown as NextRequest, + context(["gzip"]), + ); + + expect(res.status).toBe(200); + // `fetch` decoded the body, so the encoding headers must not survive. + expect(res.headers.get("content-encoding")).toBeNull(); + expect(res.headers.get("content-length")).toBeNull(); + expect(res.headers.get("etag")).toBe('"v1"'); + expect(await res.json()).toEqual({ hello: "world" }); + }); + + it("should not relay hop-by-hop headers from a chunked upstream response", async () => { + const handler = createBackendProxyHandler({ backendUrl, auth: async () => null }); + const res = await handler( + new Request("http://app.test/api/gzip") as unknown as NextRequest, + context(["gzip"]), + ); + + expect(res.headers.get("connection")).toBeNull(); + expect(res.headers.get("keep-alive")).toBeNull(); + expect(res.headers.get("transfer-encoding")).toBeNull(); + }); +}); diff --git a/src/server/__tests__/route-handlers.test.ts b/src/server/__tests__/route-handlers.test.ts index cce73c6..2b0c8a1 100644 --- a/src/server/__tests__/route-handlers.test.ts +++ b/src/server/__tests__/route-handlers.test.ts @@ -212,4 +212,139 @@ describe("createBackendProxyHandler", () => { const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; expect(headers.get("Authorization")).toBe("Bearer real-token"); }); + it("should forward Content-Length so the backend can reject over-sized uploads early", async () => { + const fetchMock = mockFetch(new Response(null, { status: 201 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + await handler( + makeRequest("http://app.test/api/upload", { + method: "PUT", + headers: { "Content-Type": "audio/webm", "Content-Length": "7" }, + body: "payload", + }), + context(["upload"]), + ); + + const headers = (fetchMock.mock.calls[0][1]! as RequestInit).headers as Headers; + // Without this the streamed request is framed as chunked and a backend + // asking for the declared length sees -1, disabling its fast rejection. + expect(headers.get("Content-Length")).toBe("7"); + }); + + it("should relay a 3xx instead of following it, because a streamed body cannot be replayed", async () => { + const fetchMock = mockFetch( + new Response(null, { status: 307, headers: { Location: "/api/elsewhere" } }), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const res = await handler( + makeRequest("http://app.test/api/upload", { method: "POST", body: "payload" }), + context(["upload"]), + ); + + const init = fetchMock.mock.calls[0][1]! as RequestInit; + expect(init.redirect).toBe("manual"); + expect(res.status).toBe(307); + expect(res.headers.get("Location")).toBe("/api/elsewhere"); + }); + + it("should pass the request abort signal to the upstream fetch", async () => { + const fetchMock = mockFetch(new Response(null, { status: 200 })); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const req = makeRequest("http://app.test/api/upload", { method: "POST", body: "payload" }); + await handler(req, context(["upload"])); + + const init = fetchMock.mock.calls[0][1]! as RequestInit; + expect(init.signal).toBe(req.signal); + }); + + it("should drop Content-Encoding and Content-Length when fetch decoded the response body", async () => { + // `fetch` decompresses transparently but leaves both headers describing the + // encoded bytes; relaying them yields ERR_CONTENT_DECODING_FAILED. + const fetchMock = mockFetch( + new Response("decoded-json", { + status: 200, + headers: { + "Content-Type": "application/json", + "Content-Encoding": "gzip", + "Content-Length": "49", + ETag: '"v1"', + }, + }), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const res = await handler(makeRequest("http://app.test/api/me"), context(["me"])); + + expect(res.headers.get("Content-Encoding")).toBeNull(); + expect(res.headers.get("Content-Length")).toBeNull(); + // Headers describing the resource itself survive. + expect(res.headers.get("Content-Type")).toBe("application/json"); + expect(res.headers.get("ETag")).toBe('"v1"'); + expect(await res.text()).toBe("decoded-json"); + }); + + it("should keep Content-Length on an uncompressed ranged response", async () => { + const fetchMock = mockFetch( + new Response("chunk", { + status: 206, + headers: { + "Content-Length": "5", + "Content-Range": "bytes 0-4/100", + "Accept-Ranges": "bytes", + }, + }), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const res = await handler( + makeRequest("http://app.test/api/audio.mp3", { headers: { Range: "bytes=0-4" } }), + context(["audio.mp3"]), + ); + + expect(res.status).toBe(206); + expect(res.headers.get("Content-Length")).toBe("5"); + expect(res.headers.get("Content-Range")).toBe("bytes 0-4/100"); + expect(res.headers.get("Accept-Ranges")).toBe("bytes"); + }); + + it("should not relay hop-by-hop response headers", async () => { + const fetchMock = mockFetch( + new Response("body", { + status: 200, + headers: { Connection: "keep-alive", "Keep-Alive": "timeout=5" }, + }), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const handler = createBackendProxyHandler({ + backendUrl: BACKEND_URL, + auth: async () => null, + }); + const res = await handler(makeRequest("http://app.test/api/me"), context(["me"])); + + expect(res.headers.get("Connection")).toBeNull(); + expect(res.headers.get("Keep-Alive")).toBeNull(); + }); }); diff --git a/src/server/route-handlers.ts b/src/server/route-handlers.ts index 0527800..c7dccdf 100644 --- a/src/server/route-handlers.ts +++ b/src/server/route-handlers.ts @@ -17,19 +17,27 @@ export interface BackendProxyConfig { * * Security-sensitive headers are always excluded regardless of this * list: `Cookie`, `Host`, `Authorization` (the proxy sets its own - * bearer token), and the hop-by-hop / fetch-managed headers - * `Content-Length` and `Connection`. + * bearer token) and the hop-by-hop header `Connection`. */ readonly forwardRequestHeaders?: readonly string[]; } /** - * Request headers that are always forwarded to the backend. In addition to - * `Content-Type` and `Accept` these include the conditional / range headers - * that browsers rely on for media seeking and caching. + * Request headers that are always forwarded to the backend. Besides + * `Content-Type` and `Accept` these are the conditional / range headers that + * browsers rely on for media seeking and caching, plus `Content-Length`. + * + * `Content-Length` matters because the body is *streamed*: without it `fetch` + * frames the upstream request with `Transfer-Encoding: chunked`, and a backend + * asking for the declared length (e.g. Servlet `getContentLengthLong()`) then + * gets `-1`. Backends use that value to reject an over-sized upload before + * reading a single byte, so dropping the header silently disables the check + * and turns a cheap rejection into a full transfer. The body is passed through + * byte for byte, so the incoming length stays accurate. */ const ALWAYS_FORWARDED_HEADERS = [ "content-type", + "content-length", "accept", "range", "if-range", @@ -42,16 +50,54 @@ const ALWAYS_FORWARDED_HEADERS = [ /** * Headers that must never be forwarded to the backend, even when listed in * `forwardRequestHeaders`. `Cookie` and `Host` would leak session state and - * confuse virtual-host routing; `Authorization` is set by the proxy itself; - * `Content-Length` and `Connection` are hop-by-hop / managed by `fetch`. + * confuse virtual-host routing, `Authorization` is set by the proxy itself, + * and `Connection` is hop-by-hop. */ -const NEVER_FORWARDED_HEADERS = new Set([ - "cookie", - "host", - "authorization", - "content-length", +const NEVER_FORWARDED_HEADERS = new Set(["cookie", "host", "authorization", "connection"]); + +/** + * Hop-by-hop response headers (RFC 9110 7.6.1). They describe the *upstream* + * connection, not the response, and must not be relayed to the client: a + * relayed `Transfer-Encoding: chunked` contradicts the framing Next.js applies + * to the response it actually sends. + */ +const HOP_BY_HOP_RESPONSE_HEADERS = [ "connection", -]); + "keep-alive", + "transfer-encoding", + "te", + "trailer", + "upgrade", + "proxy-authenticate", + "proxy-authorization", +]; + +/** + * The upstream response headers, corrected for what `fetch` did to the body. + * + * Node's `fetch` adds `Accept-Encoding: gzip, deflate` on its own and + * transparently *decodes* a compressed response, but leaves `Content-Encoding` + * and `Content-Length` describing the encoded bytes. Relaying them hands the + * client plain bytes labelled `gzip` and a length that does not match, which + * browsers report as `ERR_CONTENT_DECODING_FAILED`. Both are therefore dropped + * whenever the upstream response was encoded — and only then, so an + * uncompressed `206 Partial Content` keeps its `Content-Length`. + * + * Everything that describes the resource — `ETag`, `Accept-Ranges`, + * `Content-Range`, `Cache-Control`, `Content-Type`, `Content-Disposition` — is + * preserved untouched. + */ +function relayedResponseHeaders(upstream: Headers): Headers { + const headers = new Headers(upstream); + for (const name of HOP_BY_HOP_RESPONSE_HEADERS) { + headers.delete(name); + } + if (headers.has("content-encoding")) { + headers.delete("content-encoding"); + headers.delete("content-length"); + } + return headers; +} /** * Create a route handler that proxies an authenticated request to the @@ -103,6 +149,17 @@ export function createBackendProxyHandler(config: BackendProxyConfig): RouteHand const init: RequestInit & { duplex?: "half" } = { method: req.method, headers, + // A streamed body is single-use and cannot be replayed, so following a + // 307/308 — which must repeat the request with the same method and body — + // would fail after the stream has been consumed. Relay the 3xx and its + // `Location` to the client instead and let it decide. Unlike the browser + // Fetch spec, undici returns the real response here, not an opaque + // redirect with status 0. + redirect: "manual", + // Propagate cancellation: when the client aborts an upload, abort the + // upstream request too, so the backend stops reading and can roll back + // its partial write instead of draining a connection nobody is on. + signal: req.signal, }; if (hasBody && req.body) { init.body = req.body; @@ -113,7 +170,7 @@ export function createBackendProxyHandler(config: BackendProxyConfig): RouteHand return new Response(response.body, { status: response.status, - headers: response.headers, + headers: relayedResponseHeaders(response.headers), }); }; }