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 d9c1bc1dfd68..4b76928cfeef 100644 --- a/packages/astro/e2e/actions-blog.test.js +++ b/packages/astro/e2e/actions-blog.test.js @@ -135,4 +135,16 @@ test.describe('Astro Actions - Blog', () => { await logoutButton.click(); await expect(page).toHaveURL(astro.resolveUrl('/blog/')); }); + + test('Should redirect to the origin pathname when there is a rewrite', async ({ + page, + astro, + }) => { + await page.goto(astro.resolveUrl('/sum')); + const submitButton = page.getByTestId('submit'); + await submitButton.click(); + await expect(page).toHaveURL(astro.resolveUrl('/sum')); + const p = page.locator('p').nth(0); + await expect(p).toContainText('Form result: {"data":3}'); + }); }); diff --git a/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts b/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts index 43ffb43d42d4..a5437ad49a3c 100644 --- a/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts +++ b/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts @@ -56,4 +56,14 @@ export const server = { }, }), }, + sum: defineAction({ + accept: "form", + input: z.object({ + a: z.number(), + b: z.number(), + }), + async handler({ a, b }) { + return a + b + }, + }) }; diff --git a/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts b/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts new file mode 100644 index 000000000000..53bb8235ac2a --- /dev/null +++ b/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts @@ -0,0 +1,9 @@ +import { defineMiddleware } from "astro:middleware"; + +export const onRequest = defineMiddleware((ctx, next) => { + if (ctx.request.method === "GET" && ctx.url.pathname === "/sum") { + return next("/rewritten") + } + + return next() +}) diff --git a/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro b/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro new file mode 100644 index 000000000000..72eebf1bb006 --- /dev/null +++ b/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro @@ -0,0 +1,18 @@ +--- +import { actions } from "astro:actions"; + +const result = Astro.getActionResult(actions.sum) + +--- + + + +
+ + + +
+

Form result: {JSON.stringify(result)}

+ + + diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts index a61e1652c8b7..fcd42b799994 100644 --- a/packages/astro/src/actions/runtime/middleware.ts +++ b/packages/astro/src/actions/runtime/middleware.ts @@ -1,5 +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 { ACTION_QUERY_PARAMS } from '../consts.js'; import { formContentTypes, hasContentType } from './utils.js'; @@ -9,6 +10,7 @@ import { type SerializedActionResult, serializeActionResult, } from './virtual/shared.js'; +import {getOriginHeader} from "../../core/routing/rewrite.js"; export type ActionPayload = { actionResult: SerializedActionResult; @@ -32,7 +34,7 @@ export const onRequest = defineMiddleware(async (context, next) => { const locals = context.locals as Locals; // Actions middleware may have run already after a path rewrite. - // See https://github.com/withastro/roadmap/blob/feat/reroute/proposals/0047-rerouting.md#ctxrewrite + // See https://github.com/withastro/roadmap/blob/main/proposals/0048-rerouting.md#ctxrewrite // `_actionPayload` is the same for every page, // so short circuit if already defined. if (locals._actionPayload) return next(); @@ -135,6 +137,11 @@ 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 274f86797077..3fb937ac484c 100644 --- a/packages/astro/src/core/constants.ts +++ b/packages/astro/src/core/constants.ts @@ -28,6 +28,11 @@ 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 03ae7d3a3503..1e4c92f123d6 100644 --- a/packages/astro/src/core/middleware/sequence.ts +++ b/packages/astro/src/core/middleware/sequence.ts @@ -2,6 +2,7 @@ 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 @@ -36,9 +37,9 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler { if (payload instanceof Request) { newRequest = payload; } else if (payload instanceof URL) { - newRequest = new Request(payload, handleContext.request); + newRequest = copyRequest(payload, handleContext.request); } else { - newRequest = new Request( + newRequest = copyRequest( 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 8850da132e5a..a79c01922672 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -20,6 +20,7 @@ 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, @@ -36,6 +37,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, setOriginHeader} from './routing/rewrite.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes'); @@ -81,6 +83,7 @@ export class RenderContext { Pick >): Promise { const pipelineMiddleware = await pipeline.getMiddleware(); + setOriginHeader(request, pathname) return new RenderContext( pipeline, locals, @@ -153,7 +156,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); @@ -253,7 +256,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); @@ -561,33 +564,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..29d84ce746aa 100644 --- a/packages/astro/src/core/routing/rewrite.ts +++ b/packages/astro/src/core/routing/rewrite.ts @@ -1,7 +1,9 @@ import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js'; import { shouldAppendForwardSlash } from '../build/util.js'; +import { AstroError, AstroErrorData } from '../errors/index.js'; import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js'; import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js'; +import {ASTRO_ORIGIN_HEADER} from "../constants.js"; export type FindRouteToRewrite = { payload: RewritePayload; @@ -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 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 +}