From e216250146fbff746efd542612ce9bae6db9601f Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 5 Dec 2024 13:18:00 +0000 Subject: [PATCH] fix(ssr): duplicate custom headers (#12518) * fix(ssr): duplicate custom headers * rebase --- .changeset/proud-wombats-mate.md | 5 ++++ packages/astro/src/core/app/index.ts | 14 +++++++--- .../test/fixtures/ssr-error/package.json | 8 ++++++ .../ssr-error/src/pages/[...slug].astro | 8 ++++++ .../fixtures/ssr-error/src/pages/index.astro | 6 ++++ packages/astro/test/ssr-error-pages.test.js | 28 +++++++++++++++++++ pnpm-lock.yaml | 6 ++++ 7 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 .changeset/proud-wombats-mate.md create mode 100644 packages/astro/test/fixtures/ssr-error/package.json create mode 100644 packages/astro/test/fixtures/ssr-error/src/pages/[...slug].astro create mode 100644 packages/astro/test/fixtures/ssr-error/src/pages/index.astro diff --git a/.changeset/proud-wombats-mate.md b/.changeset/proud-wombats-mate.md new file mode 100644 index 000000000000..2b80d3a7821e --- /dev/null +++ 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 index 0e776ae9f2c6..26a6523d8ec9 100644 --- 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 new file mode 100644 index 000000000000..ee13c03790c2 --- /dev/null +++ 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 new file mode 100644 index 000000000000..1fbb67e0c776 --- /dev/null +++ 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 new file mode 100644 index 000000000000..9509e01e50f3 --- /dev/null +++ b/packages/astro/test/fixtures/ssr-error/src/pages/index.astro @@ -0,0 +1,6 @@ + +Hello world + +

Hello world

+ + diff --git a/packages/astro/test/ssr-error-pages.test.js b/packages/astro/test/ssr-error-pages.test.js index f62c507b53d8..9ebc58770d3e 100644 --- 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 index f87bd6cd9e0d..e2daaaf64341 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3825,6 +3825,12 @@ importers: specifier: ^10.25.0 version: 10.25.0 + packages/astro/test/fixtures/ssr-error: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/ssr-error-pages: dependencies: astro: