From 29be91ef9caf7b572bc21dc9b5f7f598e67ac77b Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Sat, 1 Aug 2020 20:59:48 +0200 Subject: [PATCH 1/9] initial impl --- packages/next/client/link.tsx | 26 ++++----- .../next/next-server/lib/router/router.ts | 14 ++++- packages/next/next-server/lib/utils.ts | 9 +++ .../build-output/test/index.test.js | 2 +- .../client-navigation/pages/absolute-url.js | 26 +++++++++ .../client-navigation/test/index.test.js | 28 ++++++++++ test/integration/invalid-href/pages/second.js | 2 +- test/integration/invalid-href/pages/third.js | 26 --------- .../invalid-href/test/index.test.js | 55 ++----------------- 9 files changed, 90 insertions(+), 98 deletions(-) create mode 100644 test/integration/client-navigation/pages/absolute-url.js delete mode 100644 test/integration/invalid-href/pages/third.js diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 84ad71186e518..9ed9d10331aec 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -3,19 +3,10 @@ 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 { execOnce, isLocalURL } 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 +80,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 @@ -125,11 +117,6 @@ function linkClicked( return } - if (!isLocal(href)) { - // ignore click if it's outside our scope (e.g. https://google.com) - return - } - e.preventDefault() // avoid scroll for urls with anchor refs @@ -177,7 +164,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 +217,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 125ee15fdf5b5..43dd3be425294 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, + isLocalURL, } from '../utils' import { isDynamicRoute } from './utils/is-dynamic' import { getRouteMatcher } from './utils/route-matcher' @@ -93,9 +94,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 } @@ -445,6 +448,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/utils.ts b/packages/next/next-server/lib/utils.ts index 5b74fc017a65c..9adb69e80a51f 100644 --- a/packages/next/next-server/lib/utils.ts +++ b/packages/next/next-server/lib/utils.ts @@ -284,6 +284,15 @@ export function getURL() { return href.substring(origin.length) } +/** + * Detects whether a given url is from the same origin as the current page (browser only). + */ +export function isLocalURL(url: string): boolean { + const locationOrigin = getLocationOrigin() + const resolved = new URL(url, locationOrigin) + return resolved.origin === locationOrigin +} + export function getDisplayName

(Component: ComponentType

) { return typeof Component === 'string' ? Component diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 2b4f1738042b6..4b7acbab20276 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.34).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..e6ad238c3d156 --- /dev/null +++ b/test/integration/client-navigation/pages/absolute-url.js @@ -0,0 +1,26 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +export default function Page() { + const router = useRouter() + return ( + <> + + Go + + + + + ) +} diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js index e8d59546df887..2609793a57f15 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,15 @@ 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') + await browser.waitForElementByCss('#absolute-link').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) }) describe('With url property', () => { @@ -872,6 +882,24 @@ 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') + 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') + await browser.waitForElementByCss('#router-replace').click() + await check( + () => browser.eval(() => window.location.origin), + 'https://vercel.com' + ) + }) }) 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 deleted file mode 100644 index 509367eab393d..0000000000000 --- a/test/integration/invalid-href/pages/third.js +++ /dev/null @@ -1,26 +0,0 @@ -import { useRouter } from 'next/router' -import { useState } from 'react' - -const invalidLink = 'https://vercel.com/' - -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} - - ) -} diff --git a/test/integration/invalid-href/test/index.test.js b/test/integration/invalid-href/test/index.test.js index 37825ba3dbe82..d5050fe078e27 100644 --- a/test/integration/invalid-href/test/index.test.js +++ b/test/integration/invalid-href/test/index.test.js @@ -20,9 +20,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 +96,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 { @@ -142,18 +123,6 @@ describe('Invalid hrefs', () => { await browser.close() } }) - - 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('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') - }) }) describe('dev mode', () => { @@ -163,28 +132,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 () => { From 7ee521854300007ce3bf826e799f4c22d2664f82 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Mon, 3 Aug 2020 17:58:28 +0200 Subject: [PATCH 2/9] update build-output --- test/integration/build-output/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 4b7acbab20276..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.34).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 59.4).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { From bdec3277df40174ee6719a690911840768c094c4 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 4 Aug 2020 14:33:29 +0200 Subject: [PATCH 3/9] add local url tests --- .../lib/router/utils/parse-relative-url.ts | 27 ++++++--- .../client-navigation/pages/absolute-url.js | 25 +++++++- .../client-navigation/test/index.test.js | 57 ++++++++++++++++++- 3 files changed, 98 insertions(+), 11 deletions(-) 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/client-navigation/pages/absolute-url.js b/test/integration/client-navigation/pages/absolute-url.js index e6ad238c3d156..90f7e6e04d4b4 100644 --- a/test/integration/client-navigation/pages/absolute-url.js +++ b/test/integration/client-navigation/pages/absolute-url.js @@ -2,7 +2,14 @@ import React from 'react' import Link from 'next/link' import { useRouter } from 'next/router' -export default function Page() { +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 ( <> @@ -21,6 +28,22 @@ export default function Page() { > Go replace + + + Go + + + ) } diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js index 2609793a57f15..626adec82c656 100644 --- a/test/integration/client-navigation/test/index.test.js +++ b/test/integration/client-navigation/test/index.test.js @@ -127,13 +127,30 @@ describe('Client Navigation', () => { }) it('should navigate an absolute url', async () => { - const browser = await webdriver(context.appPort, '/absolute-url') + 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() + await check( + () => browser.eval(() => window.location.pathname), + '/nav/about' + ) + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) }) describe('With url property', () => { @@ -884,7 +901,10 @@ describe('Client Navigation', () => { }) it('should navigate an absolute url on push', async () => { - const browser = await webdriver(context.appPort, '/absolute-url') + const browser = await webdriver( + context.appPort, + `/absolute-url?port=${context.appPort}` + ) await browser.waitForElementByCss('#router-push').click() await check( () => browser.eval(() => window.location.origin), @@ -893,13 +913,44 @@ describe('Client Navigation', () => { }) it('should navigate an absolute url on replace', async () => { - const browser = await webdriver(context.appPort, '/absolute-url') + 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() + await check( + () => browser.eval(() => window.location.pathname), + '/nav/about' + ) + 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() + await check( + () => browser.eval(() => window.location.pathname), + '/nav/about' + ) + expect(await browser.eval(() => window._didNotNavigate)).toBe(true) + }) }) describe('with next/link', () => { From ef22ef4391738701af12c5491dd334d45c56c2ea Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 4 Aug 2020 16:20:59 +0200 Subject: [PATCH 4/9] more tests --- .../client-navigation/pages/absolute-url.js | 6 +++ .../pages/dynamic/[slug]/route.js | 2 +- .../client-navigation/test/index.test.js | 39 +++++++++++++------ 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/test/integration/client-navigation/pages/absolute-url.js b/test/integration/client-navigation/pages/absolute-url.js index 90f7e6e04d4b4..112585a1310a9 100644 --- a/test/integration/client-navigation/pages/absolute-url.js +++ b/test/integration/client-navigation/pages/absolute-url.js @@ -32,6 +32,12 @@ export default function Page({ port }) { Go + + Go + +
- +
- Go + http://localhost:{port}/nav/about +
- Go + + http://localhost:{port}/dynamic/hello/route + +
+
+
+ + mailto:idk@idk.com + ) } From ee821b41cc6c2a7872dc8d942a07ec45e12bfaa9 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 4 Aug 2020 20:51:36 +0200 Subject: [PATCH 6/9] native behavior for non-local url --- packages/next/client/link.tsx | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 078bdf4a9da20..0cdea47a66548 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -95,6 +95,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, @@ -104,15 +115,9 @@ 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)) - ) { + const { nodeName } = e.currentTarget + + if (nodeName === 'A' && (isNewTabRequest(e) || !isLocalURL(href))) { // ignore click for new tab / new window behavior return } From aa935526b89ec9b54b7e0de66aeea2a23e899ce8 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 4 Aug 2020 22:51:45 +0200 Subject: [PATCH 7/9] invalid urls --- packages/next/next-server/lib/router/router.ts | 16 ++++++++++------ packages/next/next-server/lib/utils.ts | 8 ++++++-- test/integration/invalid-href/pages/third.js | 10 ++++++++++ test/integration/invalid-href/test/index.test.js | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 test/integration/invalid-href/pages/third.js diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index ee804f5574392..f96e2827c46b0 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -70,12 +70,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) { diff --git a/packages/next/next-server/lib/utils.ts b/packages/next/next-server/lib/utils.ts index 890aede5a364a..f572732f27f34 100644 --- a/packages/next/next-server/lib/utils.ts +++ b/packages/next/next-server/lib/utils.ts @@ -290,8 +290,12 @@ export function getURL() { */ export function isLocalURL(url: string): boolean { const locationOrigin = getLocationOrigin() - const resolved = new URL(url, locationOrigin) - return resolved.origin === locationOrigin + try { + const resolved = new URL(url, locationOrigin) + return resolved.origin === locationOrigin + } catch (_) { + return false + } } export function getDisplayName

(Component: ComponentType

) { diff --git a/test/integration/invalid-href/pages/third.js b/test/integration/invalid-href/pages/third.js new file mode 100644 index 0000000000000..c55c4f85d0c91 --- /dev/null +++ b/test/integration/invalid-href/pages/third.js @@ -0,0 +1,10 @@ +import Link from 'next/link' +const invalidLink = 'https://' + +export default function Page() { + return ( + + invalid link :o + + ) +} diff --git a/test/integration/invalid-href/test/index.test.js b/test/integration/invalid-href/test/index.test.js index d5050fe078e27..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' @@ -123,6 +125,16 @@ describe('Invalid hrefs', () => { await browser.close() } }) + + it("doesn't fail on invalid url", async () => { + await noError('/third') + }) + + 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://') + }) }) describe('dev mode', () => { @@ -152,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', From 8af53910bb61bd30aa4206832862817d9a61be97 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Tue, 4 Aug 2020 23:25:13 +0200 Subject: [PATCH 8/9] udate size imit --- test/integration/size-limit/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 }) From 04a3d48d42cc884377115c5a892fd8667423bd06 Mon Sep 17 00:00:00 2001 From: Jan Potoms <2109932+Janpot@users.noreply.github.com> Date: Wed, 5 Aug 2020 18:52:30 +0200 Subject: [PATCH 9/9] fix basePath --- packages/next/client/link.tsx | 10 +++-- .../next/next-server/lib/router/router.ts | 20 +++++++++- packages/next/next-server/lib/utils.ts | 13 ------- .../basepath/pages/absolute-url-basepath.js | 19 +++++++++ .../pages/absolute-url-no-basepath.js | 19 +++++++++ .../basepath/pages/absolute-url.js | 16 ++++++++ test/integration/basepath/test/index.test.js | 39 +++++++++++++++++++ 7 files changed, 117 insertions(+), 19 deletions(-) create mode 100644 test/integration/basepath/pages/absolute-url-basepath.js create mode 100644 test/integration/basepath/pages/absolute-url-no-basepath.js create mode 100644 test/integration/basepath/pages/absolute-url.js diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 0cdea47a66548..33f1affaadb82 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -1,9 +1,11 @@ -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, isLocalURL } 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' diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index f96e2827c46b0..35d8e7952812f 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -11,7 +11,7 @@ import { loadGetInitialProps, NextPageContext, ST, - isLocalURL, + getLocationOrigin, } from '../utils' import { isDynamicRoute } from './utils/is-dynamic' import { getRouteMatcher } from './utils/route-matcher' @@ -48,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 @@ -59,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 /** diff --git a/packages/next/next-server/lib/utils.ts b/packages/next/next-server/lib/utils.ts index f572732f27f34..37e7d58a0833f 100644 --- a/packages/next/next-server/lib/utils.ts +++ b/packages/next/next-server/lib/utils.ts @@ -285,19 +285,6 @@ export function getURL() { return href.substring(origin.length) } -/** - * Detects whether a given url is from the same origin as the current page (browser only). - */ -export function isLocalURL(url: string): boolean { - const locationOrigin = getLocationOrigin() - try { - const resolved = new URL(url, locationOrigin) - return resolved.origin === locationOrigin - } catch (_) { - return false - } -} - export function getDisplayName

(Component: ComponentType

) { return typeof Component === 'string' ? Component 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,