Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix edge function req.url handling with rewrite #41139

Merged
merged 5 commits into from Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
45 changes: 19 additions & 26 deletions packages/next/server/next-server.ts
Expand Up @@ -2033,44 +2033,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 +2073,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
25 changes: 25 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 @@ -259,6 +265,25 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
if (!pathname) {
throw new Error('pathname is undefined')
}
console.log('web-server render', req.url)
ijjk marked this conversation as resolved.
Show resolved Hide resolved

// 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
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