fix(router): fall back when serializing protocol-relative URLs · angular/angular@576df28 · GitHub
Skip to content

Commit 576df28

Browse files
SkyZeroZxatscott
authored and
NothingEverHappens
committed
fix(router): fall back when serializing protocol-relative URLs
Avoid throwing when DefaultUrlSerializer encounters a UrlTree that would serialize to a protocol-relative URL. Warn in development mode and serialize the tree with a root-relative path instead while preserving query parameters and fragments. Additionally, account for WHATWG URL normalization (e.g. dot segments and backslashes collapsing into protocol-relative paths such as `/.//` or `/..//`). Fixes #69700 Fixes #71076 Co-authored-by: Andrew Scott <atscott01@gmail.com> (cherry picked from commit a1bffc2)
1 parent 519d446 commit 576df28

7 files changed

Lines changed: 242 additions & 2 deletions

File tree

‎goldens/public-api/router/errors.api.md‎

Lines changed: 2 additions & 0 deletions

‎packages/core/test/bundling/router/bundle.golden_symbols.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"DI_DECORATOR_FLAG",
7575
"DOCUMENT",
7676
"DOCUMENT2",
77+
"DUMMY_BASE_URL",
7778
"DefaultDomRenderer2",
7879
"DefaultRouteReuseStrategy",
7980
"DefaultTitleStrategy",
@@ -862,6 +863,7 @@
862863
"isPositive",
863864
"isPromise",
864865
"isPromise2",
866+
"isProtocolRelative",
865867
"isPublicRouterEvent",
866868
"isReadableStreamLike",
867869
"isRedirect",

‎packages/router/src/errors.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,5 @@ export const enum RuntimeErrorCode {
2929
INFINITE_REDIRECT = 4016,
3030
INVALID_ROUTER_LINK_INPUTS = 4017,
3131
ERROR_PARSING_URL = 4018,
32+
PROTOCOL_RELATIVE_URL_NOT_ALLOWED = 4019,
3233
}

‎packages/router/src/url_tree.ts‎

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

9-
import {computed, ɵRuntimeError as RuntimeError, Service, Signal} from '@angular/core';
9+
import {
10+
computed,
11+
ɵformatRuntimeError as formatRuntimeError,
12+
ɵRuntimeError as RuntimeError,
13+
Service,
14+
Signal,
15+
} from '@angular/core';
1016

1117
import {RuntimeErrorCode} from './errors';
1218
import type {Router} from './router';
@@ -472,7 +478,18 @@ export class DefaultUrlSerializer implements UrlSerializer {
472478

473479
/** Converts a `UrlTree` into a url */
474480
serialize(tree: UrlTree): string {
475-
const segment = `/${serializeSegment(tree.root, true)}`;
481+
let segment = `/${serializeSegment(tree.root, true)}`;
482+
if (isProtocolRelative(segment)) {
483+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
484+
console.warn(
485+
formatRuntimeError(
486+
RuntimeErrorCode.PROTOCOL_RELATIVE_URL_NOT_ALLOWED,
487+
`Cannot serialize a UrlTree that would produce a protocol-relative URL. Falling back to '/' instead.`,
488+
),
489+
);
490+
}
491+
segment = '/';
492+
}
476493
const query = serializeQueryParams(tree.queryParams);
477494
const fragment =
478495
typeof tree.fragment === `string` ? `#${encodeUriFragment(tree.fragment)}` : '';
@@ -481,6 +498,23 @@ export class DefaultUrlSerializer implements UrlSerializer {
481498
}
482499
}
483500

501+
const DUMMY_BASE_URL = 'http://fake';
502+
503+
/**
504+
* Determines whether a serialized path would produce a protocol-relative URL when interpreted
505+
* by a browser or server. Under the WHATWG URL standard, paths starting with `//` or `/\`, or paths
506+
* where leading dot segments collapse to `//` (such as `/.//` or `/..//`), resolve to an external
507+
* origin or a protocol-relative pathname.
508+
*/
509+
function isProtocolRelative(url: string): boolean {
510+
try {
511+
const resolved = new URL(url, DUMMY_BASE_URL);
512+
return resolved.origin !== DUMMY_BASE_URL || resolved.pathname.startsWith('//');
513+
} catch {
514+
return true;
515+
}
516+
}
517+
484518
const DEFAULT_SERIALIZER = new DefaultUrlSerializer();
485519

