Skip to content

Commit

Permalink
Fix router not working on some protocol (vercel#16650)
Browse files Browse the repository at this point in the history
Co-authored-by: Tim Neutkens <[email protected]>
Co-authored-by: Joe Haddad <[email protected]>
Co-authored-by: JJ Kasper <[email protected]>
  • Loading branch information
4 people authored Nov 3, 2020
1 parent 3c33794 commit 28e1287
Show file tree
Hide file tree
Showing 11 changed files with 688 additions and 42 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_test_deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ jobs:
# TODO: remove after we fix watchpack watching too much
- run: echo fs.inotify.max_user_watches=524288 | sudo tee -a /etc/sysctl.conf && sudo sysctl -p

- run: node run-tests.js --timings -g ${{ matrix.group }}/6 -c 3
- run: xvfb-run node run-tests.js --timings -g ${{ matrix.group }}/6 -c 3

testYarnPnP:
runs-on: ubuntu-latest
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"coveralls": "3.0.3",
"cross-env": "6.0.3",
"cross-spawn": "6.0.5",
"electron": "^5.0.0",
"escape-string-regexp": "2.0.0",
"eslint": "6.8.0",
"eslint-plugin-import": "2.20.2",
Expand Down Expand Up @@ -118,6 +119,7 @@
"selenium-standalone": "6.18.0",
"selenium-webdriver": "4.0.0-alpha.7",
"shell-quote": "1.7.2",
"spectron": "^7.0.0",
"styled-components": "5.1.0",
"styled-jsx-plugin-postcss": "2.0.1",
"tailwindcss": "1.1.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/next/compiled/conf/index.js

Large diffs are not rendered by default.

29 changes: 10 additions & 19 deletions packages/next/next-server/lib/router/utils/parse-relative-url.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,29 @@
import { getLocationOrigin } from '../../utils'
import { searchParamsToUrlQuery } from './querystring'

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 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,
protocol,
} = new URL(url, resolvedBase)
if (
origin !== DUMMY_BASE.origin ||
(protocol !== 'http:' && protocol !== 'https:')
) {
const globalBase = new URL(
typeof window === 'undefined' ? 'http://n' : getLocationOrigin()
)
const resolvedBase = base ? new URL(base, globalBase) : globalBase
const { pathname, searchParams, search, hash, href, origin } = new URL(
url,
resolvedBase
)
if (origin !== globalBase.origin) {
throw new Error('invariant: invalid relative URL')
}
return {
pathname,
query: searchParamsToUrlQuery(searchParams),
search,
hash,
href: href.slice(DUMMY_BASE.origin.length),
href: href.slice(globalBase.origin.length),
}
}
6 changes: 6 additions & 0 deletions test/integration/with-electron/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const path = require('path')
const outdir = path.join(__dirname, 'out')

module.exports = {
basePath: outdir,
}
10 changes: 10 additions & 0 deletions test/integration/with-electron/pages/about.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Link from 'next/link'

export default () => (
<div id="about-page">
<p>This is the about page</p>
<Link href="/">
<a>Go Back</a>
</Link>
</div>
)
19 changes: 19 additions & 0 deletions test/integration/with-electron/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Link from 'next/link'
import Router from 'next/router'

function routeToAbout(e) {
e.preventDefault()
Router.push('/about')
}

export default () => (
<div id="home-page">
<p>This is the home page</p>
<Link href="/about">
<a id="about-via-link">about via Link</a>
</Link>
<a id="about-via-router" onClick={routeToAbout} href="#">
about via Router
</a>
</div>
)
31 changes: 31 additions & 0 deletions test/integration/with-electron/public/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
const { join } = require('path')
const { app, BrowserWindow } = require('electron')
const url = require('url')

function createWindow() {
const mainWindow = new BrowserWindow({
width: 800,
height: 600,
webPreferences: {
nodeIntegration: false,
},
})
mainWindow.loadURL(
url.format({
pathname: join(__dirname, 'index.html'),
protocol: 'file:',
slashes: true,
})
)
}

app.whenReady().then(() => {
createWindow()
app.on('activate', () => {
if (BrowserWindow.getAllWindows().length === 0) createWindow()
})
})

app.on('window-all-closed', () => {
app.quit()
})
75 changes: 75 additions & 0 deletions test/integration/with-electron/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/* eslint-env jest */

