feat(warn): warn against infinite redirections

posva · posva · commit e3dcc8d9477e · 2020-08-06T16:10:10.000+02:00
diff --git a/__tests__/warnings.spec.ts b/__tests__/warnings.spec.ts
@@ -187,4 +187,24 @@ describe('warnings', () => {
     await expect(router.push({ path: '/foo2' })).resolves.toBe(undefined)
     expect(`No match found for location with path "/foo2"`).toHaveBeenWarned()
   })
+
+  it('warns if next is called with the same location too many times', async () => {
+    let router = createRouter({
+      history: createMemoryHistory(),
+      routes: [
+        { path: '/', name: 'a', component },
+        { path: '/b', component },
+      ],
+    })
+
+    router.beforeEach(to => {
+      if (to.path === '/b') return '/b'
+      return
+    })
+
+    await router.push('/b').catch(() => {})
+    expect(
+      'Detected an infinite redirection in a navigation guard when going from "/" to "/b"'
+    ).toHaveBeenWarned()
+  })
 })
diff --git a/src/navigationGuards.ts b/src/navigationGuards.ts
@@ -157,9 +157,6 @@ export function guardToPromiseFn(
           record && record.instances[name!],
           to,
           from,
-          // TODO: could wrap in dev to check if the guard returns before
-          // calling next with 3 or more arguments. This would help people
-          // forgetting to remove the `next` argument
           __DEV__ ? canOnlyBeCalledOnce(next, to, from) : next
         )
       )
diff --git a/src/router.ts b/src/router.ts
@@ -490,17 +490,42 @@ export function createRouter(options: RouterOptions): Router {
         if (failure) {
           if (
             isNavigationFailure(failure, ErrorTypes.NAVIGATION_GUARD_REDIRECT)
-          )
-            // preserve the original redirectedFrom if any
+          ) {
+            if (
+              __DEV__ &&
+              // we are redirecting to the same location we were already at
+              isSameRouteLocation(
+                stringifyQuery,
+                resolve(failure.to),
+                toLocation
+              ) &&
+              // and we have done it a couple of times
+              redirectedFrom &&
+              // @ts-ignore
+              (redirectedFrom._count = redirectedFrom._count
+                ? // @ts-ignore
+                  redirectedFrom._count + 1
+                : 1) > 10
+            ) {
+              warn(
+                `Detected an infinite redirection in a navigation guard when going from "${from.fullPath}" to "${toLocation.fullPath}". Aborting to avoid a Stack Overflow. This will break in production if not fixed.`
+              )
+              return Promise.reject(
+                new Error('Infinite redirect in navigation guard')
+              )
+            }
+
             return pushWithRedirect(
               // keep options
               assign(locationAsObject(failure.to), {
                 state: data,
                 force,
                 replace,
               }),
+              // preserve the original redirectedFrom if any
               redirectedFrom || toLocation
             )
+          }
         } else {
           // if we fail we don't finalize the navigation
           failure = finalizeNavigation(

Original file line number	Diff line number	Diff line change
`@@ -157,9 +157,6 @@ export function guardToPromiseFn(`
`157`	`157`	`record && record.instances[name!],`
`158`	`158`	`to,`
`159`	`159`	`from,`
`160`		`- // TODO: could wrap in dev to check if the guard returns before`
`161`		`- // calling next with 3 or more arguments. This would help people`
`162`		- // forgetting to remove the `next` argument
`163`	`160`	`__DEV__ ? canOnlyBeCalledOnce(next, to, from) : next`
`164`	`161`	`)`
`165`	`162`	`)`