diff --git a/goldens/public-api/router/errors.md b/goldens/public-api/router/errors.md index 91ca86d2f1d14..e7a04e60a449d 100644 --- a/goldens/public-api/router/errors.md +++ b/goldens/public-api/router/errors.md @@ -11,6 +11,8 @@ export const enum RuntimeErrorCode { // (undocumented) FOR_ROOT_CALLED_TWICE = 4007, // (undocumented) + INFINITE_REDIRECT = 4016, + // (undocumented) INVALID_DOUBLE_DOTS = 4005, // (undocumented) INVALID_ROOT_URL_SEGMENT = 4015, diff --git a/packages/router/src/errors.ts b/packages/router/src/errors.ts index 1efbec6695239..7cad7518c99db 100644 --- a/packages/router/src/errors.ts +++ b/packages/router/src/errors.ts @@ -26,4 +26,5 @@ export const enum RuntimeErrorCode { OUTLET_ALREADY_ACTIVATED = 4013, INVALID_ROUTE_CONFIG = 4014, INVALID_ROOT_URL_SEGMENT = 4015, + INFINITE_REDIRECT = 4016, } diff --git a/packages/router/src/models.ts b/packages/router/src/models.ts index c203aed435cf1..39138a260a8e3 100644 --- a/packages/router/src/models.ts +++ b/packages/router/src/models.ts @@ -519,7 +519,6 @@ export interface Route { * A URL to redirect to when the path matches. * * Absolute if the URL begins with a slash (/), otherwise relative to the path URL. - * Note that no further redirects are evaluated after an absolute redirect. * * When not present, router does not redirect. */ diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts index d974691df6dac..50eeeeef51757 100644 --- a/packages/router/src/recognize.ts +++ b/packages/router/src/recognize.ts @@ -10,7 +10,7 @@ import {EnvironmentInjector, Type, ɵRuntimeError as RuntimeError} from '@angula import {from, Observable, of} from 'rxjs'; import {catchError, concatMap, defaultIfEmpty, first, last as rxjsLast, map, mergeMap, scan, switchMap, tap} from 'rxjs/operators'; -import {absoluteRedirect, AbsoluteRedirect, ApplyRedirects, canLoadFails, noMatch, NoMatch} from './apply_redirects'; +import {AbsoluteRedirect, ApplyRedirects, canLoadFails, noMatch, NoMatch} from './apply_redirects'; import {createUrlTreeFromSnapshot} from './create_url_tree'; import {RuntimeErrorCode} from './errors'; import {Data, LoadedRouterConfig, ResolveData, Route, Routes} from './models'; @@ -44,9 +44,12 @@ export function recognize( .recognize(); } +const MAX_ALLOWED_REDIRECTS = 31; + export class Recognizer { - allowRedirects = true; private applyRedirects = new ApplyRedirects(this.urlSerializer, this.urlTree); + private absoluteRedirectCount = 0; + allowRedirects = true; constructor( private injector: EnvironmentInjector, private configLoader: RouterConfigLoader, @@ -86,13 +89,11 @@ export class Recognizer { } - private match(root: UrlSegmentGroup): Observable[]> { - const expanded$ = this.processSegmentGroup(this.injector, this.config, root, PRIMARY_OUTLET); + private match(rootSegmentGroup: UrlSegmentGroup): Observable[]> { + const expanded$ = + this.processSegmentGroup(this.injector, this.config, rootSegmentGroup, PRIMARY_OUTLET); return expanded$.pipe(catchError((e: any) => { if (e instanceof AbsoluteRedirect) { - // After an absolute redirect we do not apply any more redirects! - // If this implementation changes, update the documentation note in `redirectTo`. - this.allowRedirects = false; this.urlTree = e.urlTree; return this.match(e.urlTree.root); } @@ -216,7 +217,7 @@ export class Recognizer { return this.matchSegmentAgainstRoute(injector, rawSegment, route, segments, outlet); } - if (allowRedirects && this.allowRedirects) { + if (this.allowRedirects && allowRedirects) { return this.expandSegmentAgainstRouteUsingRedirect( injector, rawSegment, routes, route, segments, outlet); } @@ -237,6 +238,22 @@ export class Recognizer { match(segmentGroup, route, segments); if (!matched) return noMatch(segmentGroup); + // TODO(atscott): Move all of this under an if(ngDevMode) as a breaking change and allow stack + // size exceeded in production + if (route.redirectTo!.startsWith('/')) { + this.absoluteRedirectCount++; + if (this.absoluteRedirectCount > MAX_ALLOWED_REDIRECTS) { + if (ngDevMode) { + throw new RuntimeError( + RuntimeErrorCode.INFINITE_REDIRECT, + `Detected possible infinite redirect when redirecting from '${this.urlTree}' to '${ + route.redirectTo}'.\n` + + `This is currently a dev mode only error but will become a` + + ` call stack size exceeded error in production in a future major version.`); + } + this.allowRedirects = false; + } + } const newTree = this.applyRedirects.applyRedirectCommands( consumedSegments, route.redirectTo!, positionalParamSegments); diff --git a/packages/router/test/apply_redirects.spec.ts b/packages/router/test/apply_redirects.spec.ts index f4c1639d2cc3d..e1bb3500316f3 100644 --- a/packages/router/test/apply_redirects.spec.ts +++ b/packages/router/test/apply_redirects.spec.ts @@ -176,6 +176,19 @@ describe('redirects', () => { }); }); + it('should throw an error on infinite absolute redirect', () => { + recognize( + TestBed.inject(EnvironmentInjector), TestBed.inject(RouterConfigLoader), null, + [{path: '**', redirectTo: '/404'}], tree('/'), new DefaultUrlSerializer()) + .subscribe({ + next: () => fail('expected infinite redirect error'), + error: e => { + expect((e as Error).message).toMatch(/infinite redirect/); + } + }); + }); + + it('should support absolute redirects', () => { checkRedirect( [ @@ -1216,8 +1229,9 @@ describe('redirects', () => { it('should work when using absolute redirects (wildcard)', () => { checkRedirect( [ - {path: '**', redirectTo: '/b(aux:c)'}, {path: 'b', component: ComponentB}, - {path: 'c', component: ComponentC, outlet: 'aux'} + {path: 'b', component: ComponentB}, + {path: 'c', component: ComponentC, outlet: 'aux'}, + {path: '**', redirectTo: '/b(aux:c)'}, ], 'a/1', (t: UrlTree) => { expectTreeToBe(t, '/b(aux:c)');