Skip to content

Commit

Permalink
Fix edge function req.url handling with rewrite (vercel#41139)
Browse files Browse the repository at this point in the history
This ensures we correctly normalize the URL in edge functions and handle rewrites properly. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

Fixes: [slack thread](https://vercel.slack.com/archives/C035J346QQL/p1664821822240719?thread_ts=1664813755.099319&cid=C035J346QQL)
  • Loading branch information
ijjk authored and Kikobeats committed Oct 24, 2022
1 parent bb31025 commit 9e8f99a
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 49 deletions.
51 changes: 32 additions & 19 deletions packages/next/build/webpack/loaders/next-serverless-loader/utils.ts
Expand Up @@ -68,6 +68,26 @@ export type ServerlessHandlerCtx = {
i18n?: NextConfig['i18n']
}

export function normalizeVercelUrl(
req: BaseNextRequest | IncomingMessage,
trustQuery: boolean,
paramKeys?: string[],
pageIsDynamic?: boolean,
defaultRouteRegex?: ReturnType<typeof getNamedRouteRegex> | undefined
) {
// make sure to normalize req.url on Vercel to strip dynamic params
// from the query which are added during routing
if (pageIsDynamic && trustQuery && defaultRouteRegex) {
const _parsedUrl = parseUrl(req.url!, true)
delete (_parsedUrl as any).search

for (const param of paramKeys || Object.keys(defaultRouteRegex.groups)) {
delete _parsedUrl.query[param]
}
req.url = formatUrl(_parsedUrl)
}
}

export function interpolateDynamicPath(
pathname: string,
params: ParsedUrlQuery,
Expand Down Expand Up @@ -336,24 +356,6 @@ export function getUtils({
)(req.headers['x-now-route-matches'] as string) as ParsedUrlQuery
}

function normalizeVercelUrl(
req: BaseNextRequest | IncomingMessage,
trustQuery: boolean,
paramKeys?: string[]
) {
// make sure to normalize req.url on Vercel to strip dynamic params
// from the query which are added during routing
if (pageIsDynamic && trustQuery && defaultRouteRegex) {
const _parsedUrl = parseUrl(req.url!, true)
delete (_parsedUrl as any).search

for (const param of paramKeys || Object.keys(defaultRouteRegex.groups)) {
delete _parsedUrl.query[param]
}
req.url = formatUrl(_parsedUrl)
}
}

function normalizeDynamicRouteParams(
params: ParsedUrlQuery,
ignoreOptional?: boolean
Expand Down Expand Up @@ -571,11 +573,22 @@ export function getUtils({
handleRewrites,
handleBasePath,
defaultRouteRegex,
normalizeVercelUrl,
dynamicRouteMatcher,
defaultRouteMatches,
getParamsFromRouteMatches,
normalizeDynamicRouteParams,
normalizeVercelUrl: (
req: BaseNextRequest | IncomingMessage,
trustQuery: boolean,
paramKeys?: string[]
) =>
normalizeVercelUrl(
req,
trustQuery,
paramKeys,
pageIsDynamic,
defaultRouteRegex
),
interpolateDynamicPath: (
pathname: string,
params: Record<string, string | string[]>
Expand Down
47 changes: 19 additions & 28 deletions packages/next/server/next-server.ts
Expand Up @@ -100,8 +100,6 @@ import { checkIsManualRevalidate } from './api-utils'
import { shouldUseReactRoot, isTargetLikeServerless } from './utils'
import ResponseCache from './response-cache'
import { IncrementalCache } from './lib/incremental-cache'
import { interpolateDynamicPath } from '../build/webpack/loaders/next-serverless-loader/utils'
import { getNamedRouteRegex } from '../shared/lib/router/utils/route-regex'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'

if (shouldUseReactRoot) {
Expand Down Expand Up @@ -2033,44 +2031,37 @@ export default class NextNodeServer extends BaseServer {
appPaths: string[] | null
onWarning?: (warning: Error) => void
}): Promise<FetchEventResult | null> {
let middlewareInfo: ReturnType<typeof this.getEdgeFunctionInfo> | undefined
let edgeInfo: ReturnType<typeof this.getEdgeFunctionInfo> | undefined

const { query, page } = params

await this.ensureEdgeFunction({ page, appPaths: params.appPaths })
middlewareInfo = this.getEdgeFunctionInfo({
edgeInfo = this.getEdgeFunctionInfo({
page,
middleware: false,
})

if (!middlewareInfo) {
if (!edgeInfo) {
return null
}

// For middleware to "fetch" we must always provide an absolute URL
const locale = query.__nextLocale
// For edge to "fetch" we must always provide an absolute URL
const isDataReq = !!query.__nextDataReq
const queryString = urlQueryToSearchParams(query).toString()
const initialUrl = new URL(
getRequestMeta(params.req, '__NEXT_INIT_URL') || '/',
'http://n'
)
const queryString = urlQueryToSearchParams({
...Object.fromEntries(initialUrl.searchParams),
...query,
...params.params,
}).toString()

if (isDataReq) {
params.req.headers['x-nextjs-data'] = '1'
}

let normalizedPathname = normalizeAppPath(page)
if (isDynamicRoute(normalizedPathname)) {
const routeRegex = getNamedRouteRegex(normalizedPathname)
normalizedPathname = interpolateDynamicPath(
normalizedPathname,
Object.assign({}, params.params, query),
routeRegex
)
}

const url = `${getRequestMeta(params.req, '_protocol')}://${
this.hostname
}:${this.port}${locale ? `/${locale}` : ''}${normalizedPathname}${
queryString ? `?${queryString}` : ''
}`
initialUrl.search = queryString
const url = initialUrl.toString()

if (!url.startsWith('http')) {
throw new Error(
Expand All @@ -2080,10 +2071,10 @@ export default class NextNodeServer extends BaseServer {

const result = await run({
distDir: this.distDir,
name: middlewareInfo.name,
paths: middlewareInfo.paths,
env: middlewareInfo.env,
edgeFunctionEntry: middlewareInfo,
name: edgeInfo.name,
paths: edgeInfo.paths,
env: edgeInfo.env,
edgeFunctionEntry: edgeInfo,
request: {
headers: params.req.headers,
method: params.req.method,
Expand Down
24 changes: 24 additions & 0 deletions packages/next/server/web-server.ts
Expand Up @@ -21,6 +21,12 @@ import getRouteFromAssetPath from '../shared/lib/router/utils/get-route-from-ass
import { detectDomainLocale } from '../shared/lib/i18n/detect-domain-locale'
import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path'
import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-slash'
import { isDynamicRoute } from '../shared/lib/router/utils'
import {
interpolateDynamicPath,
normalizeVercelUrl,
} from '../build/webpack/loaders/next-serverless-loader/utils'
import { getNamedRouteRegex } from '../shared/lib/router/utils/route-regex'

interface WebServerOptions extends Options {
webServerConfig: {
Expand Down Expand Up @@ -260,6 +266,24 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
throw new Error('pathname is undefined')
}

// interpolate query information into page for dynamic route
// so that rewritten paths are handled properly
if (pathname !== this.serverOptions.webServerConfig.page) {
pathname = this.serverOptions.webServerConfig.page

if (isDynamicRoute(pathname)) {
const routeRegex = getNamedRouteRegex(pathname)
pathname = interpolateDynamicPath(pathname, query, routeRegex)
normalizeVercelUrl(
req,
true,
Object.keys(routeRegex.routeKeys),
true,
routeRegex
)
}
}

// next.js core assumes page path without trailing slash
pathname = removeTrailingSlash(pathname)

Expand Down
14 changes: 14 additions & 0 deletions test/e2e/edge-render-getserversideprops/app/next.config.js
@@ -0,0 +1,14 @@
module.exports = {
rewrites() {
return [
{
source: '/rewrite-me',
destination: '/',
},
{
source: '/rewrite-me-dynamic',
destination: '/first',
},
]
},
}
3 changes: 2 additions & 1 deletion test/e2e/edge-render-getserversideprops/app/pages/[id].js
Expand Up @@ -11,9 +11,10 @@ export default function Page(props) {
)
}

export function getServerSideProps({ params, query }) {
export function getServerSideProps({ req, params, query }) {
return {
props: {
url: req.url,
query,
params,
now: Date.now(),
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/edge-render-getserversideprops/app/pages/index.js
Expand Up @@ -11,9 +11,10 @@ export default function Page(props) {
)
}

export function getServerSideProps({ params, query }) {
export function getServerSideProps({ req, params, query }) {
return {
props: {
url: req.url,
query,
now: Date.now(),
params: params || null,
Expand Down
26 changes: 26 additions & 0 deletions test/e2e/edge-render-getserversideprops/index.test.ts
Expand Up @@ -28,6 +28,7 @@ describe('edge-render-getserversideprops', () => {
const props = JSON.parse($('#props').text())
expect(props.query).toEqual({})
expect(props.params).toBe(null)
expect(props.url).toBe('/')
})

it('should have correct query/params on /[id]', async () => {
Expand All @@ -37,6 +38,31 @@ describe('edge-render-getserversideprops', () => {
const props = JSON.parse($('#props').text())
expect(props.query).toEqual({ id: '123', hello: 'world' })
expect(props.params).toEqual({ id: '123' })
expect(props.url).toBe('/123?hello=world')
})

it('should have correct query/params on rewrite', async () => {
const html = await renderViaHTTP(next.url, '/rewrite-me', {
hello: 'world',
})
const $ = cheerio.load(html)
expect($('#page').text()).toBe('/index')
const props = JSON.parse($('#props').text())
expect(props.query).toEqual({ hello: 'world' })
expect(props.params).toEqual(null)
expect(props.url).toBe('/rewrite-me?hello=world')
})

it('should have correct query/params on dynamic rewrite', async () => {
const html = await renderViaHTTP(next.url, '/rewrite-me-dynamic', {
hello: 'world',
})
const $ = cheerio.load(html)
expect($('#page').text()).toBe('/[id]')
const props = JSON.parse($('#props').text())
expect(props.query).toEqual({ id: 'first', hello: 'world' })
expect(props.params).toEqual({ id: 'first' })
expect(props.url).toBe('/rewrite-me-dynamic?hello=world')
})

it('should respond to _next/data for index correctly', async () => {
Expand Down

0 comments on commit 9e8f99a

Please sign in to comment.