fix(ssr): duplicate custom headers (#12518)

* fix(ssr): duplicate custom headers * rebase
withastro · Dec 5, 2024 · e216250 · e216250
1 parent d14d967
commit e216250
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 4 deletions.
diff --git a/.changeset/proud-wombats-mate.md b/.changeset/proud-wombats-mate.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes an issue where SSR error pages would return duplicated custom headers.
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -439,6 +439,15 @@ export class App {
 			// this function could throw an error...
 			originalResponse.headers.delete('Content-type');
 		} catch {}
+		// we use a map to remove duplicates
+		const mergedHeaders = new Map([
+			...Array.from(newResponse.headers),
+			...Array.from(originalResponse.headers),
+		]);
+		const newHeaders = new Headers();
+		for (const [name, value] of mergedHeaders) {
+			newHeaders.set(name, value);
+		}
 		return new Response(newResponse.body, {
 			status,
 			statusText: status === 200 ? newResponse.statusText : originalResponse.statusText,
@@ -447,10 +456,7 @@ export class App {
 			// If users see something weird, it's because they are setting some headers they should not.
 			//
 			// Although, we don't want it to replace the content-type, because the error page must return `text/html`
-			headers: new Headers([
-				...Array.from(newResponse.headers),
-				...Array.from(originalResponse.headers),
-			]),
+			headers: newHeaders,
 		});
 	}
 

diff --git a/packages/astro/test/fixtures/ssr-error/package.json b/packages/astro/test/fixtures/ssr-error/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/ssr",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}
diff --git a/packages/astro/test/fixtures/ssr-error/src/pages/[...slug].astro b/packages/astro/test/fixtures/ssr-error/src/pages/[...slug].astro
@@ -0,0 +1,8 @@
+---
+return new Response("oops", {
+	status: 500,
+	headers: new Headers({
+		"X-Debug": "1234",
+	}),
+});
+---
diff --git a/packages/astro/test/fixtures/ssr-error/src/pages/index.astro b/packages/astro/test/fixtures/ssr-error/src/pages/index.astro
@@ -0,0 +1,6 @@
+<html>
+<head><title>Hello world</title></head>
+<body>
+<h1>Hello world</h1>
+</body>
+</html>
diff --git a/packages/astro/test/ssr-error-pages.test.js b/packages/astro/test/ssr-error-pages.test.js
@@ -4,6 +4,34 @@ import * as cheerio from 'cheerio';
 import testAdapter from './test-adapter.js';
 import { loadFixture } from './test-utils.js';
 
+describe('Default 500 page', () => {
+	/** @type {import('./test-utils.js').Fixture} */
+	let fixture;
+	/** @type {import('./test-utils.js').App} */
+	let app;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/ssr-error/',
+			output: 'server',
+			adapter: testAdapter(),
+			// test suite was authored when inlineStylesheets defaulted to never
+			build: { inlineStylesheets: 'never' },
+		});
+		await fixture.build({});
+		app = await fixture.loadTestAdapterApp();
+	});
+
+	it('should correctly merge headers coming from the original response and the 500 response, when calling a catch-all route', async () => {
+		const request = new Request('http://example.com/any');
+		const response = await app.render(request);
+		assert.equal(response.status, 500);
+		assert.equal(response.headers.get('x-debug'), '1234');
+		const html = await response.text();
+		assert.match(html, /oops/);
+	});
+});
+
 describe('404 and 500 pages', () => {
 	/** @type {import('./test-utils.js').Fixture} */
 	let fixture;

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml