diff --git a/.changeset/tough-snakes-reflect.md b/.changeset/tough-snakes-reflect.md new file mode 100644 index 000000000000..7fdaca60397e --- /dev/null +++ b/.changeset/tough-snakes-reflect.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where Astro Actions couldn't redirect to the correct pathname when there was a rewrite involved. diff --git a/packages/astro/e2e/actions-blog.test.js b/packages/astro/e2e/actions-blog.test.js index b87384f9d8d2..4b76928cfeef 100644 --- a/packages/astro/e2e/actions-blog.test.js +++ b/packages/astro/e2e/actions-blog.test.js @@ -136,8 +136,7 @@ test.describe('Astro Actions - Blog', () => { await expect(page).toHaveURL(astro.resolveUrl('/blog/')); }); - // TODO: fix regression #12201 and #12202 - test.skip('Should redirect to the origin pathname when there is a rewrite', async ({ + test('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 bfa7047c87de..fcf3eb4aabed 100644 --- a/packages/astro/src/actions/runtime/middleware.ts +++ b/packages/astro/src/actions/runtime/middleware.ts @@ -1,6 +1,7 @@ import { yellow } from 'kleur/colors'; import type { APIContext, MiddlewareNext } from '../../@types/astro.js'; import { defineMiddleware } from '../../core/middleware/index.js'; +import { getOriginPathname } 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'; @@ -135,6 +136,11 @@ async function redirectWithResult({ return context.redirect(referer); } + const referer = getOriginPathname(context.request); + if (referer) { + return context.redirect(referer); + } + return context.redirect(context.url.pathname); } diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index ff80c4e14e27..b2a60f90ef77 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import type { IncomingMessage, ServerResponse } from 'node:http'; import { Http2ServerResponse } from 'node:http2'; import type { RouteData } from '../../@types/astro.js'; +import { clientAddressSymbol } from '../constants.js'; import { deserializeManifest } from './common.js'; import { createOutgoingHttpHeaders } from './createOutgoingHttpHeaders.js'; import { App } from './index.js'; @@ -10,8 +11,6 @@ import type { SSRManifest, SerializedSSRManifest } from './types.js'; export { apply as applyPolyfills } from '../polyfill.js'; -const clientAddressSymbol = Symbol.for('astro.clientAddress'); - /** * Allow the request body to be explicitly overridden. For example, this * is used by the Express JSON middleware. diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts index 274f86797077..6183e155721c 100644 --- a/packages/astro/src/core/constants.ts +++ b/packages/astro/src/core/constants.ts @@ -28,6 +28,7 @@ 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'; + export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes'; /** @@ -68,6 +69,11 @@ export const clientAddressSymbol = Symbol.for('astro.clientAddress'); */ export const clientLocalsSymbol = Symbol.for('astro.locals'); +/** + * Use this symbol to set and retrieve the original pathname of a request. This is useful when working with redirects and rewrites + */ +export const originPathnameSymbol = Symbol.for('astro.originPathname'); + /** * The symbol used as a field on the response object to keep track of streaming. * diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 02a0b46f6d0b..7ccbdd2ef564 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -36,6 +36,7 @@ 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, setOriginPathname } from './routing/rewrite.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes'); @@ -83,6 +84,7 @@ export class RenderContext { Pick >): Promise { const pipelineMiddleware = await pipeline.getMiddleware(); + setOriginPathname(request, pathname); return new RenderContext( pipeline, locals, @@ -156,7 +158,7 @@ export class RenderContext { if (payload instanceof Request) { this.request = payload; } else { - this.request = this.#copyRequest(newUrl, this.request); + this.request = copyRequest(newUrl, this.request); } this.isRewriting = true; this.url = new URL(this.request.url); @@ -256,7 +258,7 @@ export class RenderContext { if (reroutePayload instanceof Request) { this.request = reroutePayload; } else { - this.request = this.#copyRequest(newUrl, this.request); + this.request = copyRequest(newUrl, this.request); } this.url = new URL(this.request.url); this.cookies = new AstroCookies(this.request); @@ -570,33 +572,4 @@ 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 d50434f22096..e57fd49cd550 100644 --- a/packages/astro/src/core/routing/rewrite.ts +++ b/packages/astro/src/core/routing/rewrite.ts @@ -1,5 +1,7 @@ import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js'; import { shouldAppendForwardSlash } from '../build/util.js'; +import { originPathnameSymbol } 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'; @@ -70,3 +72,44 @@ 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 setOriginPathname(request: Request, pathname: string): void { + Reflect.set(request, originPathnameSymbol, encodeURIComponent(pathname)); +} + +export function getOriginPathname(request: Request): string | undefined { + const origin = Reflect.get(request, originPathnameSymbol); + if (origin) { + return decodeURIComponent(origin); + } + return undefined; +}