fix(frontend): strip hop-by-hop headers from proxied responses too
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.90.4"
|
version = "0.90.5"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.90.4"
|
version = "0.90.5"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
default-run = "mangalord"
|
default-run = "mangalord"
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.90.4",
|
"version": "0.90.5",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
@@ -7,7 +7,11 @@ import {
|
|||||||
afterEach,
|
afterEach,
|
||||||
type MockInstance
|
type MockInstance
|
||||||
} from 'vitest';
|
} 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
|
// `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'`
|
// 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');
|
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 () => {
|
it('aborts and returns 502 when the upstream stalls past the timeout', async () => {
|
||||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||||
// Simulate an aborted fetch (AbortController.abort() raises a
|
// Simulate an aborted fetch (AbortController.abort() raises a
|
||||||
@@ -301,3 +329,25 @@ describe('shouldBypassProxyTimeout', () => {
|
|||||||
expect(shouldBypassProxyTimeout(new Headers())).toBe(false);
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -32,6 +32,21 @@ const HOP_BY_HOP_HEADERS = [
|
|||||||
'upgrade'
|
'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
|
* 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
|
* 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/')) {
|
if (event.url.pathname.startsWith('/api/')) {
|
||||||
const target = `${BACKEND_URL}${event.url.pathname}${event.url.search}`;
|
const target = `${BACKEND_URL}${event.url.pathname}${event.url.search}`;
|
||||||
|
|
||||||
const headers = new Headers(event.request.headers);
|
const headers = stripHopByHopHeaders(event.request.headers);
|
||||||
for (const h of HOP_BY_HOP_HEADERS) headers.delete(h);
|
|
||||||
|
|
||||||
// AbortController times the upstream fetch out so a backend
|
// AbortController times the upstream fetch out so a backend
|
||||||
// wedged on a slow DB query doesn't keep the browser request
|
// 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, {
|
return new Response(upstream.body, {
|
||||||
status: upstream.status,
|
status: upstream.status,
|
||||||
statusText: upstream.statusText,
|
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);
|
return resolve(event);
|
||||||
|
|||||||
Reference in New Issue
Block a user