Skip to content

Commit

Permalink
Support colons in Middleware NextResponse.rewrite path (#32543)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
io and nkzawa committed Mar 3, 2022
1 parent 728d826 commit 2c7f8b3
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 7 deletions.
10 changes: 3 additions & 7 deletions packages/next/server/next-server.ts
Expand Up @@ -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, {
Expand Down Expand Up @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions 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>
</>
)
}
10 changes: 10 additions & 0 deletions test/integration/middleware/core/pages/rewrites/_middleware.js
Expand Up @@ -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')
}
Expand Down
80 changes: 80 additions & 0 deletions test/integration/middleware/core/test/index.test.js
Expand Up @@ -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}`,
Expand Down

0 comments on commit 2c7f8b3

Please sign in to comment.