From 2c7f8b3d7b098d5f0847662c1b1ee1a10f1efbc1 Mon Sep 17 00:00:00 2001
From: io <jonas@klarstrup.dk>
Date: Thu, 3 Mar 2022 20:36:53 +0100
Subject: [PATCH] Support colons in Middleware NextResponse.rewrite path
 (#32543)

* Add failing colon rewrite test

* add test fixture

* better colon rewrite tests

* middleware rewrite colon tests with query parameters

* fix #31523

this addresses the symptom but the real systemic issue is that prepareDestination is called on rewrite/redirect URLs, which have no defined special behavior for colons and they should not be compiled at all

* hack around prepareDestination to skip compiling x-middleware-rewrite

this is a bit nicer than just escaping colons, but ideally we find a way to obviate prepareDestination

* obviate prepareDestination for x-middleware-rewrite handling

* don't clobber rewrite query data

* omit redundant type

* catch up to main

* It looks like newUrl should contain only pathname

Co-authored-by: Naoyuki Kanezawa <naoyuki.kanezawa@gmail.com>
---
 packages/next/server/next-server.ts           | 10 +--
 .../middleware/core/pages/rewrites/[param].js | 12 +++
 .../core/pages/rewrites/_middleware.js        | 10 +++
 .../middleware/core/test/index.test.js        | 80 +++++++++++++++++++
 4 files changed, 105 insertions(+), 7 deletions(-)
 create mode 100644 test/integration/middleware/core/pages/rewrites/[param].js

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
index 8782735790a5..65876d9f2fdb 100644
--- a/packages/next/server/next-server.ts
+++ b/packages/next/server/next-server.ts
@@ -49,7 +49,7 @@ import { getExtension, serveStatic } from './serve-static'
 import { ParsedUrlQuery } from 'querystring'
 import { apiResolver } from './api-utils/node'
 import { RenderOpts, renderToHTML } from './render'
-import { ParsedUrl } from '../shared/lib/router/utils/parse-url'
+import { ParsedUrl, parseUrl } from '../shared/lib/router/utils/parse-url'
 import * as Log from '../build/output/log'
 
 import BaseServer, {
@@ -1131,12 +1131,8 @@ export default class NextNodeServer extends BaseServer {
           const rewritePath = result.response.headers.get(
             'x-middleware-rewrite'
           )!
-          const { newUrl, parsedDestination } = prepareDestination({
-            appendParamsToQuery: false,
-            destination: rewritePath,
-            params: _params,
-            query: {},
-          })
+          const parsedDestination = parseUrl(rewritePath)
+          const newUrl = parsedDestination.pathname
 
           // TODO: remove after next minor version current `v12.0.9`
           this.warnIfQueryParametersWereDeleted(
diff --git a/test/integration/middleware/core/pages/rewrites/[param].js b/test/integration/middleware/core/pages/rewrites/[param].js
new file mode 100644
index 000000000000..d33f75dd2da6
--- /dev/null
+++ b/test/integration/middleware/core/pages/rewrites/[param].js
@@ -0,0 +1,12 @@
+export const getServerSideProps = ({ params, query }) => {
+  return { props: { params, query } }
+}
+
+export default function Page({ params: { param }, query: { qp } }) {
+  return (
+    <>
+      <p id="props">{param}</p>
+      <p id="qp">{qp}</p>
+    </>
+  )
+}
diff --git a/test/integration/middleware/core/pages/rewrites/_middleware.js b/test/integration/middleware/core/pages/rewrites/_middleware.js
index d9b83488318c..dcb61e1d176d 100644
--- a/test/integration/middleware/core/pages/rewrites/_middleware.js
+++ b/test/integration/middleware/core/pages/rewrites/_middleware.js
@@ -41,6 +41,16 @@ export async function middleware(request) {
     return NextResponse.rewrite(url)
   }
 
+  if (url.pathname === '/rewrites/rewrite-me-with-a-colon') {
+    url.pathname = '/rewrites/with:colon'
+    return NextResponse.rewrite(url)
+  }
+
+  if (url.pathname === '/rewrites/colon:here') {
+    url.pathname = '/rewrites/no-colon-here'
+    return NextResponse.rewrite(url)
+  }
+
   if (url.pathname === '/rewrites/rewrite-me-to-vercel') {
     return NextResponse.rewrite('https://vercel.com')
   }
diff --git a/test/integration/middleware/core/test/index.test.js b/test/integration/middleware/core/test/index.test.js
index 5f309b9dcbf7..b1ad4d164eec 100644
--- a/test/integration/middleware/core/test/index.test.js
+++ b/test/integration/middleware/core/test/index.test.js
@@ -359,6 +359,86 @@ function rewriteTests(log, locale = '') {
     expect($('.title').text()).toBe('About Page')
   })
 
+  it(`${locale} support colons in path`, async () => {
+    const path = `${locale}/rewrites/not:param`
+    const res = await fetchViaHTTP(context.appPort, path)
+    const html = await res.text()
+    const $ = cheerio.load(html)
+    expect($('#props').text()).toBe('not:param')
+    const browser = await webdriver(context.appPort, path)
+    try {
+      expect(await browser.eval(`window.location.pathname`)).toBe(path)
+    } finally {
+      await browser.close()
+    }
+  })
+
+  it(`${locale} can rewrite to path with colon`, async () => {
+    const path = `${locale}/rewrites/rewrite-me-with-a-colon`
+    const res = await fetchViaHTTP(context.appPort, path)
+    const html = await res.text()
+    const $ = cheerio.load(html)
+    expect($('#props').text()).toBe('with:colon')
+    const browser = await webdriver(context.appPort, path)
+    try {
+      expect(await browser.eval(`window.location.pathname`)).toBe(path)
+    } finally {
+      await browser.close()
+    }
+  })
+
+  it(`${locale} can rewrite from path with colon`, async () => {
+    const path = `${locale}/rewrites/colon:here`
+    const res = await fetchViaHTTP(context.appPort, path)
+    const html = await res.text()
+    const $ = cheerio.load(html)
+    expect($('#props').text()).toBe('no-colon-here')
+    const browser = await webdriver(context.appPort, path)
+    try {
+      expect(await browser.eval(`window.location.pathname`)).toBe(path)
+    } finally {
+      await browser.close()
+    }
+  })
+
+  it(`${locale} can rewrite from path with colon and retain query parameter`, async () => {
+    const path = `${locale}/rewrites/colon:here?qp=arg`
+    const res = await fetchViaHTTP(context.appPort, path)
+    const html = await res.text()
+    const $ = cheerio.load(html)
+    expect($('#props').text()).toBe('no-colon-here')
+    expect($('#qp').text()).toBe('arg')
+    const browser = await webdriver(context.appPort, path)
+    try {
+      expect(
+        await browser.eval(
+          `window.location.href.replace(window.location.origin, '')`
+        )
+      ).toBe(path)
+    } finally {
+      await browser.close()
+    }
+  })
+
+  it(`${locale} can rewrite to path with colon and retain query parameter`, async () => {
+    const path = `${locale}/rewrites/rewrite-me-with-a-colon?qp=arg`
+    const res = await fetchViaHTTP(context.appPort, path)
+    const html = await res.text()
+    const $ = cheerio.load(html)
+    expect($('#props').text()).toBe('with:colon')
+    expect($('#qp').text()).toBe('arg')
+    const browser = await webdriver(context.appPort, path)
+    try {
+      expect(
+        await browser.eval(
+          `window.location.href.replace(window.location.origin, '')`
+        )
+      ).toBe(path)
+    } finally {
+      await browser.close()
+    }
+  })
+
   it(`${locale} should rewrite when not using localhost`, async () => {
     const res = await fetchViaHTTP(
       `http://localtest.me:${context.appPort}`,