feat(routing): decode pathname early, don't decode params (#12079)

* feat(routing): decode pathname early

* linting

* Update packages/astro/src/core/fs/index.ts

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

* address feedback

* Update .changeset/wet-foxes-walk.md

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

* fix changeset

---------

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>

.changeset/wet-foxes-walk.md

@@ -0,0 +1,28 @@
+---
+'astro': major
+---
+
+`params` passed in `getStaticPaths` are no longer automatically decoded.
+
+### [changed]: `params` aren't decoded anymore.
+In Astro v4.x, `params` in ` were automatically decoded using `decodeURIComponent`. 
+
+Astro v5.0 doesn't automatically decode `params` in `getStaticPaths` anymore, so you'll need to manually decode them yourself if needed
+
+#### What should I do?
+If you were relying on the automatic decode, you'll need to manually decode it using `decodeURI`.
+
+Note that the use of [`decodeURIComponent`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent)) is discouraged for `getStaticPaths` because it decodes more characters than it should, for example `/`, `?`, `#` and more.
+
+```diff
+---
+export function getStaticPaths() {
+  return [
++    { params: { id: decodeURI("%5Bpage%5D") } },
+-    { params: { id: "%5Bpage%5D" } },
+  ]
+}
+
+const { id } = Astro.params;
+---
+```

packages/astro/src/core/build/generate.ts

@@ -419,7 +419,7 @@ async function generatePath(
 	});
 	const renderContext = await RenderContext.create({
 		pipeline,
-		pathname,
+		pathname: pathname,
 		request,
 		routeData: route,
 	});
@@ -469,8 +469,11 @@ async function generatePath(
 		body = Buffer.from(await response.arrayBuffer());
 	}
 
