Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
---

`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;
---
```
18 changes: 11 additions & 7 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ async function generatePath(
});
const renderContext = await RenderContext.create({
pipeline,
pathname,
pathname: pathname,
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(pathname);
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: 2 additions & 5 deletions packages/astro/src/core/fs/index.ts
Original file line number Diff line number Diff line change
@@ -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);
}
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.

Loading