From 4306d1c96aad97e205545a7ba489ec9b5390c848 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 1 Jul 2026 07:17:32 +0200 Subject: [PATCH] fix(frontend): strip hop-by-hop headers from proxied responses too Co-Authored-By: Claude Opus 4.8 --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- frontend/package.json | 2 +- frontend/src/hooks.server.test.ts | 52 ++++++++++++++++++++++++++++++- frontend/src/hooks.server.ts | 23 ++++++++++++-- 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 8d202e5..5dc3953 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.90.4" +version = "0.90.5" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 30cbe98..10ddecd 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.90.4" +version = "0.90.5" edition = "2021" default-run = "mangalord" diff --git a/frontend/package.json b/frontend/package.json index dd8f9a0..29cbfac 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.90.4", + "version": "0.90.5", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/hooks.server.test.ts b/frontend/src/hooks.server.test.ts index a2c7b9d..97137c5 100644 --- a/frontend/src/hooks.server.test.ts +++ b/frontend/src/hooks.server.test.ts @@ -7,7 +7,11 @@ import { afterEach, type MockInstance } from 'vitest'; -import { handle, shouldBypassProxyTimeout } from './hooks.server'; +import { + handle, + shouldBypassProxyTimeout, + stripHopByHopHeaders +} from './hooks.server'; // `BACKEND_URL` is read at module load time, so the values used in the // asserts below assume the test env didn't set it. `?? 'http://localhost:8080'` @@ -173,6 +177,30 @@ describe('hooks.server proxy', () => { expect(headers.get('x-custom')).toBe('pass-through'); }); + it('strips hop-by-hop headers from the proxied RESPONSE', async () => { + // The upstream's connection-scoped headers must not ride along on + // the re-streamed response (RFC 7230 §6.1). A normal header passes. + fetchSpy.mockResolvedValueOnce( + new Response('[]', { + status: 200, + headers: { + connection: 'keep-alive', + 'keep-alive': 'timeout=5', + 'transfer-encoding': 'chunked', + upgrade: 'h2c', + 'content-type': 'application/json' + } + }) + ); + const resolve = vi.fn(); + const resp = await handle({ event: makeEvent('/api/v1/health'), resolve }); + for (const h of ['connection', 'keep-alive', 'transfer-encoding', 'upgrade']) { + expect(resp.headers.get(h), `${h} should be stripped from response`).toBeNull(); + } + // A normal response header survives. + expect(resp.headers.get('content-type')).toContain('application/json'); + }); + it('aborts and returns 502 when the upstream stalls past the timeout', async () => { const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); // Simulate an aborted fetch (AbortController.abort() raises a @@ -301,3 +329,25 @@ describe('shouldBypassProxyTimeout', () => { expect(shouldBypassProxyTimeout(new Headers())).toBe(false); }); }); + +describe('stripHopByHopHeaders', () => { + it('removes hop-by-hop headers and keeps the rest', () => { + const src = new Headers({ + connection: 'keep-alive', + 'transfer-encoding': 'chunked', + 'content-type': 'application/json', + 'x-request-id': 'abc' + }); + const out = stripHopByHopHeaders(src); + expect(out.get('connection')).toBeNull(); + expect(out.get('transfer-encoding')).toBeNull(); + expect(out.get('content-type')).toBe('application/json'); + expect(out.get('x-request-id')).toBe('abc'); + }); + + it('does not mutate the source headers', () => { + const src = new Headers({ connection: 'close' }); + stripHopByHopHeaders(src); + expect(src.get('connection')).toBe('close'); + }); +}); diff --git a/frontend/src/hooks.server.ts b/frontend/src/hooks.server.ts index fc69cbb..2ea07e2 100644 --- a/frontend/src/hooks.server.ts +++ b/frontend/src/hooks.server.ts @@ -32,6 +32,21 @@ const HOP_BY_HOP_HEADERS = [ 'upgrade' ]; +/** + * Return a copy of `src` with the hop-by-hop headers removed. Applied to + * BOTH the proxied request and the proxied response: per RFC 7230 §6.1 a + * proxy must not forward connection-scoped headers in either direction. + * On the response side this also drops the upstream `content-length` / + * `transfer-encoding`, which the runtime recomputes for the re-streamed + * body — forwarding the upstream values risks a framing mismatch. + * Exported for unit-test coverage. + */ +export function stripHopByHopHeaders(src: Headers): Headers { + const out = new Headers(src); + for (const h of HOP_BY_HOP_HEADERS) out.delete(h); + return out; +} + /** * Cap each proxied request at 5 minutes. The bound exists to surface * a wedged backend (stuck on a slow DB query, deadlocked, etc.) as a @@ -74,8 +89,7 @@ export const handle: Handle = async ({ event, resolve }) => { if (event.url.pathname.startsWith('/api/')) { const target = `${BACKEND_URL}${event.url.pathname}${event.url.search}`; - const headers = new Headers(event.request.headers); - for (const h of HOP_BY_HOP_HEADERS) headers.delete(h); + const headers = stripHopByHopHeaders(event.request.headers); // AbortController times the upstream fetch out so a backend // wedged on a slow DB query doesn't keep the browser request @@ -148,7 +162,10 @@ export const handle: Handle = async ({ event, resolve }) => { return new Response(upstream.body, { status: upstream.status, statusText: upstream.statusText, - headers: upstream.headers + // Strip hop-by-hop headers on the way back too — the upstream's + // transfer-encoding / content-length / connection headers must + // not ride along on the re-streamed response. + headers: stripHopByHopHeaders(upstream.headers) }); } return resolve(event);