diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 7387630b28bfe..33f1affaadb82 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -1,21 +1,14 @@ -declare const __NEXT_DATA__: any - import React, { Children } from 'react' import { UrlObject } from 'url' -import { PrefetchOptions, NextRouter } from '../next-server/lib/router/router' -import { execOnce, getLocationOrigin } from '../next-server/lib/utils' +import { + PrefetchOptions, + NextRouter, + isLocalURL, +} from '../next-server/lib/router/router' +import { execOnce } from '../next-server/lib/utils' import { useRouter } from './router' import { addBasePath, resolveHref } from '../next-server/lib/router/router' -/** - * Detects whether a given url is from the same origin as the current page (browser only). - */ -function isLocal(url: string): boolean { - const locationOrigin = getLocationOrigin() - const resolved = new URL(url, locationOrigin) - return resolved.origin === locationOrigin -} - type Url = string | UrlObject export type LinkProps = { @@ -89,6 +82,7 @@ function prefetch( options?: PrefetchOptions ): void { if (typeof window === 'undefined') return + if (!isLocalURL(href)) return // Prefetch the JSON page if asked (only in the client) // We need to handle a prefetch error here since we may be // loading with priority which can reject but we don't @@ -103,6 +97,17 @@ function prefetch( prefetched[href + '%' + as] = true } +function isNewTabRequest(event: React.MouseEvent) { + const { target } = event.currentTarget as HTMLAnchorElement + return ( + (target && target !== '_self') || + event.metaKey || + event.ctrlKey || + event.shiftKey || + (event.nativeEvent && event.nativeEvent.which === 2) + ) +} + function linkClicked( e: React.MouseEvent, router: NextRouter, @@ -112,21 +117,10 @@ function linkClicked( shallow?: boolean, scroll?: boolean ): void { - const { nodeName, target } = e.currentTarget as HTMLAnchorElement - if ( - nodeName === 'A' && - ((target && target !== '_self') || - e.metaKey || - e.ctrlKey || - e.shiftKey || - (e.nativeEvent && e.nativeEvent.which === 2)) - ) { - // ignore click for new tab / new window behavior - return - } + const { nodeName } = e.currentTarget - if (!isLocal(href)) { - // ignore click if it's outside our scope (e.g. https://google.com) + if (nodeName === 'A' && (isNewTabRequest(e) || !isLocalURL(href))) { + // ignore click for new tab / new window behavior return } @@ -177,7 +171,13 @@ function Link(props: React.PropsWithChildren) { }, [pathname, props.href, props.as]) React.useEffect(() => { - if (p && IntersectionObserver && childElm && childElm.tagName) { + if ( + p && + IntersectionObserver && + childElm && + childElm.tagName && + isLocalURL(href) + ) { // Join on an invalid URI character const isPrefetched = prefetched[href + '%' + as] if (!isPrefetched) { @@ -224,6 +224,7 @@ function Link(props: React.PropsWithChildren) { if (p) { childProps.onMouseEnter = (e: React.MouseEvent) => { + if (!isLocalURL(href)) return if (child.props && typeof child.props.onMouseEnter === 'function') { child.props.onMouseEnter(e) } diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 719bde8a9a564..35d8e7952812f 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -11,6 +11,7 @@ import { loadGetInitialProps, NextPageContext, ST, + getLocationOrigin, } from '../utils' import { isDynamicRoute } from './utils/is-dynamic' import { getRouteMatcher } from './utils/route-matcher' @@ -47,7 +48,8 @@ export function hasBasePath(path: string): boolean { } export function addBasePath(path: string): string { - return basePath + // we only add the basepath on relative urls + return basePath && path.startsWith('/') ? path === '/' ? normalizePathTrailingSlash(basePath) : basePath + path @@ -58,6 +60,21 @@ export function delBasePath(path: string): string { return path.slice(basePath.length) || '/' } +/** + * Detects whether a given url is routable by the Next.js router (browser only). + */ +export function isLocalURL(url: string): boolean { + if (url.startsWith('/')) return true + try { + // absolute urls can be local if they are on the same origin + const locationOrigin = getLocationOrigin() + const resolved = new URL(url, locationOrigin) + return resolved.origin === locationOrigin && hasBasePath(resolved.pathname) + } catch (_) { + return false + } +} + type Url = UrlObject | string /** @@ -69,12 +86,16 @@ export function resolveHref(currentPath: string, href: Url): string { const base = new URL(currentPath, 'http://n') const urlAsString = typeof href === 'string' ? href : formatWithValidation(href) - const finalUrl = new URL(urlAsString, base) - finalUrl.pathname = normalizePathTrailingSlash(finalUrl.pathname) - // if the origin didn't change, it means we received a relative href - return finalUrl.origin === base.origin - ? finalUrl.href.slice(finalUrl.origin.length) - : finalUrl.href + try { + const finalUrl = new URL(urlAsString, base) + finalUrl.pathname = normalizePathTrailingSlash(finalUrl.pathname) + // if the origin didn't change, it means we received a relative href + return finalUrl.origin === base.origin + ? finalUrl.href.slice(finalUrl.origin.length) + : finalUrl.href + } catch (_) { + return urlAsString + } } function prepareUrlAs(router: NextRouter, url: Url, as: Url) { @@ -93,9 +114,11 @@ function tryParseRelativeUrl( return parseRelativeUrl(url) } catch (err) { if (process.env.NODE_ENV !== 'production') { - throw new Error( - `Invalid href passed to router: ${url} https://err.sh/vercel/next.js/invalid-href-passed` - ) + setTimeout(() => { + throw new Error( + `Invalid href passed to router: ${url} https://err.sh/vercel/next.js/invalid-href-passed` + ) + }, 0) } return null } @@ -434,6 +457,11 @@ export default class Router implements BaseRouter { as: string, options: TransitionOptions ): Promise { + if (!isLocalURL(url)) { + window.location.href = url + return false + } + if (!(options as any)._h) { this.isSsr = false } diff --git a/packages/next/next-server/lib/router/utils/parse-relative-url.ts b/packages/next/next-server/lib/router/utils/parse-relative-url.ts index 3b1c026a07352..6475ea6630fa5 100644 --- a/packages/next/next-server/lib/router/utils/parse-relative-url.ts +++ b/packages/next/next-server/lib/router/utils/parse-relative-url.ts @@ -1,17 +1,30 @@ -const DUMMY_BASE = new URL('http://n') +import { getLocationOrigin } from '../../utils' + +const DUMMY_BASE = new URL( + typeof window === 'undefined' ? 'http://n' : getLocationOrigin() +) /** * Parses path-relative urls (e.g. `/hello/world?foo=bar`). If url isn't path-relative * (e.g. `./hello`) then at least base must be. - * Absolute urls are rejected. + * Absolute urls are rejected with one exception, in the browser, absolute urls that are on + * the current origin will be parsed as relative */ export function parseRelativeUrl(url: string, base?: string) { const resolvedBase = base ? new URL(base, DUMMY_BASE) : DUMMY_BASE - const { pathname, searchParams, search, hash, href, origin } = new URL( - url, - resolvedBase - ) - if (origin !== DUMMY_BASE.origin) { + const { + pathname, + searchParams, + search, + hash, + href, + origin, + protocol, + } = new URL(url, resolvedBase) + if ( + origin !== DUMMY_BASE.origin || + (protocol !== 'http:' && protocol !== 'https:') + ) { throw new Error('invariant: invalid relative URL') } return { diff --git a/test/integration/basepath/pages/absolute-url-basepath.js b/test/integration/basepath/pages/absolute-url-basepath.js new file mode 100644 index 0000000000000..330865a6b5bc6 --- /dev/null +++ b/test/integration/basepath/pages/absolute-url-basepath.js @@ -0,0 +1,19 @@ +import React from 'react' +import Link from 'next/link' + +export async function getServerSideProps({ query: { port } }) { + if (!port) { + throw new Error('port required') + } + return { props: { port } } +} + +export default function Page({ port }) { + return ( + <> + + http://localhost:{port}/docs/something-else + + + ) +} diff --git a/test/integration/basepath/pages/absolute-url-no-basepath.js b/test/integration/basepath/pages/absolute-url-no-basepath.js new file mode 100644 index 0000000000000..fb91bedaab51b --- /dev/null +++ b/test/integration/basepath/pages/absolute-url-no-basepath.js @@ -0,0 +1,19 @@ +import React from 'react' +import Link from 'next/link' + +export async function getServerSideProps({ query: { port } }) { + if (!port) { + throw new Error('port required') + } + return { props: { port } } +} + +export default function Page({ port }) { + return ( + <> + + http://localhost:{port}/rewrite-no-basepath + + + ) +} diff --git a/test/integration/basepath/pages/absolute-url.js b/test/integration/basepath/pages/absolute-url.js new file mode 100644 index 0000000000000..9d3c45efa7d30 --- /dev/null +++ b/test/integration/basepath/pages/absolute-url.js @@ -0,0 +1,16 @@ +import React from 'react' +import Link from 'next/link' + +export default function Page() { + return ( + <> + + https://vercel.com/ + +
+ + mailto:idk@idk.com + + + ) +} diff --git a/test/integration/basepath/test/index.test.js b/test/integration/basepath/test/index.test.js index f3d48a960f78f..c37233146eee3 100644 --- a/test/integration/basepath/test/index.test.js +++ b/test/integration/basepath/test/index.test.js @@ -390,6 +390,45 @@ const runTests = (context, dev = false) => { expect(pathname).toBe('/docs') }) + it('should navigate an absolute url', async () => { + const browser = await webdriver(context.appPort, `/docs/absolute-url`) + await browser.waitForElementByCss('#absolute-link').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) + + it('should navigate an absolute local url with basePath', async () => { + const browser = await webdriver( + context.appPort, + `/docs/absolute-url-basepath?port=${context.appPort}` + ) + await browser.eval(() => (window._didNotNavigate = true)) + await browser.waitForElementByCss('#absolute-link').click() + const text = await browser + .waitForElementByCss('#something-else-page') + .text() + + expect(text).toBe('something else') + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) + + it('should navigate an absolute local url without basePath', async () => { + const browser = await webdriver( + context.appPort, + `/docs/absolute-url-no-basepath?port=${context.appPort}` + ) + await browser.waitForElementByCss('#absolute-link').click() + await check( + () => browser.eval(() => location.pathname), + '/rewrite-no-basepath' + ) + const text = await browser.elementByCss('body').text() + + expect(text).toBe('hello from external') + }) + it('should 404 when manually adding basePath with ', async () => { const browser = await webdriver( context.appPort, diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 2b4f1738042b6..01adfb3cb6703 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -104,7 +104,7 @@ describe('Build Output', () => { expect(parseFloat(err404FirstLoad) - 63).toBeLessThanOrEqual(0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 59.3).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 59.4).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { diff --git a/test/integration/client-navigation/pages/absolute-url.js b/test/integration/client-navigation/pages/absolute-url.js new file mode 100644 index 0000000000000..0c2f32cab3ef3 --- /dev/null +++ b/test/integration/client-navigation/pages/absolute-url.js @@ -0,0 +1,66 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +export async function getServerSideProps({ query: { port } }) { + if (!port) { + throw new Error('port required') + } + return { props: { port } } +} + +export default function Page({ port }) { + const router = useRouter() + return ( + <> + + https://vercel.com/ + +
+ +
+ +
+ + http://localhost:{port}/nav/about + +
+ + + http://localhost:{port}/dynamic/hello/route + + +
+ +
+ +
+ + mailto:idk@idk.com + + + ) +} diff --git a/test/integration/client-navigation/pages/dynamic/[slug]/route.js b/test/integration/client-navigation/pages/dynamic/[slug]/route.js index 0f1da367eac78..668eb33fe0fa7 100644 --- a/test/integration/client-navigation/pages/dynamic/[slug]/route.js +++ b/test/integration/client-navigation/pages/dynamic/[slug]/route.js @@ -8,6 +8,6 @@ export default class DynamicRoute extends React.Component { } render() { - return

{this.props.query.slug}

+ return

{this.props.query.slug}

} } diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js index e8d59546df887..2131b29a02477 100644 --- a/test/integration/client-navigation/test/index.test.js +++ b/test/integration/client-navigation/test/index.test.js @@ -10,6 +10,7 @@ import { launchApp, renderViaHTTP, waitFor, + check, } from 'next-test-utils' import webdriver from 'next-webdriver' import { join } from 'path' @@ -124,6 +125,47 @@ describe('Client Navigation', () => { expect(counterText).toBe('Counter: 1') await browser.close() }) + + it('should navigate an absolute url', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.waitForElementByCss('#absolute-link').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) + + it('should navigate an absolute local url', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.eval(() => (window._didNotNavigate = true)) + await browser.waitForElementByCss('#absolute-local-link').click() + const text = await browser + .waitForElementByCss('.nav-about') + .elementByCss('p') + .text() + + expect(text).toBe('This is the about page.') + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) + + it('should navigate an absolute local url with as', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.eval(() => (window._didNotNavigate = true)) + await browser.waitForElementByCss('#absolute-local-dynamic-link').click() + expect(await browser.waitForElementByCss('#dynamic-page').text()).toBe( + 'hello' + ) + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) }) describe('With url property', () => { @@ -872,6 +914,60 @@ describe('Client Navigation', () => { expect(asPath).toBe('/nav/as-path-using-router') await browser.close() }) + + it('should navigate an absolute url on push', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.waitForElementByCss('#router-push').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) + + it('should navigate an absolute url on replace', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.waitForElementByCss('#router-replace').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) + + it('should navigate an absolute local url on push', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.eval(() => (window._didNotNavigate = true)) + await browser.waitForElementByCss('#router-local-push').click() + const text = await browser + .waitForElementByCss('.nav-about') + .elementByCss('p') + .text() + expect(text).toBe('This is the about page.') + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) + + it('should navigate an absolute local url on replace', async () => { + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) + await browser.eval(() => (window._didNotNavigate = true)) + await browser.waitForElementByCss('#router-local-replace').click() + const text = await browser + .waitForElementByCss('.nav-about') + .elementByCss('p') + .text() + expect(text).toBe('This is the about page.') + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) }) describe('with next/link', () => { diff --git a/test/integration/invalid-href/pages/second.js b/test/integration/invalid-href/pages/second.js index a1a411bac6e2e..09af70d09ddf7 100644 --- a/test/integration/invalid-href/pages/second.js +++ b/test/integration/invalid-href/pages/second.js @@ -1,7 +1,7 @@ import Link from 'next/link' import { useRouter } from 'next/router' -const invalidLink = 'https://google.com/another' +const invalidLink = 'https://vercel.com/solutions/nextjs' export default function Page() { const { query, ...router } = useRouter() diff --git a/test/integration/invalid-href/pages/third.js b/test/integration/invalid-href/pages/third.js index 509367eab393d..c55c4f85d0c91 100644 --- a/test/integration/invalid-href/pages/third.js +++ b/test/integration/invalid-href/pages/third.js @@ -1,26 +1,10 @@ -import { useRouter } from 'next/router' -import { useState } from 'react' - -const invalidLink = 'https://vercel.com/' +import Link from 'next/link' +const invalidLink = 'https://' export default function Page() { - const { query, ...router } = useRouter() - const [isDone, setIsDone] = useState(false) - const { method = 'push' } = query - - function click(e) { - router[method](invalidLink).then( - () => setIsDone(true), - () => setIsDone(true) - ) - } - return ( - <> - - {isDone ?
Done
: null} - + + invalid link :o + ) } diff --git a/test/integration/invalid-href/test/index.test.js b/test/integration/invalid-href/test/index.test.js index 37825ba3dbe82..3fdc794bdfd9e 100644 --- a/test/integration/invalid-href/test/index.test.js +++ b/test/integration/invalid-href/test/index.test.js @@ -10,7 +10,9 @@ import { nextStart, waitFor, check, + fetchViaHTTP, } from 'next-test-utils' +import cheerio from 'cheerio' import webdriver from 'next-webdriver' import { join } from 'path' @@ -20,9 +22,6 @@ 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/ - // This test doesn't seem to benefit from retries, let's disable them until the test gets fixed // to prevent long running times jest.retryTimes(0) @@ -99,26 +98,10 @@ describe('Invalid hrefs', () => { await noError('/first') }) - it('does not show error in production when mailto: is used as href on router.push', async () => { - await noError('/first?method=push', true) - }) - - it('does not show error in production when mailto: is used as href on router.replace', async () => { - await noError('/first?method=replace', true) - }) - it('does not show error in production when https://google.com is used as href on Link', async () => { await noError('/second') }) - it('does not show error in production when http://google.com is used as href on router.push', async () => { - await noError('/second?method=push', true) - }) - - it('does not show error in production when https://google.com is used as href on router.replace', async () => { - await noError('/second?method=replace', true) - }) - it('shows error when dynamic route mismatch is used on Link', async () => { const browser = await webdriver(appPort, '/dynamic-route-mismatch') try { @@ -143,16 +126,14 @@ describe('Invalid hrefs', () => { } }) - it('makes sure that router push with bad links resolve', async () => { - const browser = await webdriver(appPort, '/third') - await browser.elementByCss('#click-me').click() - await browser.waitForElementByCss('#is-done') + it("doesn't fail on invalid url", async () => { + await noError('/third') }) - it('makes sure that router replace with bad links resolve', async () => { - const browser = await webdriver(appPort, '/third?method=replace') - await browser.elementByCss('#click-me').click() - await browser.waitForElementByCss('#is-done') + it('renders a link with invalid href', async () => { + const res = await fetchViaHTTP(appPort, '/third') + const $ = cheerio.load(await res.text()) + expect($('#click-me').attr('href')).toBe('https://') }) }) @@ -163,28 +144,12 @@ describe('Invalid hrefs', () => { }) 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('does not show error when mailto: is used as href on Link', async () => { + await noError('/first') }) - it('shows error when https://google.com is used as href on router.replace', async () => { - await showsError('/second?method=replace', secondErrorRegex, true) + it('does not show error when https://google.com is used as href on Link', async () => { + await noError('/second') }) it('shows error when dynamic route mismatch is used on Link', async () => { @@ -199,6 +164,10 @@ describe('Invalid hrefs', () => { await noError('/dynamic-route-mismatch-manual', true) }) + it("doesn't fail on invalid url", async () => { + await noError('/third') + }) + it('shows warning when dynamic route mismatch is used on Link', async () => { await showsError( '/dynamic-route-mismatch', diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 08f7bb4591540..36628b7511dcc 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -100,7 +100,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 166 * 1024 + const delta = responseSizesBytes - 167 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target })