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
vercel · Feb 14, 2020 · a7337a9 · a7337a9
1 parent fbbb184
commit a7337a9
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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')
+    })
   })
 }