486520
export function serializePaths(segment: UrlSegmentGroup): string {

‎packages/router/test/create_url_tree.spec.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ import {timeout} from '@angular/private/testing';
2222

2323
describe('createUrlTree', () => {
2424
const serializer = new DefaultUrlSerializer();
25+
const protocolRelativeUrlWarning = `NG04019: Cannot serialize a UrlTree that would produce a protocol-relative URL. Falling back to '/' instead.`;
26+
27+
function expectProtocolRelativeUrlFallback(tree: UrlTree, expected = '/'): void {
28+
const warn = spyOn(console, 'warn');
29+
30+
expect(serializer.serialize(tree)).toEqual(expected);
31+
expect(warn).toHaveBeenCalledOnceWith(protocolRelativeUrlWarning);
32+
}
33+
2534
let router: Router;
2635
beforeEach(() => {
2736
router = TestBed.inject(Router);
@@ -134,6 +143,83 @@ describe('createUrlTree', () => {
134143
expect(serializer.serialize(t)).toEqual('/%2Fone/two%2Fthree');
135144
});
136145

146+
describe('leading empty path commands', () => {
147+
it('should fall back for absolute navigations that would serialize as protocol-relative', () => {
148+
const t = router.createUrlTree(['/', '', '', 'attacker.example', 'collect']);
149+
150+
expectProtocolRelativeUrlFallback(t);
151+
});
152+
153+
it('should fall back for an unsafe primary outlet string', async () => {
154+
await router.navigateByUrl('/safe');
155+
const t = router.createUrlTree([{outlets: {primary: '/attacker.example/collect'}}]);
156+
157+
expectProtocolRelativeUrlFallback(t);
158+
});
159+
160+
it('should fall back for an unsafe primary outlet array', () => {
161+
const t = router.createUrlTree([{outlets: {primary: ['', 'attacker.example', 'collect']}}]);
162+
163+
expectProtocolRelativeUrlFallback(t);
164+
});
165+
166+
it('should fall back for unsafe parent-relative commands', async () => {
167+
router.resetConfig([{path: 'source', component: class {}}]);
168+
await router.navigateByUrl('/source');
169+
const t = create(router.routerState.root.firstChild!, [
170+
'../',
171+
'',
172+
'attacker.example',
173+
'collect',
174+
]);
175+
176+
expectProtocolRelativeUrlFallback(t);
177+
});
178+
179+
it('should fall back for an escaped slash after an empty path command', () => {
180+
const t = router.createUrlTree(['/', '', {segmentPath: '/'}]);
181+
182+
expectProtocolRelativeUrlFallback(t);
183+
});
184+
185+
it('should fall back for final empty path commands', () => {
186+
const t = router.createUrlTree(['/', '', '']);
187+
188+
expectProtocolRelativeUrlFallback(t);
189+
});
190+
191+
it('should fall back for dot segments that normalize to protocol-relative', () => {
192+
const t = router.createUrlTree(['/', '.', '', 'attacker.example', 'collect']);
193+
194+
expectProtocolRelativeUrlFallback(t);
195+
});
196+
197+
it('should fall back for parent dot segments that normalize to protocol-relative', () => {
198+
const t = router.createUrlTree(['/', '..', '', 'attacker.example', 'collect']);
199+
200+
expectProtocolRelativeUrlFallback(t);
201+
});
202+
203+
it('should preserve query params and fragment when falling back', () => {
204+
const t = router.createUrlTree(['/', '', '', 'attacker.example', 'collect'], {
205+
queryParams: {foo: 'bar'},
206+
fragment: 'frag',
207+
});
208+
209+
expectProtocolRelativeUrlFallback(t, '/?foo=bar#frag');
210+
});
211+
212+
it('should not normalize a leading empty path command in a secondary outlet', () => {
213+
const t = router.createUrlTree(['/', {outlets: {right: ['', 'child']}}]);
214+
215+
expect(t.root.children['right'].segments.map((segment) => segment.path)).toEqual([
216+
'',
217+
'child',
218+
]);
219+
expect(serializer.serialize(t)).toEqual('/(right:/child)');
220+
});
221+
});
222+
137223
describe('named outlets', () => {
138224
it('should preserve secondary segments', async () => {
139225
const p = serializer.parse('/a/11/b(right:c)');

‎packages/router/test/router_link_spec.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,4 +329,52 @@ describe('RouterLink', () => {
329329
await harness.navigateByUrl('/different');
330330
expect(anchor.getAttribute('href')).toBe('/different/child');
331331
});
332+
333+
it('falls back to the root for a link that would generate a protocol-relative href', async () => {
334+
@Component({
335+
template: `<a [routerLink]="commands" queryParamsHandling="preserve">commands</a>`,
336+
imports: [RouterLink],
337+
})
338+
class WithLink {
339+
readonly commands = ['/', '', 'attacker.example', 'collect'];
340+
}
341+
342+
TestBed.configureTestingModule({
343+
providers: [provideRouter([{path: '', component: WithLink}])],
344+
});
345+
const warn = spyOn(console, 'warn');
346+
const fixture = TestBed.createComponent(WithLink);
347+
348+
await fixture.whenStable();
349+
350+
expect(fixture.nativeElement.querySelector('a').getAttribute('href')).toBe('/');
351+
expect(warn).toHaveBeenCalledWith(
352+
`NG04019: Cannot serialize a UrlTree that would produce a protocol-relative URL. Falling back to '/' instead.`,
353+
);
354+
});
355+
356+
it('preserves query params and fragment when falling back for a protocol-relative link', async () => {
357+
@Component({
358+
template: `<a [routerLink]="commands" [queryParams]="{ref: '123'}" fragment="section"
359+
>commands</a
360+
>`,
361+
imports: [RouterLink],
362+
})
363+
class WithLink {
364+
readonly commands = ['/', '', 'attacker.example', 'collect'];
365+
}
366+
367+
TestBed.configureTestingModule({
368+
providers: [provideRouter([{path: '', component: WithLink}])],
369+
});
370+
const warn = spyOn(console, 'warn');
371+
const fixture = TestBed.createComponent(WithLink);
372+
373+
await fixture.whenStable();
374+
375+
expect(fixture.nativeElement.querySelector('a').getAttribute('href')).toBe('/?ref=123#section');
376+
expect(warn).toHaveBeenCalledWith(
377+
`NG04019: Cannot serialize a UrlTree that would produce a protocol-relative URL. Falling back to '/' instead.`,
378+
);
379+
});
332380
});

‎packages/router/test/url_serializer.spec.ts‎

Lines changed: 67 additions & 0 deletions

0 commit comments

Comments
 (0)