-	const outFolder = getOutFolder(pipeline.settings, pathname, route);
-	const outFile = getOutFile(config, outFolder, pathname, route);
+	// We encode the path because some paths will received encoded characters, e.g. /[page] VS /%5Bpage%5D.
+	// Node.js decodes the paths, so to avoid a clash between paths, do encode paths again, so we create the correct files and folders requested by the user.
+	const encodedPath = encodeURI(pathname);
+	const outFolder = getOutFolder(pipeline.settings, encodedPath, route);
+	const outFile = getOutFile(config, outFolder, encodedPath, route);
 	if (route.distURL) {
 		route.distURL.push(outFile);
 	} else {
@@ -484,13 +487,13 @@ async function generatePath(
 function getPrettyRouteName(route: RouteData): string {
 	if (isRelativePath(route.component)) {
 		return route.route;
-	} else if (route.component.includes('node_modules/')) {
+	}
+	if (route.component.includes('node_modules/')) {
 		// For routes from node_modules (usually injected by integrations),
 		// prettify it by only grabbing the part after the last `node_modules/`
 		return /.*node_modules\/(.+)/.exec(route.component)?.[1] ?? route.component;
-	} else {
-		return route.component;
 	}
+	return route.component;
 }
 
 /**

packages/astro/src/core/build/static-build.ts

@@ -383,7 +383,7 @@ async function cleanServerOutput(
 			}),
 		);
 
-		removeEmptyDirs(out);
+		removeEmptyDirs(fileURLToPath(out));
 	}
 
 	// Clean out directly if the outDir is outside of root
@@ -447,7 +447,7 @@ async function ssrMoveAssets(opts: StaticBuildOptions) {
 				return fs.promises.rename(currentUrl, clientUrl);
 			}),
 		);
-		removeEmptyDirs(serverAssets);
+		removeEmptyDirs(fileURLToPath(serverAssets));
 	}
 }
 

packages/astro/src/core/fs/index.ts

@@ -1,19 +1,16 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { appendForwardSlash } from '../path.js';
 
 const isWindows = process.platform === 'win32';
 
-export function removeEmptyDirs(root: URL): void {
-	const dir = fileURLToPath(root);
+export function removeEmptyDirs(dir: string): void {
 	if (!fs.statSync(dir).isDirectory()) return;
 	let files = fs.readdirSync(dir);
 
 	if (files.length > 0) {
 		files.map((file) => {
-			const url = new URL(`./${file}`, appendForwardSlash(root.toString()));
-			removeEmptyDirs(url);
+			removeEmptyDirs(path.join(dir, file));
 		});
 		files = fs.readdirSync(dir);
 	}

packages/astro/src/core/render-context.ts

@@ -77,7 +77,7 @@ export class RenderContext {
 			pipeline,
 			locals,
 			sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware),
-			pathname,
+			decodeURI(pathname),
 			request,
 			routeData,
 			status,

packages/astro/src/core/render/params-and-props.ts

@@ -74,7 +74,7 @@ export function getParams(route: RouteData, pathname: string): Params {
 	if (!route.params.length) return {};
 	// The RegExp pattern expects a decoded string, but the pathname is encoded
 	// when the URL contains non-English characters.
-	const paramsMatch = route.pattern.exec(decodeURIComponent(pathname));
+	const paramsMatch = route.pattern.exec(pathname);
 	if (!paramsMatch) return {};
 	const params: Params = {};
 	route.params.forEach((key, i) => {

packages/astro/test/fixtures/ssr-params/src/pages/[category].astro

@@ -1,5 +1,13 @@
 ---
 const { category } = Astro.params
+export function getStaticPaths() {
+	return [
+		{ params: { category: "%23something" } },
+		{ params: { category: "%2Fsomething" } },
+		{ params: { category: "%3Fsomething" } },
+		{ params: { category: "[page]" } },
+	]
+}
 ---
 <html>
 	<head>

packages/astro/test/fixtures/ssr-params/src/pages/東西/[category].astro

@@ -1,4 +1,9 @@
 ---
+export function getStaticPaths() {
+	return [
+		{ params: { category: "food" } },
+	]
+}
 const { category } = Astro.params
 ---
 <html>

packages/astro/test/params.test.js

@@ -0,0 +1,126 @@
+import assert from 'node:assert/strict';
+import { before, describe, it } from 'node:test';
+import * as cheerio from 'cheerio';
+import testAdapter from './test-adapter.js';
+import { loadFixture } from './test-utils.js';
+
+describe('Astro.params in SSR', () => {
+	/** @type {import('./test-utils.js').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/ssr-params/',
+			adapter: testAdapter(),
+			output: 'server',
+			base: '/users/houston/',
+		});
+		await fixture.build();
+	});
+
+	it('Params are passed to component', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/food');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), 'food');
+	});
+
+	describe('Non-english characters in the URL', () => {
+		it('Params are passed to component', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/users/houston/東西/food');
+			const response = await app.render(request);
+			assert.equal(response.status, 200);
+			const html = await response.text();
+			const $ = cheerio.load(html);
+			assert.equal($('.category').text(), 'food');
+		});
+	});
+
+	it('It uses encodeURI/decodeURI to decode parameters', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/[page]');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '[page]');
+	});
+
+	it('It accepts encoded URLs, and the params decoded', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/%5Bpage%5D');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '[page]');
+	});
+
+	it("It doesn't encode/decode URI characters such as %23 (#)", async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/%23something');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%23something');
+	});
+	it("It doesn't encode/decode URI characters such as %2F (/)", async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/%2Fsomething');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%2Fsomething');
+	});
+
+	it("It doesn't encode/decode URI characters such as %3F (?)", async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const request = new Request('http://example.com/users/houston/%3Fsomething');
+		const response = await app.render(request);
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%3Fsomething');
+	});
+});
+
+describe('Astro.params in static mode', () => {
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/ssr-params/',
+		});
+		await fixture.build();
+	});
+
+	it('It creates files that have square brackets in their URL', async () => {
+		const html = await fixture.readFile(encodeURI('/[page]/index.html'));
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '[page]');
+	});
+
+	it("It doesn't encode/decode URI characters such as %23 (#)", async () => {
+		const html = await fixture.readFile(encodeURI('/%23something/index.html'));
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%23something');
+	});
+
+	it("It doesn't encode/decode URI characters such as %2F (/)", async () => {
+		const html = await fixture.readFile(encodeURI('/%2Fsomething/index.html'));
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%2Fsomething');
+	});
+
+	it("It doesn't encode/decode URI characters such as %3F (?)", async () => {
+		const html = await fixture.readFile(encodeURI('/%3Fsomething/index.html'));
+		const $ = cheerio.load(html);
+		assert.equal($('.category').text(), '%3Fsomething');
+	});
+});

packages/astro/test/ssr-params.test.js

@@ -1,52 +0,0 @@
-import assert from 'node:assert/strict';
-import { before, describe, it } from 'node:test';
-import * as cheerio from 'cheerio';
-import testAdapter from './test-adapter.js';
-import { loadFixture } from './test-utils.js';
-
-describe('Astro.params in SSR', () => {
-	/** @type {import('./test-utils.js').Fixture} */
-	let fixture;
-
-	before(async () => {
-		fixture = await loadFixture({
-			root: './fixtures/ssr-params/',
-			adapter: testAdapter(),
-			output: 'server',
-			base: '/users/houston/',
-		});
-		await fixture.build();
-	});
-
-	it('Params are passed to component', async () => {
-		const app = await fixture.loadTestAdapterApp();
-		const request = new Request('http://example.com/users/houston/food');
-		const response = await app.render(request);
-		assert.equal(response.status, 200);
-		const html = await response.text();
-		const $ = cheerio.load(html);
-		assert.equal($('.category').text(), 'food');
-	});
-
-	describe('Non-english characters in the URL', () => {
-		it('Params are passed to component', async () => {
-			const app = await fixture.loadTestAdapterApp();
-			const request = new Request('http://example.com/users/houston/東西/food');
-			const response = await app.render(request);
-			assert.equal(response.status, 200);
-			const html = await response.text();
-			const $ = cheerio.load(html);
-			assert.equal($('.category').text(), 'food');
-		});
-	});
-
-	it('No double URL decoding', async () => {
-		const app = await fixture.loadTestAdapterApp();
-		const request = new Request('http://example.com/users/houston/%25%23%3F');
-		const response = await app.render(request);
-		assert.equal(response.status, 200);
-		const html = await response.text();
-		const $ = cheerio.load(html);
-		assert.equal($('.category').text(), '%#?');
-	});
-});