fix(routing): actions should redirect the original pathname (#12173) (#…

…12235)

Co-authored-by: Bjorn Lu <[email protected]>

.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.

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,
 	}) => {

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);
 }
 

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.

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.
  *

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<RenderContext, 'locals' | 'middleware' | 'status' | 'props' | 'partial'>
 		>): Promise<RenderContext> {
 		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',
-		});
-	}
 }

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;
+}