import { join } from 'path'
import { nextBuild, nextExport } from 'next-test-utils'
import { Application } from 'spectron'
import electron from 'electron'

jest.setTimeout(1000 * 60 * 5)

const nextdir = join(__dirname, '../')
const outdir = join(nextdir, 'out')
const appdir = join(outdir, 'main.js')
let app = null

describe('Parse Relative Url', () => {
describe('File Protocol via Electron', () => {
beforeAll(async () => {
await nextBuild(nextdir)
await nextExport(nextdir, { outdir })

app = new Application({
path: electron,
args: [
'--no-sandbox',
'--disable-dev-shm-usage',
'--disable-gpu',
appdir,
],
chromeDriverArgs: [
'--no-sandbox',
'--disable-dev-shm-usage',
'--disable-gpu',
],
})
await app.start()
})

afterAll(async () => {
if (app && app.isRunning()) {
await app.stop()
}
})

it('app init', async () => {
const count = await app.client.getWindowCount()
expect(count).toEqual(1)
})

it('should render the home page', async () => {
const text = await app.client.$('#home-page p').getText()
expect(text).toBe('This is the home page')
})

it('should do navigations via Link', async () => {
await app.client.$('#about-via-link').click()
const text = await app.client.$('#about-page p').getText()

expect(text).toBe('This is the about page')
})

it('should do back to home page via Link', async () => {
await app.client.$('#about-page a').click()
const text = await app.client.$('#home-page p').getText()

expect(text).toBe('This is the home page')
})

it('should do navigations via Router', async () => {
await app.client.$('#about-via-router').click()
const text = await app.client.$('#about-page p').getText()

expect(text).toBe('This is the about page')
})
})
})
111 changes: 111 additions & 0 deletions test/unit/parse-relative-url.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/* eslint-env jest */
import { parseRelativeUrl } from 'next/dist/next-server/lib/router/utils/parse-relative-url'

// convenience function so tests can be aligned neatly
// and easy to eyeball
const check = (windowUrl, targetUrl, expected) => {
window.location = new URL(windowUrl)
if (typeof expected === 'string') {
expect(() => parseRelativeUrl(targetUrl)).toThrow(expected)
} else {
const parsedUrl = parseRelativeUrl(targetUrl)
expect(parsedUrl.pathname).toBe(expected.pathname)
expect(parsedUrl.search).toBe(expected.search)
expect(parsedUrl.hash).toBe(expected.hash)
}
}

describe('parseRelativeUrl', () => {
beforeAll(() => {
global.window = {
location: {},
}
})

afterAll(() => {
delete global.window
})

it('should parse relative url', () => {
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'/someF/pathG?fooH=barI#hashJ',
{
pathname: '/someF/pathG',
search: '?fooH=barI',
hash: '#hashJ',
}
)
})

it('should parse relative url when start with dot', () => {
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'./someF/pathG?fooH=barI#hashJ',
{
pathname: '/someF/pathG',
search: '?fooH=barI',
hash: '#hashJ',
}
)
})

it('should parse relative url on special protocol', () => {
check(
'ionic://localhost/someA/pathB?fooC=barD#hashE',
'/someF/pathG?fooH=barI#hashJ',
{
pathname: '/someF/pathG',
search: '?fooH=barI',
hash: '#hashJ',
}
)
check(
'file:///someA/pathB?fooC=barD#hashE',
'/someF/pathG?fooH=barI#hashJ',
{
pathname: '/someF/pathG',
search: '?fooH=barI',
hash: '#hashJ',
}
)
})

it('should parse the full url with current origin', () => {
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'http://example.com:3210/someF/pathG?fooH=barI#hashJ',
{
pathname: '/someF/pathG',
search: '?fooH=barI',
hash: '#hashJ',
}
)
})

it('should throw when parsing the full url with diff origin', () => {
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'http://google.com/someF/pathG?fooH=barI#hashJ',
'invariant: invalid relative URL'
)
})

it('should throw when parsing the special prefix', () => {
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'mailto:[email protected]',
'invariant: invalid relative URL'
)
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'tel:+123456789',
'invariant: invalid relative URL'
)
check(
'http://example.com:3210/someA/pathB?fooC=barD#hashE',
'sms:+123456789',
'invariant: invalid relative URL'
)
})
})
Loading

0 comments on commit 28e1287

Please sign in to comment.