From cb1d70be36c3ae94352a3ad59d36a84eaa2b1424 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:46:52 -0500 Subject: [PATCH] fix(router): limit protocol-relative URL handling to serialization Preserve createUrlTree command semantics, including custom serializer inputs, while keeping the single-leading-slash guarantee at the default serialization boundary. Expand coverage for command forms, public UrlTree values, secondary outlets, and preserved query parameters and fragments. Fixes #69700 --- packages/router/src/navigation_transition.ts | 7 +- packages/router/src/url_tree.ts | 26 +++- packages/router/test/create_url_tree.spec.ts | 56 +++++++++ .../test/integration/navigation.spec.ts | 12 ++ packages/router/test/router_link_spec.ts | 111 +++++++++++++++++- packages/router/test/url_serializer.spec.ts | 30 +++++ 6 files changed, 238 insertions(+), 4 deletions(-) diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts index 721e93e22986..6aa4ec50d447 100644 --- a/packages/router/src/navigation_transition.ts +++ b/packages/router/src/navigation_transition.ts @@ -79,7 +79,7 @@ import { } from './router_state'; import type {Params} from './shared'; import {UrlHandlingStrategy} from './url_handling_strategy'; -import {UrlSerializer, UrlTree} from './url_tree'; +import {canonicalizeUrlTree, UrlSerializer, UrlTree} from './url_tree'; import {abortSignalToObservable} from './utils/abort_signal_to_observable'; import {Checks, getAllRouteGuards} from './utils/preactivation'; import {CREATE_VIEW_TRANSITION} from './utils/view_transition'; @@ -428,7 +428,10 @@ export class NavigationTransitions { untracked(() => { this.transitions?.next({ ...request, - extractedUrl: this.urlHandlingStrategy.extract(request.rawUrl), + extractedUrl: canonicalizeUrlTree( + this.urlHandlingStrategy.extract(request.rawUrl), + this.urlSerializer, + ), targetSnapshot: null, targetRouterState: null, guards: {canActivateChecks: [], canDeactivateChecks: []}, diff --git a/packages/router/src/url_tree.ts b/packages/router/src/url_tree.ts index ee6853cc0872..4e7a81d553f3 100644 --- a/packages/router/src/url_tree.ts +++ b/packages/router/src/url_tree.ts @@ -453,7 +453,9 @@ export class DefaultUrlSerializer implements UrlSerializer { /** Converts a `UrlTree` into a url */ serialize(tree: UrlTree): string { - const segment = `/${serializeSegment(tree.root, true)}`; + // A public UrlTree can contain a leading empty primary segment. Always emit a root-relative URL + // rather than allowing the serialized path to become protocol-relative. + const segment = `/${serializeSegment(tree.root, true).replace(/^\/+/, '')}`; const query = serializeQueryParams(tree.queryParams); const fragment = typeof tree.fragment === `string` ? `#${encodeUriFragment(tree.fragment)}` : ''; @@ -464,6 +466,28 @@ export class DefaultUrlSerializer implements UrlSerializer { const DEFAULT_SERIALIZER = new DefaultUrlSerializer(); +/** + * Reparse trees whose leading empty segments are collapsed by the default serializer so route + * recognition and the browser URL use the same segment structure. + */ +export function canonicalizeUrlTree(tree: UrlTree, serializer: UrlSerializer): UrlTree { + const primary = tree.root.children[PRIMARY_OUTLET]; + const startsWithEmptyPath = + primary !== undefined && + (primary.segments.length > 0 + ? serializePath(primary.segments[0]) === '' + : primary.hasChildren()); + if (!startsWithEmptyPath) { + return tree; + } + + const {queryParams, fragment} = tree; + const canonical = serializer.parse(serializer.serialize(tree)); + canonical.queryParams = queryParams; + canonical.fragment = fragment; + return canonical; +} + export function serializePaths(segment: UrlSegmentGroup): string { return segment.segments.map((p) => serializePath(p)).join('/'); } diff --git a/packages/router/test/create_url_tree.spec.ts b/packages/router/test/create_url_tree.spec.ts index 8132136d1ed5..2820587109c8 100644 --- a/packages/router/test/create_url_tree.spec.ts +++ b/packages/router/test/create_url_tree.spec.ts @@ -134,6 +134,62 @@ describe('createUrlTree', () => { expect(serializer.serialize(t)).toEqual('/%2Fone/two%2Fthree'); }); + describe('leading empty path command serialization', () => { + it('should serialize absolute navigations with one leading slash', () => { + const t = router.createUrlTree(['/', '', '', 'attacker.example', 'collect']); + + expect(serializer.serialize(t)).toEqual('/attacker.example/collect'); + }); + + it('should serialize a primary outlet string with one leading slash', async () => { + await router.navigateByUrl('/safe'); + const t = router.createUrlTree([{outlets: {primary: '/attacker.example/collect'}}]); + + expect(serializer.serialize(t)).toEqual('/attacker.example/collect'); + }); + + it('should serialize a primary outlet array with one leading slash', () => { + const t = router.createUrlTree([{outlets: {primary: ['', 'attacker.example', 'collect']}}]); + + expect(serializer.serialize(t)).toEqual('/attacker.example/collect'); + }); + + it('should serialize parent-relative commands with one leading slash', async () => { + router.resetConfig([{path: 'source', component: class {}}]); + await router.navigateByUrl('/source'); + const t = create(router.routerState.root.firstChild!, [ + '../', + '', + 'attacker.example', + 'collect', + ]); + + expect(serializer.serialize(t)).toEqual('/attacker.example/collect'); + }); + + it('should preserve an escaped slash after an empty path command', () => { + const t = router.createUrlTree(['/', '', {segmentPath: '/'}]); + + expect(serializer.serialize(t)).toEqual('/%2F'); + }); + + it('should serialize final empty path commands as the root URL', () => { + const t = router.createUrlTree(['/', '', '']); + + expect(serializer.serialize(t)).toEqual('/'); + }); + + it('should not normalize a leading empty path command in a secondary outlet', () => { + const t = router.createUrlTree(['/', {outlets: {right: ['', 'child']}}]); + + expect(t.root.children['right'].segments.map((segment) => segment.path)).toEqual([ + '', + 'child', + ]); + expect(serializer.serialize(t)).toEqual('/(right:/child)'); + }); + }); + describe('named outlets', () => { it('should preserve secondary segments', async () => { const p = serializer.parse('/a/11/b(right:c)'); diff --git a/packages/router/test/integration/navigation.spec.ts b/packages/router/test/integration/navigation.spec.ts index 791907878dfc..de372aaec045 100644 --- a/packages/router/test/integration/navigation.spec.ts +++ b/packages/router/test/integration/navigation.spec.ts @@ -60,6 +60,18 @@ export function navigationIntegrationTestSuite(browserAPI: 'history' | 'navigati return TestBed.inject(Router); } describe('navigation', () => { + it('recognizes the route represented by the serialized URL', async () => { + const router = setup([ + {path: ':tenant/admin', component: SimpleCmp}, + {path: 'admin', component: SimpleCmp}, + ]); + + await router.navigateByUrl('/(primary://admin)'); + + expect(router.url).toBe('/admin'); + expect(router.routerState.snapshot.root.firstChild?.routeConfig?.path).toBe('admin'); + }); + it('should navigate to the current URL', async () => { TestBed.configureTestingModule({ providers: [provideRouter([], withRouterConfig({onSameUrlNavigation: 'reload'}))], diff --git a/packages/router/test/router_link_spec.ts b/packages/router/test/router_link_spec.ts index c675a03a561e..cc4a7681cca6 100644 --- a/packages/router/test/router_link_spec.ts +++ b/packages/router/test/router_link_spec.ts @@ -9,7 +9,14 @@ import {Component, inject, signal} from '@angular/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -import {Router, RouterLink, RouterModule, provideRouter} from '../index'; +import { + PRIMARY_OUTLET, + Router, + RouterLink, + RouterModule, + UrlSegment, + provideRouter, +} from '../index'; import {RouterTestingHarness} from '../testing'; describe('RouterLink', () => { @@ -329,4 +336,106 @@ describe('RouterLink', () => { await harness.navigateByUrl('/different'); expect(anchor.getAttribute('href')).toBe('/different/child'); }); + + it('does not generate protocol-relative hrefs from leading empty primary segments', async () => { + @Component({ + template: ` + commands + + outlet string + + + outlet array + + UrlTree + `, + imports: [RouterLink], + }) + class WithLink { + readonly commands = ['/', '', 'attacker.example', 'collect']; + readonly outletString = [{outlets: {primary: '/attacker.example/collect'}}]; + readonly outletArray = [{outlets: {primary: ['', 'attacker.example', 'collect']}}]; + readonly pageQueryParams = {page: 1}; + readonly urlTree = inject(Router).parseUrl( + '/attacker.example/collect?token=RESET_TOKEN#OAUTH_TOKEN', + ); + + constructor() { + // UrlTree segments are public, so this mutation exercises the serializer backstop. + this.urlTree.root.children[PRIMARY_OUTLET].segments.unshift(new UrlSegment('', {})); + } + } + + TestBed.configureTestingModule({ + providers: [provideRouter([{path: '', component: WithLink}])], + }); + const harness = await RouterTestingHarness.create('/?token=RESET_TOKEN#OAUTH_TOKEN'); + + expect(harness.fixture.nativeElement.querySelector('#commands').getAttribute('href')).toBe( + '/attacker.example/collect?token=RESET_TOKEN', + ); + expect(harness.fixture.nativeElement.querySelector('#outlet-string').getAttribute('href')).toBe( + '/attacker.example/collect?token=RESET_TOKEN&page=1', + ); + expect(harness.fixture.nativeElement.querySelector('#outlet-array').getAttribute('href')).toBe( + '/attacker.example/collect?token=RESET_TOKEN#OAUTH_TOKEN', + ); + expect(harness.fixture.nativeElement.querySelector('#url-tree').getAttribute('href')).toBe( + '/attacker.example/collect?token=RESET_TOKEN#OAUTH_TOKEN', + ); + }); + + it('navigates to the route represented by the href', async () => { + const tenantAdminGuard = jasmine.createSpy().and.returnValue(true); + const strictAdminGuard = jasmine.createSpy().and.returnValue(true); + + @Component({template: ''}) + class RoutedComponent {} + + @Component({ + template: ` + + admin + + `, + imports: [RouterLink], + }) + class WithLink {} + + TestBed.configureTestingModule({ + providers: [ + provideRouter([ + { + path: ':tenant/admin', + canActivate: [tenantAdminGuard], + component: RoutedComponent, + }, + {path: 'admin', canActivate: [strictAdminGuard], component: RoutedComponent}, + ]), + ], + }); + const fixture = TestBed.createComponent(WithLink); + await fixture.whenStable(); + + const anchor: HTMLAnchorElement = fixture.nativeElement.querySelector('a'); + expect(anchor.getAttribute('href')).toBe('/admin?token=secret#fragment'); + anchor.click(); + await fixture.whenStable(); + + const router = TestBed.inject(Router); + expect(router.url).toBe('/admin?token=secret#fragment'); + expect(router.routerState.snapshot.root.firstChild?.routeConfig?.path).toBe('admin'); + expect(strictAdminGuard).toHaveBeenCalled(); + expect(tenantAdminGuard).not.toHaveBeenCalled(); + }); }); diff --git a/packages/router/test/url_serializer.spec.ts b/packages/router/test/url_serializer.spec.ts index 70329510ab0b..f524454d6053 100644 --- a/packages/router/test/url_serializer.spec.ts +++ b/packages/router/test/url_serializer.spec.ts @@ -13,6 +13,7 @@ import { encodeUriQuery, encodeUriSegment, serializePath, + UrlSegment, UrlSegmentGroup, } from '../src/url_tree'; @@ -447,6 +448,35 @@ describe('url serializer', () => { }); }); + describe('leading empty path segments', () => { + it('should not serialize a parsed primary outlet as a protocol-relative URL', () => { + const tree = url.parse('/(primary://attacker.example/collect)?token=RESET_TOKEN'); + + expect(url.serialize(tree)).toEqual('/attacker.example/collect?token=RESET_TOKEN'); + }); + + it('should emit one slash for multiple leading empty primary segments', () => { + const tree = url.parse('/attacker.example/collect'); + tree.root.children[PRIMARY_OUTLET].segments.unshift( + new UrlSegment('', {}), + new UrlSegment('', {}), + ); + + expect(url.serialize(tree)).toEqual('/attacker.example/collect'); + }); + + it('should preserve secondary outlets, query params, and fragments when serializing', () => { + const tree = url.parse( + '/attacker.example/collect(popup:compose)?token=RESET_TOKEN#OAUTH_TOKEN', + ); + tree.root.children[PRIMARY_OUTLET].segments.unshift(new UrlSegment('', {})); + + expect(url.serialize(tree)).toEqual( + '/attacker.example/collect(popup:compose)?token=RESET_TOKEN#OAUTH_TOKEN', + ); + }); + }); + describe('error handling', () => { it('should throw when invalid characters inside children', () => { expect(() => url.parse('/one/(left#one)')).toThrowError();