From 12b00225067445629e5ae451d763d03f70065f88 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Sat, 12 Oct 2024 22:09:12 +0800 Subject: [PATCH] Revert #12173 (#12206) --- .changeset/gold-masks-trade.md | 5 +++ packages/astro/e2e/actions-blog.test.js | 3 +- .../astro/src/actions/runtime/middleware.ts | 7 --- packages/astro/src/core/constants.ts | 5 --- .../astro/src/core/middleware/sequence.ts | 5 +-- packages/astro/src/core/render-context.ts | 36 +++++++++++++--- packages/astro/src/core/routing/rewrite.ts | 43 ------------------- 7 files changed, 40 insertions(+), 64 deletions(-) create mode 100644 .changeset/gold-masks-trade.md diff --git a/.changeset/gold-masks-trade.md b/.changeset/gold-masks-trade.md new file mode 100644 index 0000000000..ce4fdd4625 --- /dev/null +++ b/.changeset/gold-masks-trade.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Reverts https://github.com/withastro/astro/pull/12173 which caused `Can't modify immutable headers` warnings and 500 errors on Cloudflare Pages diff --git a/packages/astro/e2e/actions-blog.test.js b/packages/astro/e2e/actions-blog.test.js index 4b76928cfe..b87384f9d8 100644 --- a/packages/astro/e2e/actions-blog.test.js +++ b/packages/astro/e2e/actions-blog.test.js @@ -136,7 +136,8 @@ test.describe('Astro Actions - Blog', () => { await expect(page).toHaveURL(astro.resolveUrl('/blog/')); }); - test('Should redirect to the origin pathname when there is a rewrite', async ({ + // TODO: fix regression #12201 and #12202 + test.skip('Should redirect to the origin pathname when there is a rewrite', async ({ page, astro, }) => { diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts index e5dd794b93..bfa7047c87 100644 --- a/packages/astro/src/actions/runtime/middleware.ts +++ b/packages/astro/src/actions/runtime/middleware.ts @@ -1,8 +1,6 @@ import { yellow } from 'kleur/colors'; import type { APIContext, MiddlewareNext } from '../../@types/astro.js'; -import { ASTRO_ORIGIN_HEADER } from '../../core/constants.js'; import { defineMiddleware } from '../../core/middleware/index.js'; -import { getOriginHeader } from '../../core/routing/rewrite.js'; import { ACTION_QUERY_PARAMS } from '../consts.js'; import { formContentTypes, hasContentType } from './utils.js'; import { getAction } from './virtual/get-action.js'; @@ -137,11 +135,6 @@ async function redirectWithResult({ return context.redirect(referer); } - const referer = getOriginHeader(context.request); - if (referer) { - return context.redirect(referer); - } - return context.redirect(context.url.pathname); } diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts index 3fb937ac48..274f867970 100644 --- a/packages/astro/src/core/constants.ts +++ b/packages/astro/src/core/constants.ts @@ -28,11 +28,6 @@ export const REROUTE_DIRECTIVE_HEADER = 'X-Astro-Reroute'; * This metadata is used to determine the origin of a Response. If a rewrite has occurred, it should be prioritised over other logic. */ export const REWRITE_DIRECTIVE_HEADER_KEY = 'X-Astro-Rewrite'; - -/** - * Header used to track the original URL requested by the user. This information is useful rewrites are involved. - */ -export const ASTRO_ORIGIN_HEADER = 'X-Astro-Origin'; export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes'; /** diff --git a/packages/astro/src/core/middleware/sequence.ts b/packages/astro/src/core/middleware/sequence.ts index 1e4c92f123..03ae7d3a35 100644 --- a/packages/astro/src/core/middleware/sequence.ts +++ b/packages/astro/src/core/middleware/sequence.ts @@ -2,7 +2,6 @@ import type { APIContext, MiddlewareHandler, RewritePayload } from '../../@types import { AstroCookies } from '../cookies/cookies.js'; import { apiContextRoutesSymbol } from '../render-context.js'; import { type Pipeline, getParams } from '../render/index.js'; -import { copyRequest } from '../routing/rewrite.js'; import { defineMiddleware } from './index.js'; // From SvelteKit: https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/hooks/sequence.js @@ -37,9 +36,9 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler { if (payload instanceof Request) { newRequest = payload; } else if (payload instanceof URL) { - newRequest = copyRequest(payload, handleContext.request); + newRequest = new Request(payload, handleContext.request); } else { - newRequest = copyRequest( + newRequest = new Request( new URL(payload, handleContext.url.origin), handleContext.request, ); diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 9f416c7bc2..8850da132e 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -20,7 +20,6 @@ import { import { renderEndpoint } from '../runtime/server/endpoint.js'; import { renderPage } from '../runtime/server/index.js'; import { - ASTRO_ORIGIN_HEADER, ASTRO_VERSION, REROUTE_DIRECTIVE_HEADER, REWRITE_DIRECTIVE_HEADER_KEY, @@ -37,7 +36,6 @@ import { callMiddleware } from './middleware/callMiddleware.js'; import { sequence } from './middleware/index.js'; import { renderRedirect } from './redirects/render.js'; import { type Pipeline, Slots, getParams, getProps } from './render/index.js'; -import { copyRequest, setOriginHeader } from './routing/rewrite.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes'); @@ -83,7 +81,6 @@ export class RenderContext { Pick >): Promise { const pipelineMiddleware = await pipeline.getMiddleware(); - setOriginHeader(request, pathname); return new RenderContext( pipeline, locals, @@ -156,7 +153,7 @@ export class RenderContext { if (payload instanceof Request) { this.request = payload; } else { - this.request = copyRequest(newUrl, this.request); + this.request = this.#copyRequest(newUrl, this.request); } this.isRewriting = true; this.url = new URL(this.request.url); @@ -256,7 +253,7 @@ export class RenderContext { if (reroutePayload instanceof Request) { this.request = reroutePayload; } else { - this.request = copyRequest(newUrl, this.request); + this.request = this.#copyRequest(newUrl, this.request); } this.url = new URL(this.request.url); this.cookies = new AstroCookies(this.request); @@ -564,4 +561,33 @@ export class RenderContext { if (!i18n) return; return (this.#preferredLocaleList ??= computePreferredLocaleList(request, i18n.locales)); } + + /** + * Utility function that creates a new `Request` with a new URL from an old `Request`. + * + * @param newUrl The new `URL` + * @param oldRequest The old `Request` + */ + #copyRequest(newUrl: URL, oldRequest: Request): Request { + if (oldRequest.bodyUsed) { + throw new AstroError(AstroErrorData.RewriteWithBodyUsed); + } + return new Request(newUrl, { + method: oldRequest.method, + headers: oldRequest.headers, + body: oldRequest.body, + referrer: oldRequest.referrer, + referrerPolicy: oldRequest.referrerPolicy, + mode: oldRequest.mode, + credentials: oldRequest.credentials, + cache: oldRequest.cache, + redirect: oldRequest.redirect, + integrity: oldRequest.integrity, + signal: oldRequest.signal, + keepalive: oldRequest.keepalive, + // https://fetch.spec.whatwg.org/#dom-request-duplex + // @ts-expect-error It isn't part of the types, but undici accepts it and it allows to carry over the body to a new request + duplex: 'half', + }); + } } diff --git a/packages/astro/src/core/routing/rewrite.ts b/packages/astro/src/core/routing/rewrite.ts index 493f25043c..d50434f220 100644 --- a/packages/astro/src/core/routing/rewrite.ts +++ b/packages/astro/src/core/routing/rewrite.ts @@ -1,7 +1,5 @@ import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js'; import { shouldAppendForwardSlash } from '../build/util.js'; -import { ASTRO_ORIGIN_HEADER } from '../constants.js'; -import { AstroError, AstroErrorData } from '../errors/index.js'; import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js'; import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js'; @@ -72,44 +70,3 @@ export function findRouteToRewrite({ } } } - -/** - * Utility function that creates a new `Request` with a new URL from an old `Request`. - * - * @param newUrl The new `URL` - * @param oldRequest The old `Request` - */ -export function copyRequest(newUrl: URL, oldRequest: Request): Request { - if (oldRequest.bodyUsed) { - throw new AstroError(AstroErrorData.RewriteWithBodyUsed); - } - return new Request(newUrl, { - method: oldRequest.method, - headers: oldRequest.headers, - body: oldRequest.body, - referrer: oldRequest.referrer, - referrerPolicy: oldRequest.referrerPolicy, - mode: oldRequest.mode, - credentials: oldRequest.credentials, - cache: oldRequest.cache, - redirect: oldRequest.redirect, - integrity: oldRequest.integrity, - signal: oldRequest.signal, - keepalive: oldRequest.keepalive, - // https://fetch.spec.whatwg.org/#dom-request-duplex - // @ts-expect-error It isn't part of the types, but undici accepts it and it allows to carry over the body to a new request - duplex: 'half', - }); -} - -export function setOriginHeader(request: Request, pathname: string): void { - request.headers.set(ASTRO_ORIGIN_HEADER, encodeURIComponent(pathname)); -} - -export function getOriginHeader(request: Request): string | undefined { - const origin = request.headers.get(ASTRO_ORIGIN_HEADER); - if (origin) { - return decodeURIComponent(origin); - } - return undefined; -}