Skip to content

Commit

Permalink
fix(redirect): Do not allow onBefore hooks to cause infinite redire…
Browse files Browse the repository at this point in the history
…ct loops

Closes #6
  • Loading branch information
christopherthielen committed Nov 12, 2016
1 parent 8892ecc commit 5c5f7eb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 7 deletions.
10 changes: 5 additions & 5 deletions src/transition/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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()))
Expand Down
22 changes: 20 additions & 2 deletions test/stateServiceSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()) });
Expand All @@ -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();
});
});
Expand Down

0 comments on commit 5c5f7eb

Please sign in to comment.