fix: correctly handle pathname for dynamic routing in rewrite (#13113)

withastro · Jan 31, 2025 · 3a26e45 · 3a26e45
1 parent c33eccb
commit 3a26e45
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 2 deletions.
diff --git a/.changeset/brave-cats-retire.md b/.changeset/brave-cats-retire.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes the bug that rewrite will pass encoded url to the dynamic routing and cause params mismatch.
diff --git a/packages/astro/src/core/routing/rewrite.ts b/packages/astro/src/core/routing/rewrite.ts
@@ -53,9 +53,10 @@ export function findRouteToRewrite({
 		pathname = pathname.slice(base.length);
 	}
 
+	const decodedPathname = decodeURI(pathname);
 	let foundRoute;
 	for (const route of routes) {
-		if (route.pattern.test(decodeURI(pathname))) {
+		if (route.pattern.test(decodedPathname)) {
 			foundRoute = route;
 			break;
 		}
@@ -65,7 +66,7 @@ export function findRouteToRewrite({
 		return {
 			routeData: foundRoute,
 			newUrl,
-			pathname,
+			pathname: decodedPathname,
 		};
 	} else {
 		const custom404 = routes.find((route) => route.route === '/404');

diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/astro.config.mjs b/packages/astro/test/fixtures/rewrite-dynamic-routing/astro.config.mjs
@@ -0,0 +1,6 @@
+import {defineConfig} from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+	site: "https://example.com"
+});
diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/package.json b/packages/astro/test/fixtures/rewrite-dynamic-routing/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/rewrite-dynamic-routing",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}
diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/[id].astro b/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/[id].astro
@@ -0,0 +1,17 @@
+---
+export function getStaticPaths() {
+    return [{ params: { id: 'ABC abc 123' } },
+            { params: { id: 'test' } }]
+}
+
+const { id } = Astro.params
+---
+<html>
+    <head>
+        <title>Index</title>
+    </head>
+    <body>
+        <h1>Index</h1>
+        <p>{id}</p>
+    </body>
+</html>
diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/bar.astro b/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/bar.astro
@@ -0,0 +1,3 @@
+---
+return Astro.rewrite('/ABC abc 123')
+---
diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/foo.astro b/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/foo.astro
@@ -0,0 +1,3 @@
+---
+return Astro.rewrite('/has space/test')
+---
diff --git a/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/has space/[id].astro b/packages/astro/test/fixtures/rewrite-dynamic-routing/src/pages/has space/[id].astro
@@ -0,0 +1,17 @@
+---
+export function getStaticPaths() {
+    return [{ params: { id: 'ABC abc 123' } },
+            { params: { id: 'test' } }]
+}
+
+const { id } = Astro.params
+---
+<html>
+    <head>
+        <title>Index</title>
+    </head>
+    <body>
+        <h1>Index</h1>
+        <p>{id}</p>
+    </body>
+</html>
diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js
@@ -111,6 +111,37 @@ describe('Dev rewrite, trailing slash -> never, with base', () => {
 	});
 });
 
+describe('Dev rewrite, dynamic routing', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let devServer;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/rewrite-dynamic-routing/',
+		});
+		devServer = await fixture.startDevServer();
+	});
+
+	after(async () => {
+		await devServer.stop();
+	});
+
+	it('should decode the escaped characters in the URL', async () => {
+		const html = await fixture.fetch('/foo').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.equal($('h1').text(), 'Index');
+	});
+
+	it('should decode the escaped characters in the params', async () => {
+		const html = await fixture.fetch('/bar').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.equal($('h1').text(), 'Index');
+	});
+});
+
 describe('Dev rewrite, hybrid/server', () => {
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml