Skip to content

Commit ecc6313

Browse files
committed
fix(@angular/ssr): resolve the request path through the router's URL grammar before matching
`ServerRouter.match` tokenises the pathname by splitting on `/`, while `@angular/router` parses it with `DefaultUrlSerializer`, a grammar in which `(`, `)`, `;` and `//` are metacharacters and unparseable input is silently discarded. The two therefore disagree on which route a request is: verified against @angular/router 22.1.6, `/page)`, `/page(`, `/page;` and `/(page)` all resolve to `/page`, and `/a/1//b` resolves to `/a/1`. Because `ServerRouter.match` selects the response's `headers`, `status`, `renderMode` and `preload` while `@angular/router` selects the component that renders into the body, appending a single character to a path produces a response whose body comes from one route and whose per-route configuration comes from another. A route given `Cache-Control: no-store, private` plus `X-Frame-Options: DENY` is served under the catch-all's policy with neither header, and a route declared `RenderMode.Client` is server-rendered. This is the same divergence that 85c18b4 fixed for matrix parameters, where it surfaced as URLs failing to match their route. `stripMatrixParams` handled that case; parentheses and interior `//` are the remaining ones. Normalising through the router's own serializer covers the class rather than the next symptom, and `@angular/router` is already a peer dependency of this package. A path the serializer cannot parse is returned unchanged, so malformed percent-encoding keeps its existing behaviour, and normalisation runs before `stripMatrixParams` so matrix parameters are still stripped exactly as today. Closes #34090
1 parent 31c0456 commit ecc6313

4 files changed

Lines changed: 115 additions & 2 deletions

File tree

packages/angular/ssr/src/routes/router.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { AngularAppManifest } from '../manifest';
10-
import { stripIndexHtmlFromURL, stripMatrixParams } from '../utils/url';
10+
import { normalizeUrlPath, stripIndexHtmlFromURL, stripMatrixParams } from '../utils/url';
1111
import { extractRoutesAndCreateRouteTree } from './ng-routes';
1212
import { RouteTree, RouteTreeNodeMetadata } from './route-tree';
1313

@@ -86,7 +86,9 @@ export class ServerRouter {
8686
// Strip 'index.html' from URL if present.
8787
// A request to `http://www.example.com/page/index.html` will render the Angular route corresponding to `http://www.example.com/page`.
8888
let { pathname } = stripIndexHtmlFromURL(url);
89-
pathname = stripMatrixParams(pathname);
89+
// Resolve the path through the router's own grammar before tokenising it, so the
90+
// route selected here is the route `@angular/router` will render.
91+
pathname = stripMatrixParams(normalizeUrlPath(pathname));
9092

9193
return this.routeTree.match(pathname);
9294
}

packages/angular/ssr/src/utils/url.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import { DefaultUrlSerializer } from '@angular/router';
10+
911
/**
1012
* Removes the trailing slash from a URL if it exists.
1113
*
@@ -227,6 +229,48 @@ export function stripMatrixParams(pathname: string): string {
227229
return pathname.includes(';') ? pathname.replace(MATRIX_PARAMS_REGEX, '') : pathname;
228230
}
229231

232+
/**
233+
* A single reusable serializer. `DefaultUrlSerializer` is stateless, so one instance
234+
* is enough for the lifetime of the module.
235+
*/
236+
const URL_SERIALIZER = new DefaultUrlSerializer();
237+
238+
/**
239+
* Rewrites a URL path into the spelling `@angular/router` will resolve it to.
240+
*
241+
* Server route matching tokenises the path by splitting on `/`, while the client
242+
* router parses it with `DefaultUrlSerializer`, a grammar in which `(`, `)`, `;`
243+
* and `//` are metacharacters. The two therefore disagree on inputs such as
244+
* `/page)`, which the router resolves to `/page` and the server route tree treats
245+
* as a distinct segment. Passing the path through the router's own grammar first
246+
* makes both sides agree on which route a request is.
247+
*
248+
* A path the serializer cannot parse is returned unchanged, so malformed
249+
* percent-encoding keeps its existing behaviour.
250+
*
251+
* @param pathname - The URL path to normalize.
252+
* @returns The path as `@angular/router` would resolve it.
253+
*
254+
* @example
255+
* ```ts
256+
* normalizeUrlPath('/page)'); // returns '/page'
257+
* normalizeUrlPath('/(page)'); // returns '/page'
258+
* normalizeUrlPath('/a/1//b'); // returns '/a/1'
259+
* normalizeUrlPath('/page'); // returns '/page'
260+
* ```
261+
*/
262+
export function normalizeUrlPath(pathname: string): string {
263+
try {
264+
const serialized = URL_SERIALIZER.serialize(URL_SERIALIZER.parse(pathname));
265+
// `serialize` reproduces the query string and fragment; only the path is matched.
266+
const queryOrFragment = serialized.search(/[?#]/);
267+
268+
return queryOrFragment === -1 ? serialized : serialized.slice(0, queryOrFragment);
269+
} catch {
270+
return pathname;
271+
}
272+
}
273+
230274
/**
231275
* Constructs a decoded URL string from its components.
232276
*

packages/angular/ssr/test/routes/router_spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,34 @@ describe('ServerRouter', () => {
128128
});
129129
});
130130

131+
it('should select the same route the client router will render', () => {
132+
// `@angular/router` resolves each of these to `/home`, because `(`, `)`, `;`
133+
// and `//` are metacharacters in its URL grammar. Server route matching has to
134+
// agree, or the response's headers, status and renderMode are taken from a
135+
// different route than the one that renders into the body.
136+
const home = {
137+
route: '/home',
138+
renderMode: RenderMode.Server,
139+
};
140+
141+
for (const pathname of ['/home)', '/home(', '/home;', '/(home)']) {
142+
expect(router.match(new URL(`http://localhost${pathname}`)))
143+
.withContext(pathname)
144+
.toEqual(home);
145+
}
146+
147+
// An interior `//` ends the path for the client router, so `/user/123//x`
148+
// renders the `/user/:id` route and must match its server config too.
149+
expect(router.match(new URL('http://localhost/user/123//x'))).toEqual({
150+
route: '/user/*',
151+
renderMode: RenderMode.Server,
152+
});
153+
});
154+
155+
it('should not invent a match for an unknown route', () => {
156+
expect(router.match(new URL('http://localhost/nope'))).toBeUndefined();
157+
});
158+
131159
it('should handle encoded params', () => {
132160
const encodedUserMetadata = router.match(
133161
new URL('http://localhost/user/Bob%20%2F%20Roberts'),

packages/angular/ssr/test/utils/url_spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
addTrailingSlash,
1212
buildPathWithParams,
1313
joinUrlParts,
14+
normalizeUrlPath,
1415
stripIndexHtmlFromURL,
1516
stripLeadingSlash,
1617
stripMatrixParams,
@@ -220,4 +221,42 @@ describe('URL Utils', () => {
220221
expect(stripMatrixParams('')).toBe('');
221222
});
222223
});
224+
describe('normalizeUrlPath', () => {
225+
it('should resolve spellings that `@angular/router` treats as the same route', () => {
226+
// Each left-hand value is what `DefaultUrlSerializer` resolves the path to,
227+
// verified against the published @angular/router 22.1.6.
228+
expect(normalizeUrlPath('/page)')).toBe('/page');
229+
expect(normalizeUrlPath('/page(')).toBe('/page');
230+
expect(normalizeUrlPath('/page;')).toBe('/page');
231+
expect(normalizeUrlPath('/(page)')).toBe('/page');
232+
expect(normalizeUrlPath('/a/1//b')).toBe('/a/1');
233+
expect(normalizeUrlPath('/a/b)c/d')).toBe('/a/b');
234+
});
235+
236+
it('should leave an ordinary path unchanged', () => {
237+
expect(normalizeUrlPath('/page')).toBe('/page');
238+
expect(normalizeUrlPath('/a/b/c')).toBe('/a/b/c');
239+
expect(normalizeUrlPath('/user/123')).toBe('/user/123');
240+
expect(normalizeUrlPath('/')).toBe('/');
241+
});
242+
243+
it('should preserve encoding, including an encoded slash', () => {
244+
expect(normalizeUrlPath('/a%2Fb')).toBe('/a%2Fb');
245+
expect(normalizeUrlPath('/encoding%20url')).toBe('/encoding%20url');
246+
});
247+
248+
it('should preserve matrix parameters so stripMatrixParams still owns them', () => {
249+
expect(normalizeUrlPath('/page;p=1')).toBe('/page;p=1');
250+
});
251+
252+
it('should return a path it cannot parse unchanged', () => {
253+
// Malformed percent-encoding keeps its existing behaviour.
254+
expect(normalizeUrlPath('/%zz')).toBe('/%zz');
255+
});
256+
257+
it('should not alter dot segments or index.html handling', () => {
258+
expect(normalizeUrlPath('/a/./b')).toBe('/a/./b');
259+
expect(normalizeUrlPath('/page/index.html')).toBe('/page/index.html');
260+
});
261+
});
223262
});

0 commit comments

Comments
 (0)