Skip to content

Commit

Permalink
feat(routing): decode pathname early
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Sep 26, 2024
1 parent acf264d commit 3c9b5cc
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 72 deletions.
28 changes: 28 additions & 0 deletions .changeset/wet-foxes-walk.md
Original file line number Diff line number Diff line change
@@ -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;
---
```
28 changes: 16 additions & 12 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -419,7 +419,7 @@ async function generatePath(
});
const renderContext = await RenderContext.create({
pipeline,
pathname,
pathname: path,
request,
routeData: route,
});
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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,
};
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ async function cleanServerOutput(
}),
);

removeEmptyDirs(out);
removeEmptyDirs(fileURLToPath(out));
}

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

Expand Down
7 changes: 3 additions & 4 deletions packages/astro/src/core/fs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ 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);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class RenderContext {
pipeline,
locals,
sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware),
pathname,
decodeURI(pathname),
request,
routeData,
status,
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/render/params-and-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
---
export function getStaticPaths() {
return [
{ params: { category: "food" } },
]
}
const { category } = Astro.params
---
<html>
Expand Down
126 changes: 126 additions & 0 deletions packages/astro/test/params.test.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
52 changes: 0 additions & 52 deletions packages/astro/test/ssr-params.test.js

This file was deleted.

0 comments on commit 3c9b5cc

Please sign in to comment.