From 5c5f7eb52455c4db94420c84b117830a4aa88f50 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Sat, 12 Nov 2016 15:32:21 -0600 Subject: [PATCH] fix(redirect): Do not allow `onBefore` hooks to cause infinite redirect loops Closes #6 --- src/transition/transition.ts | 10 +++++----- test/stateServiceSpec.ts | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/transition/transition.ts b/src/transition/transition.ts index c2866cd9..1b901015 100644 --- a/src/transition/transition.ts +++ b/src/transition/transition.ts @@ -442,6 +442,11 @@ export class Transition implements IHookRegistry { * @returns Returns a new [[Transition]] instance. */ redirect(targetState: TargetState): Transition { + let redirects = 1, trans: Transition = this; + while((trans = trans.redirectedFrom()) != null) { + if (++redirects > 20) throw new Error(`Too many consecutive Transition redirects (20+)`); + } + let newOptions = extend({}, this.options(), targetState.options(), { redirectedFrom: this, source: "redirect" }); targetState = new TargetState(targetState.identifier(), targetState.$state(), targetState.params(), newOptions); @@ -613,11 +618,6 @@ export class Transition implements IHookRegistry { error() { let state: State = this.$to(); - let redirects = 0, trans: Transition = this; - while((trans = trans.redirectedFrom()) != null) { - if (++redirects > 20) return `Too many Transition redirects (20+)`; - } - if (state.self.abstract) return `Cannot transition to abstract state '${state.name}'`; if (!Param.validates(state.parameters(), this.params())) diff --git a/test/stateServiceSpec.ts b/test/stateServiceSpec.ts index e5d0e6e8..2c668bae 100644 --- a/test/stateServiceSpec.ts +++ b/test/stateServiceSpec.ts @@ -59,7 +59,7 @@ describe('stateService', function () { .then(done, done); })); - it('should error after 20+ redirects', (done) => { + it('should error after 20+ async redirects', (done) => { let errors = []; $transitions.onEnter({ entering: "D" }, trans => trans.router.stateService.target('D')); $transitions.onError({}, trans => { errors.push(trans.error()) }); @@ -68,7 +68,25 @@ describe('stateService', function () { $state.go("D").catch(err => { expect(errors.length).toBe(21); - expect(err.message).toContain('Too many Transition redirects'); + expect(err.message).toContain('Too many consecutive Transition redirects'); + done(); + }); + }); + + it('synchronous redirects should not be allowed to cause an infinite redirect loop', (done) => { + let errors = []; + let count = 0; + $transitions.onBefore({ entering: "D" }, trans => { + if (count++ >= 1000) throw new Error(`Doh! 1000 redirects O_o`); + return trans.router.stateService.target('D'); + }); + $transitions.onError({}, trans => { errors.push(trans.error()) }); + + $state.defaultErrorHandler(function() {}); + + $state.go("D").catch(err => { + expect(count).toBe(21); + expect(err.message).toContain('Too many consecutive Transition redirects'); done(); }); });