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

.changeset/wet-foxes-walk.md

@@ -0,0 +1,28 @@
+---
+'astro': major
+---
+
+`Astro.params` aren't decoded anymore. 
+
+### [changed]: `Astro.params` aren't decoded anymore.
+In Astro v4.x, `Astro.params` were decoded using `decodeURIComponent`. 
+
+Astro v5.0 doesn't decode `Astro.params` anymore, so you'll need to manually decode them yourself.
+
+#### What should I do?
+If you were relying on `Astro.params` being decoded for you, you'll need to manually decode it using `decodeURI`.
+
+The use of [`decodeURIComponent`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent)) is discouraged 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

@@ -375,18 +375,18 @@ interface GeneratePathOptions {
 }
 
 async function generatePath(
-	pathname: string,
+	path: string,
 	pipeline: BuildPipeline,
 	gopts: GeneratePathOptions,
 	route: RouteData,
 ) {
 	const { mod } = gopts;
 	const { config, logger, options } = pipeline;
-	logger.debug('build', `Generating: ${pathname}`);
+	logger.debug('build', `Generating: ${path}`);
 
 	// This adds the page name to the array so it can be shown as part of stats.
 	if (route.type === 'page') {
-		addPageName(pathname, options);
+		addPageName(path, options);
 	}
 
 	// Do not render the fallback route if there is already a translated page
@@ -397,13 +397,13 @@ async function generatePath(
 		// always be rendered
 		route.pathname !== '/' &&
 		// Check if there is a translated page with the same path
-		Object.values(options.allPages).some((val) => val.route.pattern.test(pathname))
+		Object.values(options.allPages).some((val) => val.route.pattern.test(path))
 	) {
 		return;
 	}
 
 	const url = getUrlForPath(
-		pathname,
+		path,
 		config.base,
 		options.origin,
 		config.build.format,
@@ -419,7 +419,7 @@ async function generatePath(
 	});
 	const renderContext = await RenderContext.create({
 		pipeline,
-		pathname,
+		pathname: path,
 		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(path);
+	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;
 }
 
 /**
@@ -540,7 +543,8 @@ function createBuildManifest(
 				onRequest: middleware,
 			};
 		},
-		checkOrigin: (settings.config.security?.checkOrigin && settings.buildOutput === "server") ?? false,
+		checkOrigin:
+			(settings.config.security?.checkOrigin && settings.buildOutput === 'server') ?? false,
 		key,
 		envGetSecretEnabled: false,
 	};

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

@@ -427,7 +427,7 @@ async function cleanServerOutput(
 			}),
 		);
 
-		removeEmptyDirs(out);
+		removeEmptyDirs(fileURLToPath(out));
 	}
 
 	// Clean out directly if the outDir is outside of root
@@ -491,7 +491,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,17 @@
 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 {
+	// const dir = fileURLToPath(root);
 	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');
+	});
+});