Skip to content

Commit 493796b

Browse files
authored
Skip URL normalization writes that would not change the path (#17416)
The pathname setter re-parses the whole URL, so a no-op assignment is still expensive. Guard each write in normalizeUrl and FetchState. Collapse stays after the decode is assigned: the setter turns `\` into `/`, so `/a%5C/b` only becomes `/a//b` once written back.
1 parent 0c99615 commit 493796b

4 files changed

Lines changed: 92 additions & 5 deletions

File tree

.changeset/cyan-bikes-visit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Skips no-op pathname writes when normalizing SSR request URLs

packages/astro/src/core/fetch/fetch-state.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { getParams, getProps } from '../render/index.js';
3434
import { executeRewrite } from '../rewrites/handler.js';
3535
import { isRoute404or500, isRouteServerIsland } from '../routing/match.js';
3636
import { MultiLevelEncodingError, validateAndDecodePathname } from '../util/pathname.js';
37+
import { setPathname } from '../util/normalized-url.js';
3738
import { getOriginPathname, setOriginPathname } from '../routing/rewrite.js';
3839
import { computePathnameFromDomain } from '../i18n/domain.js';
3940
import { getCustom404Route, routeHasHtmlExtension } from '../routing/helpers.js';
@@ -331,8 +332,8 @@ export class FetchState implements AstroFetchState {
331332
const url = new URL(request.url);
332333
const publicPathname = this.#normalizePathname(url.pathname);
333334
const pathname = this.#computePathname(publicPathname);
334-
url.pathname = publicPathname;
335-
url.pathname = collapseDuplicateSlashes(url.pathname);
335+
setPathname(url, publicPathname);
336+
setPathname(url, collapseDuplicateSlashes(url.pathname));
336337
// For domain-based i18n routing, the locale prefix is derived from the
337338
// request's Host header rather than its URL. When a locale is detected,
338339
// the resulting pathname includes the prefix (e.g. /en/boats/1/foo) that

packages/astro/src/core/util/normalized-url.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,34 @@ export function createNormalizedurl(requestUrl: string): URL {
99
return normalizeUrl(new URL(requestUrl));
1010
}
1111

12+
/**
13+
* Assigns `url.pathname` only when the value differs.
14+
* The setter re-parses the whole URL, so a no-op write is still expensive.
15+
*/
16+
export function setPathname(url: URL, pathname: string): void {
17+
if (url.pathname !== pathname) {
18+
url.pathname = pathname;
19+
}
20+
}
21+
1222
/**
1323
* Normalizes an already-parsed URL in place: decodes and validates the
1424
* pathname, collapses duplicate slashes. Returns the same URL object.
25+
*
26+
* Collapse runs after the decode is written back: the pathname setter
27+
* rewrites `\` to `/`, so a decoded backslash only becomes `//` once assigned.
1528
*/
1629
export function normalizeUrl(url: URL): URL {
1730
try {
18-
url.pathname = validateAndDecodePathname(url.pathname);
31+
setPathname(url, validateAndDecodePathname(url.pathname));
1932
} catch {
2033
// For decoding failures (truly malformed URLs), fall back gracefully.
2134
try {
22-
url.pathname = decodeURI(url.pathname);
35+
setPathname(url, decodeURI(url.pathname));
2336
} catch {
2437
// If even basic decoding fails, return URL as-is
2538
}
2639
}
27-
url.pathname = collapseDuplicateSlashes(url.pathname);
40+
setPathname(url, collapseDuplicateSlashes(url.pathname));
2841
return url;
2942
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import assert from 'node:assert/strict';
2+
import { describe, it } from 'node:test';
3+
import { createNormalizedUrl, normalizeUrl } from '../../../dist/core/util/normalized-url.js';
4+
5+
describe('normalizeUrl', () => {
6+
// #region Plain paths (the common case: nothing to rewrite)
7+
8+
it('leaves an ordinary path unchanged', () => {
9+
assert.equal(normalizeUrl(new URL('https://e.com/about')).pathname, '/about');
10+
});
11+
12+
it('leaves the root path unchanged', () => {
13+
assert.equal(normalizeUrl(new URL('https://e.com/')).pathname, '/');
14+
});
15+
16+
it('preserves the search and hash', () => {
17+
assert.equal(normalizeUrl(new URL('https://e.com/x?q=1#h')).href, 'https://e.com/x?q=1#h');
18+
});
19+
20+
it('returns the same URL object', () => {
21+
const url = new URL('https://e.com/a');
22+
assert.equal(normalizeUrl(url), url);
23+
});
24+
25+
// #endregion
26+
// #region Duplicate slashes
27+
28+
it('collapses duplicate slashes', () => {
29+
assert.equal(normalizeUrl(new URL('https://e.com/a//b')).pathname, '/a/b');
30+
assert.equal(normalizeUrl(new URL('https://e.com///a///b')).pathname, '/a/b');
31+
});
32+
33+
// #endregion
34+
// #region Encoding
35+
36+
it('decodes single-encoded unreserved characters', () => {
37+
assert.equal(normalizeUrl(new URL('https://e.com/api/%61dmin')).pathname, '/api/admin');
38+
});
39+
40+
it('fully decodes multi-encoded unreserved characters', () => {
41+
assert.equal(normalizeUrl(new URL('https://e.com/api/%2561dmin')).pathname, '/api/admin');
42+
});
43+
44+
it('keeps reserved characters encoded', () => {
45+
assert.equal(normalizeUrl(new URL('https://e.com/path%3Fname')).pathname, '/path%3Fname');
46+
});
47+
48+
it('re-encodes characters the pathname setter escapes', () => {
49+
assert.equal(normalizeUrl(new URL('https://e.com/a%20b')).pathname, '/a%20b');
50+
});
51+
52+
// #endregion
53+
// #region Backslash
54+
55+
// Pathname setter rewrites `\` to `/`, so collapse must run after the
56+
// decode is assigned: `/a%5C/b` -> `/a\/b` -> `/a//b` -> `/a/b`.
57+
it('collapses a slash introduced by decoding a backslash', () => {
58+
assert.equal(normalizeUrl(new URL('https://e.com/a%5C/b')).pathname, '/a/b');
59+
});
60+
61+
// #endregion
62+
});
63+
64+
describe('createNormalizedUrl', () => {
65+
it('parses and normalizes a request URL string', () => {
66+
assert.equal(createNormalizedUrl('https://e.com/a//%61dmin').pathname, '/a/admin');
67+
});
68+
});

0 commit comments

Comments
 (0)