Add warning for invalid href being passed to router (#8231)

* Add warning for bad href being passed to router

* Apply suggestions from code review
Co-Authored-By: NJoe Haddad <timer150@gmail.com>

* Inline invalidHref for better code elimination

errors/invalid-href-passed.md

+# Invalid href passed to router
+
+#### Why This Error Occurred
+
+Next.js provides a router which can be utilized via a component imported via `next/link`, a wrapper `withRouter(Component)`, and now a hook `useRouter()`. 
+When using any of these, it is expected they are only used for internal navigation, i.e. navigating between pages in the same Next.js application.
+
+Either you passed a non-internal `href` to a `next/link` component or you called `Router#push` or `Router#replace` with one.
+
+Invalid `href`s include external sites (https://google.com) and `mailto:` links. In the past, usage of these invalid `href`s could have gone unnoticed but since they can cause unexpected behavior.
+We now show a warning in development for them.
+
+#### Possible Ways to Fix It
+
+Look for any usage of `next/link` or `next/router` that is being passed a non-internal `href` and replace them with either an anchor tag (`<a>`) or `window.location.href = YOUR_HREF`.
+
+### Useful Links
+
+- [Routing section in Documentation](https://nextjs.org/docs#routing)

packages/next-server/lib/router/router.ts

@@ -277,7 +277,16 @@ export default class Router implements BaseRouter {
         return resolve(true)
       }
 
-      const { pathname, query } = parse(url, true)
+      const { pathname, query, protocol } = parse(url, true)
+
+      if (!pathname || protocol) {
+        if (process.env.NODE_ENV !== 'production') {
+          throw new Error(
+            `Invalid href passed to router: ${url} https://err.sh/zeit/next.js/invalid-href-passed`
+          )
+        }
+        return resolve(false)
+      }
 
       // If asked to change the current URL we should reload the current page
       // (not location.reload() but reload getInitialProps and other Next.js stuffs)
@@ -326,7 +335,7 @@ export default class Router implements BaseRouter {
           const appComp: any = this.components['/_app'].Component
           ;(window as any).next.isPrerendered =
             appComp.getInitialProps === appComp.origGetInitialProps &&
-            !routeInfo.Component.getInitialProps
+            !(routeInfo.Component as any).getInitialProps
         }
 
         // @ts-ignore pathname is always defined
@@ -536,10 +545,18 @@ export default class Router implements BaseRouter {
    */
   prefetch(url: string): Promise<void> {
     return new Promise((resolve, reject) => {
+      const { pathname, protocol } = parse(url)
+
+      if (!pathname || protocol) {
+        if (process.env.NODE_ENV !== 'production') {
+          throw new Error(
+            `Invalid href passed to router: ${url} https://err.sh/zeit/next.js/invalid-href-passed`
+          )
+        }
+        return
+      }
       // Prefetch is not supported in development mode because it would trigger on-demand-entries
       if (process.env.NODE_ENV !== 'production') return
-
-      const { pathname } = parse(url)
       // @ts-ignore pathname is always defined
       const route = toRoute(pathname)
       this.pageLoader.prefetch(route).then(resolve, reject)

packages/next/client/link.tsx

@@ -152,7 +152,7 @@ class Link extends Component<LinkProps> {
     let { href, as } = this.formatUrls(this.props.href, this.props.as)
 
     if (!isLocal(href)) {
-      // ignore click if it's outside our scope
+      // ignore click if it's outside our scope (e.g. https://google.com)
       return
     }
 
@@ -171,14 +171,13 @@ class Link extends Component<LinkProps> {
     // replace state instead of push if prop is present
     Router[this.props.replace ? 'replace' : 'push'](href, as, {
       shallow: this.props.shallow,
+    }).then((success: boolean) => {
+      if (!success) return
+      if (scroll) {
+        window.scrollTo(0, 0)
+        document.body.focus()
+      }
     })
-      .then((success: boolean) => {
-        if (!success) return
-        if (scroll) {
-          window.scrollTo(0, 0)
-          document.body.focus()
-        }
-      })
   }
 
   prefetch() {

test/integration/invalid-href/pages/first.js

+import Link from 'next/link'
+import { useRouter } from 'next/router'
+
+const invalidLink = 'mailto:idk@idk.com'
+
+export default () => {
+  const { query, ...router } = useRouter()
+  const { method } = query
+
+  return method ? (
+    <a
+      id='click-me'
+      onClick={e => {
+        e.preventDefault()
+        router[method](invalidLink)
+      }}
+    >
+      invalid link :o
+    </a>
+  ) : (
+    // this should throw an error on load since prefetch
+    // receives the invalid href
+    <Link href={invalidLink}>
+      <a id='click-me'>invalid link :o</a>
+    </Link>
+  )
+}

test/integration/invalid-href/pages/index.js

+// page used for loading and installing error catcher
+export default () => <p>Hi 👋</p>

test/integration/invalid-href/pages/second.js

+import Link from 'next/link'
+import { useRouter } from 'next/router'
+
+const invalidLink = 'https://google.com/another'
+
+export default () => {
+  const { query, ...router } = useRouter()
+  const { method } = query
+
+  return method ? (
+    <a
+      id='click-me'
+      onClick={e => {
+        e.preventDefault()
+        router[method](invalidLink)
+      }}
+    >
+      invalid link :o
+    </a>
+  ) : (
+    // this should throw an error on load since prefetch
+    // receives the invalid href
+    <Link href={invalidLink}>
+      <a id='click-me'>invalid link :o</a>
+    </Link>
+  )
+}

test/integration/invalid-href/test/index.test.js

+/* eslint-env jest */
+/* global jasmine */
+import { join } from 'path'
+import webdriver from 'next-webdriver'
+import {
+  findPort,
+  launchApp,
+  killApp,
+  nextStart,
+  nextBuild,
+  getReactErrorOverlayContent,
+  waitFor
+} from 'next-test-utils'
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5
+let app
+let appPort
+const appDir = join(__dirname, '..')
+
+const firstErrorRegex = /Invalid href passed to router: mailto:idk@idk.com.*invalid-href-passed/
+const secondErrorRegex = /Invalid href passed to router: .*google\.com.*invalid-href-passed/
+
+const showsError = async (pathname, regex, click = false) => {
+  const browser = await webdriver(appPort, pathname)
+  if (click) {
+    await browser.elementByCss('a').click()
+  }
+  const errorContent = await getReactErrorOverlayContent(browser)
+  expect(errorContent).toMatch(regex)
+  await browser.close()
+}
+
+const noError = async (pathname, click = false) => {
+  const browser = await webdriver(appPort, '/')
+  await browser.eval(`(function() {
+    window.caughtErrors = []
+    window.addEventListener('error', function (error) {
+      window.caughtErrors.push(error.message || 1)
+    })
+    window.addEventListener('unhandledrejection', function (error) {
+      window.caughtErrors.push(error.message || 1)
+    })
+    window.next.router.replace('${pathname}')
+  })()`)
+  await waitFor(250)
+  if (click) {
+    await browser.elementByCss('a').click()
+  }
+  const numErrors = await browser.eval(`window.caughtErrors.length`)
+  expect(numErrors).toBe(0)
+  await browser.close()
+}
+
+describe('Invalid hrefs', () => {
+  describe('dev mode', () => {
+    beforeAll(async () => {
+      appPort = await findPort()
+      app = await launchApp(appDir, appPort)
+    })
+    afterAll(() => killApp(app))
+
+    it('shows error when mailto: is used as href on Link', async () => {
+      await showsError('/first', firstErrorRegex)
+    })
+
+    it('shows error when mailto: is used as href on router.push', async () => {
+      await showsError('/first?method=push', firstErrorRegex, true)
+    })
+
+    it('shows error when mailto: is used as href on router.replace', async () => {
+      await showsError('/first?method=replace', firstErrorRegex, true)
+    })
+
+    it('shows error when https://google.com is used as href on Link', async () => {
+      await showsError('/second', secondErrorRegex)
+    })
+
+    it('shows error when http://google.com is used as href on router.push', async () => {
+      await showsError('/second?method=push', secondErrorRegex, true)
+    })
+
+    it('shows error when https://google.com is used as href on router.replace', async () => {
+      await showsError('/second?method=replace', secondErrorRegex, true)
+    })
+  })
+
+  describe('production mode', () => {
+    beforeAll(async () => {
+      await nextBuild(appDir)
+      appPort = await findPort()
+      app = await nextStart(appDir, appPort)
+    })
+    afterAll(() => killApp())
+
+    it('shows error when mailto: is used as href on Link', async () => {
+      await noError('/first')
+    })
+
+    it('shows error when mailto: is used as href on router.push', async () => {
+      await noError('/first?method=push', true)
+    })
+
+    it('shows error when mailto: is used as href on router.replace', async () => {
+      await noError('/first?method=replace', true)
+    })
+
+    it('shows error when https://google.com is used as href on Link', async () => {
+      await noError('/second')
+    })
+
+    it('shows error when http://google.com is used as href on router.push', async () => {
+      await noError('/second?method=push', true)
+    })
+
+    it('shows error when https://google.com is used as href on router.replace', async () => {
+      await noError('/second?method=replace', true)
+    })
+  })
+})