diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index f43eb66296ba2..f306df06eac30 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -103,7 +103,8 @@ const nextServerlessLoader: loader.Loader = function() { if (params) { const { parsedDestination } = prepareDestination( rewrite.destination, - params + params, + parsedUrl.query ) Object.assign(parsedUrl.query, parsedDestination.query, params) delete parsedDestination.query diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 0a22081d29d70..6d988fc5b58c2 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -496,10 +496,11 @@ export default class Server { match: route.match, statusCode: route.statusCode, name: `Redirect route`, - fn: async (_req, res, params, _parsedUrl) => { + fn: async (_req, res, params, parsedUrl) => { const { parsedDestination } = prepareDestination( route.destination, params, + parsedUrl.query, true ) const updatedDestination = formatUrl(parsedDestination) @@ -528,10 +529,11 @@ export default class Server { type: route.type, name: `Rewrite route`, match: route.match, - fn: async (req, res, params, _parsedUrl) => { + fn: async (req, res, params, parsedUrl) => { const { newUrl, parsedDestination } = prepareDestination( route.destination, - params + params, + parsedUrl.query ) // external rewrite, proxy it diff --git a/packages/next/next-server/server/router.ts b/packages/next/next-server/server/router.ts index 067e0dd4aaec8..9561f14ea1348 100644 --- a/packages/next/next-server/server/router.ts +++ b/packages/next/next-server/server/router.ts @@ -1,5 +1,6 @@ import { IncomingMessage, ServerResponse } from 'http' import { parse as parseUrl, UrlWithParsedQuery } from 'url' +import { ParsedUrlQuery } from 'querystring' import { compile as compilePathToRegex } from 'next/dist/compiled/path-to-regexp' import pathMatch from './lib/path-match' @@ -36,6 +37,7 @@ export type PageChecker = (pathname: string) => Promise export const prepareDestination = ( destination: string, params: Params, + query: ParsedUrlQuery, isRedirect?: boolean ) => { const parsedDestination = parseUrl(destination, true) @@ -62,7 +64,8 @@ export const prepareDestination = ( destQuery[key] = value } - // add params to query if it's not a redirect + // add path params to query if it's not a redirect and not + // already defined in destination query if (!isRedirect) { for (const [name, value] of Object.entries(params)) { if (!(name in destQuery)) { @@ -87,6 +90,16 @@ export const prepareDestination = ( } throw err } + + // Query merge order lowest priority to highest + // 1. initial URL query values + // 2. path segment values + // 3. destination specified query values + parsedDestination.query = { + ...query, + ...parsedDestination.query, + } + return { newUrl, parsedDestination, @@ -203,11 +216,6 @@ export default class Router { // Check if the match function matched if (newParams) { - // Combine parameters and querystring - if (route.type === 'rewrite' || route.type === 'redirect') { - parsedUrlUpdated.query = { ...parsedUrlUpdated.query, ...newParams } - } - const result = await route.fn(req, res, newParams, parsedUrlUpdated) // The response was handled diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index d931611ce49fc..c3189ffd10dc4 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -181,6 +181,7 @@ const runTests = (isDev = false) => { expect(query).toEqual({ first: 'hello', second: 'world', + a: 'b', }) }) @@ -191,6 +192,31 @@ const runTests = (isDev = false) => { expect(html).toMatch(/second/) }) + it('should handle query for rewrite correctly', async () => { + // query merge order lowest priority to highest + // 1. initial URL query values + // 2. path segment values + // 3. destination specified query values + + const html = await renderViaHTTP( + appPort, + '/query-rewrite/first/second?section=overridden&name=overridden&first=overridden&second=overridden&keep=me' + ) + + const data = JSON.parse( + cheerio + .load(html)('p') + .text() + ) + expect(data).toEqual({ + first: 'first', + second: 'second', + section: 'first', + name: 'second', + keep: 'me', + }) + }) + // current routes order do not allow rewrites to override page // but allow redirects to it('should not allow rewrite to override page file', async () => { @@ -281,9 +307,26 @@ const runTests = (isDev = false) => { }) it('should support proxying to external resource', async () => { - const res = await fetchViaHTTP(appPort, '/proxy-me/first') + const res = await fetchViaHTTP(appPort, '/proxy-me/first?keep=me&and=me') expect(res.status).toBe(200) - expect([...externalServerHits]).toEqual(['/first?path=first']) + expect( + [...externalServerHits].map(u => { + const { pathname, query } = url.parse(u, true) + return { + pathname, + query, + } + }) + ).toEqual([ + { + pathname: '/first', + query: { + path: 'first', + keep: 'me', + and: 'me', + }, + }, + ]) expect(await res.text()).toContain('hi from external') })