From a7337a9b3890c5ede0a15715f0a7f20d9c832068 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 14 Feb 2020 09:53:08 -0600 Subject: [PATCH 1/2] Make sure to encode pathname for custom-route destination (#10536) * Make sure to encode pathname for custom-route destination * Add tests to security test suite --- packages/next/next-server/server/router.ts | 3 +- test/integration/custom-routes/next.config.js | 5 +++ .../custom-routes/test/index.test.js | 24 ++++++++++ test/integration/production/next.config.js | 11 +++++ test/integration/production/test/security.js | 44 ++++++++++++++++++- 5 files changed, 85 insertions(+), 2 deletions(-) diff --git a/packages/next/next-server/server/router.ts b/packages/next/next-server/server/router.ts index 7d9d6b17d2e91..73eced0696bdd 100644 --- a/packages/next/next-server/server/router.ts +++ b/packages/next/next-server/server/router.ts @@ -54,7 +54,8 @@ export const prepareDestination = (destination: string, params: Params) => { }) try { - newUrl = destinationCompiler(params) + newUrl = encodeURI(destinationCompiler(params)) + const [pathname, hash] = newUrl.split('#') parsedDestination.pathname = pathname parsedDestination.hash = `${hash ? '#' : ''}${hash || ''}` diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 43079ebd7ae78..27155c4704234 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -83,6 +83,11 @@ module.exports = { }, async redirects() { return [ + { + source: '/redirect/me/to-about/:lang', + destination: '/:lang/about', + permanent: false, + }, { source: '/docs/router-status/:code', destination: '/docs/v2/network/status-codes#:code', diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 24b7f68044e6a..f97fa593815b7 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -311,6 +311,22 @@ const runTests = (isDev = false) => { expect(JSON.parse(data)).toEqual({ query: { name: 'hello' } }) }) + it('should handle encoded value in the pathname correctly', async () => { + const res = await fetchViaHTTP( + appPort, + '/redirect/me/to-about/' + encodeURI('\\google.com'), + undefined, + { + redirect: 'manual', + } + ) + + const { pathname, hostname } = url.parse(res.headers.get('location') || '') + expect(res.status).toBe(307) + expect(pathname).toBe(encodeURI('/\\google.com/about')) + expect(hostname).not.toBe('google.com') + }) + if (!isDev) { it('should output routes-manifest successfully', async () => { const manifest = await fs.readJSON( @@ -331,6 +347,14 @@ const runTests = (isDev = false) => { pages404: false, basePath: '', redirects: [ + { + destination: '/:lang/about', + regex: normalizeRegEx( + '^\\/redirect\\/me\\/to-about(?:\\/([^\\/]+?))$' + ), + source: '/redirect/me/to-about/:lang', + statusCode: 307, + }, { source: '/docs/router-status/:code', destination: '/docs/v2/network/status-codes#:code', diff --git a/test/integration/production/next.config.js b/test/integration/production/next.config.js index cc17cf48c578f..0420a28c1c672 100644 --- a/test/integration/production/next.config.js +++ b/test/integration/production/next.config.js @@ -3,4 +3,15 @@ module.exports = { // Make sure entries are not getting disposed. maxInactiveAge: 1000 * 60 * 60, }, + experimental: { + redirects() { + return [ + { + source: '/redirect/me/to-about/:lang', + destination: '/:lang/about', + permanent: false, + }, + ] + }, + }, } diff --git a/test/integration/production/test/security.js b/test/integration/production/test/security.js index 93eac4bda2095..59ad0d15b5db2 100644 --- a/test/integration/production/test/security.js +++ b/test/integration/production/test/security.js @@ -1,8 +1,14 @@ /* eslint-env jest */ import webdriver from 'next-webdriver' import { readFileSync } from 'fs' +import url from 'url' import { join, resolve as resolvePath } from 'path' -import { renderViaHTTP, getBrowserBodyText, waitFor } from 'next-test-utils' +import { + renderViaHTTP, + getBrowserBodyText, + waitFor, + fetchViaHTTP, +} from 'next-test-utils' import { recursiveReadDir } from 'next/dist/lib/recursive-readdir' import { homedir } from 'os' @@ -152,5 +158,41 @@ module.exports = context => { await checkInjected(browser) await browser.close() }) + + it('should handle encoded value in the pathname correctly \\', async () => { + const res = await fetchViaHTTP( + context.appPort, + '/redirect/me/to-about/' + encodeURI('\\google.com'), + undefined, + { + redirect: 'manual', + } + ) + + const { pathname, hostname } = url.parse( + res.headers.get('location') || '' + ) + expect(res.status).toBe(307) + expect(pathname).toBe(encodeURI('/\\google.com/about')) + expect(hostname).not.toBe('google.com') + }) + + it('should handle encoded value in the pathname correctly %', async () => { + const res = await fetchViaHTTP( + context.appPort, + '/redirect/me/to-about/%25google.com', + undefined, + { + redirect: 'manual', + } + ) + + const { pathname, hostname } = url.parse( + res.headers.get('location') || '' + ) + expect(res.status).toBe(307) + expect(pathname).toBe('/%25google.com/about') + expect(hostname).not.toBe('google.com') + }) }) } From ecfa7f7d16ec01f413e9e7cb8f215961bd178d12 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Fri, 14 Feb 2020 10:06:05 -0600 Subject: [PATCH 2/2] v9.2.2-canary.20 --- lerna.json | 2 +- packages/create-next-app/package.json | 2 +- packages/next-bundle-analyzer/package.json | 2 +- packages/next-mdx/package.json | 2 +- packages/next-plugin-google-analytics/package.json | 2 +- packages/next-plugin-material-ui/package.json | 2 +- packages/next-plugin-sentry/package.json | 2 +- packages/next-polyfill-nomodule/package.json | 2 +- packages/next/package.json | 4 ++-- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lerna.json b/lerna.json index 9030d7853c94e..948802af90977 100644 --- a/lerna.json +++ b/lerna.json @@ -12,5 +12,5 @@ "registry": "https://registry.npmjs.org/" } }, - "version": "9.2.2-canary.19" + "version": "9.2.2-canary.20" } diff --git a/packages/create-next-app/package.json b/packages/create-next-app/package.json index 8d93fd49cc9e1..dd151039101c3 100644 --- a/packages/create-next-app/package.json +++ b/packages/create-next-app/package.json @@ -1,6 +1,6 @@ { "name": "create-next-app", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "keywords": [ "react", "next", diff --git a/packages/next-bundle-analyzer/package.json b/packages/next-bundle-analyzer/package.json index e189b84527c4f..6a9bba9043a26 100644 --- a/packages/next-bundle-analyzer/package.json +++ b/packages/next-bundle-analyzer/package.json @@ -1,6 +1,6 @@ { "name": "@next/bundle-analyzer", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "main": "index.js", "license": "MIT", "repository": { diff --git a/packages/next-mdx/package.json b/packages/next-mdx/package.json index ca7523cccf807..ef089212d8589 100644 --- a/packages/next-mdx/package.json +++ b/packages/next-mdx/package.json @@ -1,6 +1,6 @@ { "name": "@next/mdx", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "main": "index.js", "license": "MIT", "repository": { diff --git a/packages/next-plugin-google-analytics/package.json b/packages/next-plugin-google-analytics/package.json index fc0c9a17f44ad..98a1b37779357 100644 --- a/packages/next-plugin-google-analytics/package.json +++ b/packages/next-plugin-google-analytics/package.json @@ -1,6 +1,6 @@ { "name": "@next/plugin-google-analytics", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "nextjs": { "name": "Google Analytics", "required-env": [ diff --git a/packages/next-plugin-material-ui/package.json b/packages/next-plugin-material-ui/package.json index 42aede457194d..47f75e8dea01c 100644 --- a/packages/next-plugin-material-ui/package.json +++ b/packages/next-plugin-material-ui/package.json @@ -1,6 +1,6 @@ { "name": "@next/plugin-material-ui", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "nextjs": { "name": "Material UI", "required-env": [] diff --git a/packages/next-plugin-sentry/package.json b/packages/next-plugin-sentry/package.json index d7e339f6b28ff..1abe2b37c17b3 100644 --- a/packages/next-plugin-sentry/package.json +++ b/packages/next-plugin-sentry/package.json @@ -1,6 +1,6 @@ { "name": "@next/plugin-sentry", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "nextjs": { "name": "Sentry", "required-env": [ diff --git a/packages/next-polyfill-nomodule/package.json b/packages/next-polyfill-nomodule/package.json index 465e672b63e28..00d3804ec3d4d 100644 --- a/packages/next-polyfill-nomodule/package.json +++ b/packages/next-polyfill-nomodule/package.json @@ -1,6 +1,6 @@ { "name": "@next/polyfill-nomodule", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "description": "A polyfill for non-dead, nomodule browsers.", "main": "dist/polyfill-nomodule.js", "license": "MIT", diff --git a/packages/next/package.json b/packages/next/package.json index e877198846b9f..faf0bbefa2abc 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -1,6 +1,6 @@ { "name": "next", - "version": "9.2.2-canary.19", + "version": "9.2.2-canary.20", "description": "The React Framework", "main": "./dist/server/next.js", "license": "MIT", @@ -73,7 +73,7 @@ "@babel/preset-typescript": "7.7.2", "@babel/runtime": "7.7.2", "@babel/runtime-corejs2": "7.7.2", - "@next/polyfill-nomodule": "9.2.2-canary.19", + "@next/polyfill-nomodule": "9.2.2-canary.20", "amphtml-validator": "1.0.23", "async-retry": "1.2.3", "async-sema": "3.0.0",