From 8c0cae6d1bd70b332286d83d0f01cfce5272fbbe Mon Sep 17 00:00:00 2001
From: Emanuele Stoppa <my.burning@gmail.com>
Date: Thu, 19 Sep 2024 13:57:47 +0100
Subject: [PATCH] fix(routing): compute `APIContext.params` when rewriting
 using `next` (#12031)

* fix(routing): compute `APIContext.params` when rewriting using `next`

* fix(routing): compute `APIContext.params` when rewriting using `next`
---
 .changeset/chatty-worms-sit.md                |  5 +++
 packages/astro/src/core/app/pipeline.ts       |  6 +---
 packages/astro/src/core/base-pipeline.ts      |  7 +----
 packages/astro/src/core/build/pipeline.ts     | 10 +++---
 .../astro/src/core/middleware/sequence.ts     | 11 +++++--
 packages/astro/src/core/render-context.ts     | 31 ++++++++++++-------
 .../src/vite-plugin-astro-server/pipeline.ts  |  6 +---
 .../test/fixtures/reroute/src/middleware.js   | 12 +++++--
 .../reroute/src/pages/auth/[id].astro         | 23 ++++++++++++++
 .../reroute/src/pages/auth/astro-params.astro |  3 ++
 .../reroute/src/pages/auth/params.astro       |  4 +++
 packages/astro/test/rewrite.test.js           |  9 ++++++
 12 files changed, 90 insertions(+), 37 deletions(-)
 create mode 100644 .changeset/chatty-worms-sit.md
 create mode 100644 packages/astro/test/fixtures/reroute/src/pages/auth/[id].astro
 create mode 100644 packages/astro/test/fixtures/reroute/src/pages/auth/astro-params.astro

diff --git a/.changeset/chatty-worms-sit.md b/.changeset/chatty-worms-sit.md
new file mode 100644
index 0000000000..64f0676011
--- /dev/null
+++ b/.changeset/chatty-worms-sit.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug where the rewrite via `next(/*..*/)` inside a middleware didn't compute the new `APIContext.params`
diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts
index 5ae5b775dd..4f753419a0 100644
--- a/packages/astro/src/core/app/pipeline.ts
+++ b/packages/astro/src/core/app/pipeline.ts
@@ -90,11 +90,7 @@ export class AppPipeline extends Pipeline {
 		return module.page();
 	}
 
-	async tryRewrite(
-		payload: RewritePayload,
-		request: Request,
-		_sourceRoute: RouteData,
-	): Promise<TryRewriteResult> {
+	async tryRewrite(payload: RewritePayload, request: Request): Promise<TryRewriteResult> {
 		const { newUrl, pathname, routeData } = findRouteToRewrite({
 			payload,
 			request,
diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
index 2281c562dc..6742a84255 100644
--- a/packages/astro/src/core/base-pipeline.ts
+++ b/packages/astro/src/core/base-pipeline.ts
@@ -89,13 +89,8 @@ export abstract class Pipeline {
 	 *
 	 * @param {RewritePayload} rewritePayload The payload provided by the user
 	 * @param {Request} request The original request
-	 * @param {RouteData} sourceRoute The original `RouteData`
 	 */
-	abstract tryRewrite(
-		rewritePayload: RewritePayload,
-		request: Request,
-		sourceRoute: RouteData,
-	): Promise<TryRewriteResult>;
+	abstract tryRewrite(rewritePayload: RewritePayload, request: Request): Promise<TryRewriteResult>;
 
 	/**
 	 * Tells the pipeline how to retrieve a component give a `RouteData`
diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts
index 414144359a..760c353d1d 100644
--- a/packages/astro/src/core/build/pipeline.ts
+++ b/packages/astro/src/core/build/pipeline.ts
@@ -92,6 +92,10 @@ export class BuildPipeline extends Pipeline {
 		);
 	}
 
+	getRoutes(): RouteData[] {
+		return this.options.manifest.routes;
+	}
+
 	static create({
 		internals,
 		manifest,
@@ -286,11 +290,7 @@ export class BuildPipeline extends Pipeline {
 		return module.page();
 	}
 
-	async tryRewrite(
-		payload: RewritePayload,
-		request: Request,
-		_sourceRoute: RouteData,
-	): Promise<TryRewriteResult> {
+	async tryRewrite(payload: RewritePayload, request: Request): Promise<TryRewriteResult> {
 		const { routeData, pathname, newUrl } = findRouteToRewrite({
 			payload,
 			request,
diff --git a/packages/astro/src/core/middleware/sequence.ts b/packages/astro/src/core/middleware/sequence.ts
index ee08381e6f..5d79c7fede 100644
--- a/packages/astro/src/core/middleware/sequence.ts
+++ b/packages/astro/src/core/middleware/sequence.ts
@@ -1,6 +1,8 @@
 import type { APIContext, MiddlewareHandler, RewritePayload } from '../../@types/astro.js';
 import { AstroCookies } from '../cookies/cookies.js';
 import { defineMiddleware } from './index.js';
+import { getParams, type Pipeline } from '../render/index.js';
+import { apiContextRoutesSymbol } from '../render-context.js';
 
 // From SvelteKit: https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/hooks/sequence.js
 /**
@@ -15,7 +17,6 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 			return next();
 		});
 	}
-
 	return defineMiddleware((context, next) => {
 		/**
 		 * This variable is used to carry the rerouting payload across middleware functions.
@@ -28,7 +29,7 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 			// @ts-expect-error
 			// SAFETY: Usually `next` always returns something in user land, but in `sequence` we are actually
 			// doing a loop over all the `next` functions, and eventually we call the last `next` that returns the `Response`.
-			const result = handle(handleContext, async (payload: RewritePayload) => {
+			const result = handle(handleContext, async (payload?: RewritePayload) => {
 				if (i < length - 1) {
 					if (payload) {
 						let newRequest;
@@ -42,10 +43,16 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 								handleContext.request,
 							);
 						}
+						const pipeline: Pipeline = Reflect.get(handleContext, apiContextRoutesSymbol);
+						const { routeData, pathname } = await pipeline.tryRewrite(
+							payload,
+							handleContext.request,
+						);
 						carriedPayload = payload;
 						handleContext.request = newRequest;
 						handleContext.url = new URL(newRequest.url);
 						handleContext.cookies = new AstroCookies(newRequest);
+						handleContext.params = getParams(routeData, pathname);
 					}
 					return applyHandle(i + 1, handleContext);
 				} else {
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 6dfa101375..9d813e19a3 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -37,14 +37,13 @@ import { sequence } from './middleware/index.js';
 import { renderRedirect } from './redirects/render.js';
 import { type Pipeline, Slots, getParams, getProps } from './render/index.js';
 
+export const apiContextRoutesSymbol = Symbol.for('context.routes');
+
 /**
  * Each request is rendered using a `RenderContext`.
  * It contains data unique to each request. It is responsible for executing middleware, calling endpoints, and rendering the page by gathering necessary data from a `Pipeline`.
  */
 export class RenderContext {
-	// The first route that this instance of the context attempts to render
-	originalRoute: RouteData;
-
 	private constructor(
 		readonly pipeline: Pipeline,
 		public locals: App.Locals,
@@ -57,9 +56,7 @@ export class RenderContext {
 		public params = getParams(routeData, pathname),
 		protected url = new URL(request.url),
 		public props: Props = {},
-	) {
-		this.originalRoute = routeData;
-	}
+	) {}
 
 	/**
 	 * A flag that tells the render content if the rewriting was triggered
@@ -142,14 +139,24 @@ export class RenderContext {
 			if (payload) {
 				pipeline.logger.debug('router', 'Called rewriting to:', payload);
 				// we intentionally let the error bubble up
-				const { routeData, componentInstance: newComponent } = await pipeline.tryRewrite(
-					payload,
-					this.request,
-					this.originalRoute,
-				);
+				const {
+					routeData,
+					componentInstance: newComponent,
+					pathname,
+					newUrl,
+				} = await pipeline.tryRewrite(payload, this.request);
 				this.routeData = routeData;
 				componentInstance = newComponent;
+				if (payload instanceof Request) {
+					this.request = payload;
+				} else {
+					this.request = this.#copyRequest(newUrl, this.request);
+				}
 				this.isRewriting = true;
+				this.url = new URL(this.request.url);
+				this.cookies = new AstroCookies(this.request);
+				this.params = getParams(routeData, pathname);
+				this.pathname = pathname;
 				this.status = 200;
 			}
 			let response: Response;
@@ -218,6 +225,7 @@ export class RenderContext {
 		const context = this.createActionAPIContext();
 		const redirect = (path: string, status = 302) =>
 			new Response(null, { status, headers: { Location: path } });
+		Reflect.set(context, apiContextRoutesSymbol, this.pipeline);
 
 		return Object.assign(context, {
 			props,
@@ -237,7 +245,6 @@ export class RenderContext {
 		const { routeData, componentInstance, newUrl, pathname } = await this.pipeline.tryRewrite(
 			reroutePayload,
 			this.request,
-			this.originalRoute,
 		);
 		this.routeData = routeData;
 		if (reroutePayload instanceof Request) {
diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts
index 80073a54eb..0bc069cbc8 100644
--- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts
+++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts
@@ -199,11 +199,7 @@ export class DevPipeline extends Pipeline {
 		}
 	}
 
-	async tryRewrite(
-		payload: RewritePayload,
-		request: Request,
-		_sourceRoute: RouteData,
-	): Promise<TryRewriteResult> {
+	async tryRewrite(payload: RewritePayload, request: Request): Promise<TryRewriteResult> {
 		if (!this.manifestData) {
 			throw new Error('Missing manifest data. This is an internal error, please file an issue.');
 		}
diff --git a/packages/astro/test/fixtures/reroute/src/middleware.js b/packages/astro/test/fixtures/reroute/src/middleware.js
index 09f3ef73c4..eae50f866c 100644
--- a/packages/astro/test/fixtures/reroute/src/middleware.js
+++ b/packages/astro/test/fixtures/reroute/src/middleware.js
@@ -1,4 +1,5 @@
 import { sequence } from 'astro:middleware';
+import {defineMiddleware} from "astro/middleware";
 
 let contextReroute = false;
 
@@ -22,16 +23,23 @@ export const second = async (context, next) => {
 		if (context.url.pathname.includes('/auth/params')) {
 			return  next('/?foo=bar');
 		}
+
+		if (context.url.pathname.includes('/auth/astro-params')) {
+			return next('/auth/1234');
+		}
 	}
 	return next();
 };
 
-export const third = async (context, next) => {
+export const third = defineMiddleware(async (context, next) => {
 	// just making sure that we are testing the change in context coming from `next()`
 	if (context.url.pathname.startsWith('/') && contextReroute === false) {
 		context.locals.auth = 'Third function called';
 	}
+	if (context.params?.id === '1234') {
+		context.locals.auth = 'Params changed'
+	}
 	return next();
-};
+});
 
 export const onRequest = sequence(first, second, third);
diff --git a/packages/astro/test/fixtures/reroute/src/pages/auth/[id].astro b/packages/astro/test/fixtures/reroute/src/pages/auth/[id].astro
new file mode 100644
index 0000000000..f60d2b807f
--- /dev/null
+++ b/packages/astro/test/fixtures/reroute/src/pages/auth/[id].astro
@@ -0,0 +1,23 @@
+---
+
+export function getStaticPaths( ) {
+	return [{
+		params: {
+			id: "1234"
+		}
+	}]
+}
+
+const { id } = Astro.params;
+const auth = Astro.locals.auth;
+---
+<html>
+<head>
+	<title>Index with params</title>
+</head>
+<body>
+<h1>Index with params</h1>
+<p id="params">Param: {id}</p>
+<p id="locals">Locals: {auth}</p>
+</body>
+</html>
diff --git a/packages/astro/test/fixtures/reroute/src/pages/auth/astro-params.astro b/packages/astro/test/fixtures/reroute/src/pages/auth/astro-params.astro
new file mode 100644
index 0000000000..853d812bb3
--- /dev/null
+++ b/packages/astro/test/fixtures/reroute/src/pages/auth/astro-params.astro
@@ -0,0 +1,3 @@
+---
+
+---
diff --git a/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro
index 3f4dfad875..7530bf7a5f 100644
--- a/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro
+++ b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro
@@ -1,4 +1,6 @@
 ---
+const { id } = Astro.params;
+const auth = Astro.locals.auth;
 ---
 <html>
 <head>
@@ -6,5 +8,7 @@
 </head>
 <body>
 <h1>Index with params</h1>
+<p id="params">Param: {id}</p>
+<p id="locals">Locals: {auth}</p>
 </body>
 </html>
diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js
index 10d7c70d24..26cdac5c04 100644
--- a/packages/astro/test/rewrite.test.js
+++ b/packages/astro/test/rewrite.test.js
@@ -388,6 +388,15 @@ describe('Middleware', () => {
 
 		assert.match($('h1').text(), /Index/);
 	});
+
+	it('should render correctly compute the new params next("/auth/1234")', async () => {
+		const html = await fixture.fetch('/auth/astro-params').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.match($('h1').text(), /Index with params/);
+		assert.match($('#params').text(), /Param: 1234/);
+		assert.match($('#locals').text(), /Locals: Params changed/);
+	});
 });
 
 describe('Middleware with custom 404.astro and 500.astro', () => {