fix: race condition in resolvedPathname

.changeset/fix-resolved-pathname-race.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes a race condition where concurrent requests to dynamic routes in the dev server could produce incorrect params.

packages/astro/src/core/app/base.ts

@@ -82,6 +82,13 @@ export interface RenderOptions {
 	 * Default: `app.match(request)`
 	 */
 	routeData?: RouteData;
+
+	/**
+	 * @internal
+	 * The resolved pathname from dev route matching (may differ from the request
+	 * pathname, e.g. after .html stripping). Used to extract correct params.
+	 */
+	resolvedPathname?: string;
 }
 
 export interface RenderErrorOptions {
@@ -409,11 +416,13 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 				});
 			}
 		}
+		let resolvedPathname: string | undefined = renderOptions?.resolvedPathname;
 		if (!routeData) {
 			if (this.isDev()) {
 				const result = await this.devMatch(this.getPathnameFromRequest(request));
 				if (result) {
 					routeData = result.routeData;
+					resolvedPathname = result.resolvedPathname;
 				}
 			} else {
 				routeData = this.match(request);
@@ -440,7 +449,7 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
 				prerenderedErrorPageFetch: prerenderedErrorPageFetch,
 			});
 		}
-		const pathname = this.getPathnameFromRequest(request);
+		const pathname = resolvedPathname ?? this.getPathnameFromRequest(request);
 		const defaultStatus = this.getDefaultStatusCode(routeData, pathname);
 
 		let response;

packages/astro/src/core/app/dev/app.ts

@@ -2,7 +2,7 @@
 import { MiddlewareNoDataOrNextCalled, MiddlewareNotAResponse } from '../../errors/errors-data.js';
 import { type AstroError, isAstroError } from '../../errors/index.js';
 import type { Logger } from '../../logger/core.js';
 import type { CreateRenderContext, RenderContext } from '../../render-context.js';
 import {
 	BaseApp,
 	type DevMatch,
@@ -20,7 +20,6 @@
 
 export class DevApp extends BaseApp<NonRunnablePipeline> {
 	logger: Logger;
-	resolvedPathname: string | undefined = undefined;
 	constructor(manifest: SSRManifest, streaming = true, logger: Logger) {
 		super(manifest, streaming, logger);
 		this.logger = logger;
@@ -60,20 +59,12 @@
 		);
 		if (!matchedRoute) return undefined;
 
-		this.resolvedPathname = matchedRoute.resolvedPathname;
 		return {
 			routeData: matchedRoute.route,
 			resolvedPathname: matchedRoute.resolvedPathname,
 		};
 	}
 
-	async createRenderContext(payload: CreateRenderContext): Promise<RenderContext> {
-		return super.createRenderContext({
-			...payload,
-			pathname: this.resolvedPathname ?? payload.pathname,
-		});
-	}
-
 	async renderError(
 		request: Request,
 		{ locals, skipMiddleware = false, error, clientAddress, status }: RenderErrorOptions,

packages/astro/src/vite-plugin-app/app.ts

@@ -31,7 +31,6 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 	loader: ModuleLoader;
 	manifestData: RoutesList;
 	currentRenderContext: RenderContext | undefined = undefined;
-	resolvedPathname: string | undefined = undefined;
 	constructor(
 		manifest: SSRManifest,
 		streaming = true,
@@ -119,10 +118,7 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 	}
 
 	async createRenderContext(payload: CreateRenderContext): Promise<RenderContext> {
-		this.currentRenderContext = await super.createRenderContext({
-			...payload,
-			pathname: this.resolvedPathname ?? payload.pathname,
-		});
+		this.currentRenderContext = await super.createRenderContext(payload);
 		return this.currentRenderContext;
 	}
 
@@ -184,7 +180,6 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 					throw new Error('No route matched, and default 404 route was not found.');
 				}
 
-				self.resolvedPathname = matchedRoute.resolvedPathname;
 				const request = createRequest({
 					url,
 					headers: incomingRequest.headers,
@@ -207,6 +202,7 @@ export class AstroServerApp extends BaseApp<RunnablePipeline> {
 				const response = await self.render(request, {
 					locals,
 					routeData: matchedRoute.routeData,
+					resolvedPathname: matchedRoute.resolvedPathname,
 					clientAddress,
 				});
 

packages/astro/test/units/routing/resolved-pathname.test.js

@@ -0,0 +1,91 @@
+import * as assert from 'node:assert/strict';
+import { after, before, describe, it } from 'node:test';
+
+import { createContainer } from '../../../dist/core/dev/container.js';
+import testAdapter from '../../test-adapter.js';
+import {
+	createBasicSettings,
+	createFixture,
+	createRequestAndResponse,
+	defaultLogger,
+} from '../test-utils.js';
+
+const fileSystem = {
+	'/src/pages/api/[category]/[id].ts': `
+		export const prerender = false;
+		export function GET({ params, url }) {
+			return Response.json({ params, pathname: url.pathname });
+		}
+	`,
+	'/src/pages/api/[category]/index.ts': `
+		export const prerender = false;
+		export function GET({ params, url }) {
+			return Response.json({ params, pathname: url.pathname });
+		}
+	`,
+};
+
+describe('Resolved pathname in dev server', () => {
+	let container;
+
+	before(async () => {
+		const fixture = await createFixture(fileSystem);
+		const settings = await createBasicSettings({
+			root: fixture.path,
+			output: 'server',
+			adapter: testAdapter(),
+			trailingSlash: 'never',
+		});
+		container = await createContainer({
+			settings,
+			logger: defaultLogger,
+		});
+	});
+
+	after(async () => {
+		await container.close();
+	});
+
+	it('should resolve params correctly for .html requests to dynamic routes', async () => {
+		const { req, res, json } = createRequestAndResponse({
+			method: 'GET',
+			url: '/api/books.html',
+		});
+		container.handle(req, res);
+		const body = await json();
+
+		assert.equal(body.params.category, 'books');
+		assert.equal(body.params.id, undefined);
+	});
+
+	it('should resolve params correctly for .html requests to nested dynamic routes', async () => {
+		const { req, res, json } = createRequestAndResponse({
+			method: 'GET',
+			url: '/api/books/42.html',
+		});
+		container.handle(req, res);
+		const body = await json();
+
+		assert.equal(body.params.category, 'books');
+		assert.equal(body.params.id, '42');
+	});
+
+	it('should not cross-contaminate resolved pathnames between concurrent requests', async () => {
+		// Fire both requests before awaiting either response.
+		// Before the fix, resolvedPathname was stored as shared instance state,
+		// so the second request could overwrite the first's pathname.
+		const r1 = createRequestAndResponse({ method: 'GET', url: '/api/books/1.html' });
+		const r2 = createRequestAndResponse({ method: 'GET', url: '/api/movies/99' });
+
+		container.handle(r1.req, r1.res);
+		container.handle(r2.req, r2.res);
+
+		const [body1, body2] = await Promise.all([r1.json(), r2.json()]);
+
+		assert.equal(body1.params.category, 'books');
+		assert.equal(body1.params.id, '1');
+
+		assert.equal(body2.params.category, 'movies');
+		assert.equal(body2.params.id, '99');
+	});
+});

packages/integrations/cloudflare/src/utils/handler.ts

@@ -70,10 +70,12 @@ export async function handle(
 	}
 
 	let routeData: RouteData | undefined = undefined;
+	let resolvedPathname: string | undefined = undefined;
 	if (app.isDev()) {
 		const result = await app.devMatch(app.getPathnameFromRequest(request));
 		if (result) {
 			routeData = result.routeData;
+			resolvedPathname = result.resolvedPathname;
 		}
 	} else {
 		routeData = app.match(request);
@@ -120,6 +122,7 @@ export async function handle(
 
 	const response = await app.render(request, {
 		routeData,
+		resolvedPathname,
 		locals,
 		prerenderedErrorPageFetch: async (url: string) => {
 			// NOTE this ASSETS binding path is needed for users who are using `run_worker_first` routing