From 5fc4325aa62b7e85b3b4707c20d4a8ce35ab9fee Mon Sep 17 00:00:00 2001 From: Javi Velasco Date: Tue, 9 Nov 2021 02:28:39 +0100 Subject: [PATCH] Fix middleware i18n rewrites (#31174) Fixes #30897 This PR fixes the linked issue where rewrites are not being applied for locale. It adds the corresponding test but also, as it was added in debugging process, it introduces a helper to read/write into the `request` object. We are currently writing directly into the request by casting to `any` and then using flags like `_nextRewrote`. Instead, this PR puts all of this metadata under a symbol so it is not directly accessible. This also allows to have a single place where all of this metadata is listed so we can add comments describing the purpose of each flag. In the same way, there is metadata written in the querystring. This is adding some types for it in order to throw some visibility on where is this metadata accessed. In an upcoming PR we can move all of it to the `request` object if possible to simplify the system. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have helpful link attached, see `contributing.md` --- .../loaders/next-serverless-loader/utils.ts | 20 +- packages/next/export/worker.ts | 15 +- packages/next/server/next-server.ts | 183 ++++++++++-------- packages/next/server/render.tsx | 7 +- packages/next/server/request-meta.ts | 66 +++++++ packages/next/server/router.ts | 24 ++- .../lib/router/utils/prepare-destination.ts | 125 ++++++------ .../lib/router/utils/resolve-rewrites.ts | 14 +- .../middleware/with-i18n/next.config.js | 6 + .../middleware/with-i18n/pages/_middleware.js | 20 ++ .../with-i18n/pages/test/[country].js | 22 +++ .../middleware/with-i18n/test/index.test.js | 64 ++++++ 12 files changed, 383 insertions(+), 183 deletions(-) create mode 100644 packages/next/server/request-meta.ts create mode 100644 test/integration/middleware/with-i18n/next.config.js create mode 100644 test/integration/middleware/with-i18n/pages/_middleware.js create mode 100644 test/integration/middleware/with-i18n/pages/test/[country].js create mode 100644 test/integration/middleware/with-i18n/test/index.test.js diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts b/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts index 27383855f07a..92adf265a278 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts @@ -6,8 +6,9 @@ import { normalizeLocalePath } from '../../../../shared/lib/i18n/normalize-local import pathMatch from '../../../../shared/lib/router/utils/path-match' import { getRouteRegex } from '../../../../shared/lib/router/utils/route-regex' import { getRouteMatcher } from '../../../../shared/lib/router/utils/route-matcher' -import prepareDestination, { +import { matchHas, + prepareDestination, } from '../../../../shared/lib/router/utils/prepare-destination' import { __ApiPreviewProps } from '../../../../server/api-utils' import { BuildManifest } from '../../../../server/get-page-files' @@ -23,6 +24,7 @@ import { denormalizePagePath } from '../../../../server/denormalize-page-path' import cookie from 'next/dist/compiled/cookie' import { TEMPORARY_REDIRECT_STATUS } from '../../../../shared/lib/constants' import { NextConfig } from '../../../../server/config' +import { addRequestMeta } from '../../../../server/request-meta' const getCustomRouteMatcher = pathMatch(true) @@ -99,12 +101,12 @@ export function getUtils({ } if (params) { - const { parsedDestination } = prepareDestination( - rewrite.destination, - params, - parsedUrl.query, - true - ) + const { parsedDestination } = prepareDestination({ + appendParamsToQuery: true, + destination: rewrite.destination, + params: params, + query: parsedUrl.query, + }) Object.assign(parsedUrl.query, parsedDestination.query) delete (parsedDestination as any).query @@ -372,7 +374,7 @@ export function getUtils({ if (detectedDomain) { defaultLocale = detectedDomain.defaultLocale detectedLocale = defaultLocale - ;(req as any).__nextIsLocaleDomain = true + addRequestMeta(req, '__nextIsLocaleDomain', true) } // if not domain specific locale use accept-language preferred @@ -392,7 +394,7 @@ export function getUtils({ ...parsedUrl, pathname: localePathResult.pathname, }) - ;(req as any).__nextStrippedLocale = true + addRequestMeta(req, '__nextStrippedLocale', true) parsedUrl.pathname = localePathResult.pathname } diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index 158d92aee5be..180def8d1cd2 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -1,3 +1,9 @@ +import type { ComponentType } from 'react' +import type { FontManifest } from '../server/font-utils' +import type { GetStaticProps } from '../types' +import type { IncomingMessage, ServerResponse } from 'http' +import type { NextConfigComplete } from '../server/config-shared' +import type { NextParsedUrlQuery } from '../server/request-meta' import url from 'url' import { extname, join, dirname, sep } from 'path' import { renderToHTML } from '../server/render' @@ -10,15 +16,10 @@ import { getRouteRegex } from '../shared/lib/router/utils/route-regex' import { normalizePagePath } from '../server/normalize-page-path' import { SERVER_PROPS_EXPORT_ERROR } from '../lib/constants' import '../server/node-polyfill-fetch' -import { IncomingMessage, ServerResponse } from 'http' -import { ComponentType } from 'react' -import { GetStaticProps } from '../types' import { requireFontManifest } from '../server/require' -import { FontManifest } from '../server/font-utils' import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path' import { trace } from '../trace' import { isInAmpMode } from '../shared/lib/amp' -import { NextConfigComplete } from '../server/config-shared' import { setHttpAgentOptions } from '../server/config' import RenderResult from '../server/render-result' import isError from '../lib/is-error' @@ -39,7 +40,7 @@ interface AmpValidation { interface PathMap { page: string - query?: { [key: string]: string | string[] } + query?: NextParsedUrlQuery } interface ExportPageInput { @@ -128,7 +129,7 @@ export default async function exportPage({ let query = { ...originalQuery } let params: { [key: string]: string | string[] } | undefined - let updatedPath = (query.__nextSsgPath as string) || path + let updatedPath = query.__nextSsgPath || path let locale = query.__nextLocale || renderOpts.locale delete query.__nextLocale delete query.__nextSsgPath diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index 1d4d37651ed1..a8185a08b2db 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -70,8 +70,9 @@ import Router, { route, Route, } from './router' -import prepareDestination, { +import { compileNonPath, + prepareDestination, } from '../shared/lib/router/utils/prepare-destination' import { sendRenderResult, setRevalidateHeaders } from './send-payload' import { serveStatic } from './serve-static' @@ -99,7 +100,6 @@ import { NextConfigComplete } from './config-shared' import { parseNextUrl } from '../shared/lib/router/utils/parse-next-url' import isError from '../lib/is-error' import { getMiddlewareInfo } from './require' -import { parseUrl as simpleParseUrl } from '../shared/lib/router/utils/parse-url' import { MIDDLEWARE_ROUTE } from '../lib/constants' import { NextResponse } from './web/spec-extension/response' import { run } from './web/sandbox' @@ -108,6 +108,8 @@ import type { FetchEventResult } from './web/types' import type { MiddlewareManifest } from '../build/webpack/plugins/middleware-plugin' import type { ParsedNextUrl } from '../shared/lib/router/utils/parse-next-url' import type { ParsedUrl } from '../shared/lib/router/utils/parse-url' +import type { NextParsedUrlQuery, NextUrlWithParsedQuery } from './request-meta' +import { addRequestMeta, getRequestMeta } from './request-meta' import { toNodeHeaders } from './web/utils' const getCustomRouteMatcher = pathMatch(true) @@ -120,7 +122,7 @@ type ExpressMiddleware = ( export type FindComponentsResult = { components: LoadComponentsReturnType - query: ParsedUrlQuery + query: NextParsedUrlQuery } interface RoutingItem { @@ -150,7 +152,7 @@ type RequestContext = { req: IncomingMessage res: ServerResponse pathname: string - query: ParsedUrlQuery + query: NextParsedUrlQuery renderOpts: RenderOptsPartial } @@ -331,7 +333,7 @@ export default class Server { private async handleRequest( req: IncomingMessage, res: ServerResponse, - parsedUrl?: UrlWithParsedQuery + parsedUrl?: NextUrlWithParsedQuery ): Promise { const urlParts = (req.url || '').split('?') const urlNoQuery = urlParts[0] @@ -349,18 +351,16 @@ export default class Server { // Parse url if parsedUrl not provided if (!parsedUrl || typeof parsedUrl !== 'object') { - const url: any = req.url - parsedUrl = parseUrl(url, true) + parsedUrl = parseUrl(req.url!, true) } - const { basePath, i18n } = this.nextConfig // Parse the querystring ourselves if the user doesn't handle querystring parsing if (typeof parsedUrl.query === 'string') { parsedUrl.query = parseQs(parsedUrl.query) } - ;(req as any).__NEXT_INIT_URL = req.url - ;(req as any).__NEXT_INIT_QUERY = Object.assign({}, parsedUrl.query) + addRequestMeta(req, '__NEXT_INIT_URL', req.url) + addRequestMeta(req, '__NEXT_INIT_QUERY', { ...parsedUrl.query }) const url = parseNextUrl({ headers: req.headers, @@ -369,8 +369,8 @@ export default class Server { }) if (url.basePath) { - ;(req as any)._nextHadBasePath = true - req.url = req.url!.replace(basePath, '') || '/' + req.url = req.url!.replace(this.nextConfig.basePath, '') || '/' + addRequestMeta(req, '_nextHadBasePath', true) } if ( @@ -387,17 +387,17 @@ export default class Server { isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), true ) - const { pathname, query } = parsedPath - let matchedPathname = pathname as string + + let matchedPathname = parsedPath.pathname! let matchedPathnameNoExt = isDataUrl ? matchedPathname.replace(/\.json$/, '') : matchedPathname - if (i18n) { + if (this.nextConfig.i18n) { const localePathResult = normalizeLocalePath( matchedPathname || '/', - i18n.locales + this.nextConfig.i18n.locales ) if (localePathResult.detectedLocale) { @@ -432,7 +432,7 @@ export default class Server { if (pageIsDynamic) { let params: ParsedUrlQuery | false = {} - Object.assign(parsedUrl.query, query) + Object.assign(parsedUrl.query, parsedPath.query) const paramsResult = utils.normalizeDynamicRouteParams( parsedUrl.query ) @@ -444,7 +444,7 @@ export default class Server { params = utils.getParamsFromRouteMatches( req, opts, - (parsedUrl.query.__nextLocale as string | undefined) || '' + parsedUrl.query.__nextLocale || '' ) if (opts.locale) { @@ -482,19 +482,21 @@ export default class Server { throw err } - parsedUrl.pathname = `${basePath || ''}${ - matchedPathname === '/' && basePath ? '' : matchedPathname + parsedUrl.pathname = `${this.nextConfig.basePath || ''}${ + matchedPathname === '/' && this.nextConfig.basePath + ? '' + : matchedPathname }` } - ;(req as any).__nextHadTrailingSlash = url.locale?.trailingSlash + addRequestMeta(req, '__nextHadTrailingSlash', url.locale?.trailingSlash) if (url.locale?.domain) { - ;(req as any).__nextIsLocaleDomain = true + addRequestMeta(req, '__nextIsLocaleDomain', true) } if (url.locale?.path.detectedLocale) { req.url = formatUrl(url) - ;(req as any).__nextStrippedLocale = true + addRequestMeta(req, '__nextStrippedLocale', true) if (url.pathname === '/api' || url.pathname.startsWith('/api/')) { return this.render404(req, res, parsedUrl) } @@ -679,7 +681,7 @@ export default class Server { i18n: this.nextConfig.i18n, trailingSlash: this.nextConfig.trailingSlash, }, - url: (params.request as any).__NEXT_INIT_URL, + url: getRequestMeta(params.request, '__NEXT_INIT_URL')!, page: page, }, ssr: !!this.nextConfig.experimental.concurrentFeatures, @@ -827,15 +829,16 @@ export default class Server { let pathname = `/${params.path.join('/')}` pathname = getRouteFromAssetPath(pathname, '.json') - const { i18n } = this.nextConfig - - if (i18n) { + if (this.nextConfig.i18n) { const { host } = req?.headers || {} // remove port from host and remove port if present const hostname = host?.split(':')[0].toLowerCase() - const localePathResult = normalizeLocalePath(pathname, i18n.locales) + const localePathResult = normalizeLocalePath( + pathname, + this.nextConfig.i18n.locales + ) const { defaultLocale } = - detectDomainLocale(i18n.domains, hostname) || {} + detectDomainLocale(this.nextConfig.i18n.domains, hostname) || {} let detectedLocale = '' @@ -844,9 +847,9 @@ export default class Server { detectedLocale = localePathResult.detectedLocale } - _parsedUrl.query.__nextLocale = detectedLocale! + _parsedUrl.query.__nextLocale = detectedLocale _parsedUrl.query.__nextDefaultLocale = - defaultLocale || i18n.defaultLocale + defaultLocale || this.nextConfig.i18n.defaultLocale if (!detectedLocale) { _parsedUrl.query.__nextLocale = @@ -970,7 +973,9 @@ export default class Server { // we need to re-encode them here but still allow passing through // values from rewrites/redirects const stringifyQuery = (req: IncomingMessage, query: ParsedUrlQuery) => { - const initialQueryValues = Object.values((req as any).__NEXT_INIT_QUERY) + const initialQueryValues = Object.values( + getRequestMeta(req, '__NEXT_INIT_QUERY') || {} + ) return stringifyQs(query, undefined, undefined, { encodeURIComponent(value) { @@ -1037,12 +1042,12 @@ export default class Server { statusCode: redirectRoute.statusCode, name: `Redirect route ${redirectRoute.source}`, fn: async (req, res, params, parsedUrl) => { - const { parsedDestination } = prepareDestination( - redirectRoute.destination, - params, - parsedUrl.query, - false - ) + const { parsedDestination } = prepareDestination({ + appendParamsToQuery: false, + destination: redirectRoute.destination, + params: params, + query: parsedUrl.query, + }) const { query } = parsedDestination delete (parsedDestination as any).query @@ -1082,21 +1087,20 @@ export default class Server { name: `Rewrite route ${rewriteRoute.source}`, match: rewriteRoute.match, fn: async (req, res, params, parsedUrl) => { - const { newUrl, parsedDestination } = prepareDestination( - rewriteRoute.destination, - params, - parsedUrl.query, - true - ) + const { newUrl, parsedDestination } = prepareDestination({ + appendParamsToQuery: true, + destination: rewriteRoute.destination, + params: params, + query: parsedUrl.query, + }) // external rewrite, proxy it if (parsedDestination.protocol) { return proxyRequest(req, res, parsedDestination) } - ;(req as any)._nextRewroteUrl = newUrl - ;(req as any)._nextDidRewrite = - (req as any)._nextRewroteUrl !== req.url + addRequestMeta(req, '_nextRewroteUrl', newUrl) + addRequestMeta(req, '_nextDidRewrite', newUrl !== req.url) return { finished: false, @@ -1135,7 +1139,7 @@ export default class Server { type: 'route', name: 'middleware catchall', fn: async (req, res, _params, parsed) => { - const fullUrl = (req as any).__NEXT_INIT_URL + const fullUrl = getRequestMeta(req, '__NEXT_INIT_URL') const parsedUrl = parseNextUrl({ url: fullUrl, headers: req.headers, @@ -1222,23 +1226,35 @@ export default class Server { } if (result.response.headers.has('x-middleware-rewrite')) { - const rewrite = result.response.headers.get('x-middleware-rewrite')! - const rewriteParsed = simpleParseUrl(rewrite) - if (rewriteParsed.protocol) { - return proxyRequest(req, res, rewriteParsed) + const { newUrl, parsedDestination } = prepareDestination({ + appendParamsToQuery: true, + destination: result.response.headers.get('x-middleware-rewrite')!, + params: _params, + query: parsedUrl.query, + }) + + if (parsedDestination.protocol) { + return proxyRequest(req, res, parsedDestination) } - ;(req as any)._nextRewroteUrl = rewrite - ;(req as any)._nextDidRewrite = - (req as any)._nextRewroteUrl !== req.url + if (this.nextConfig.i18n) { + const localePathResult = normalizeLocalePath( + newUrl, + this.nextConfig.i18n.locales + ) + if (localePathResult.detectedLocale) { + parsedDestination.query.__nextLocale = + localePathResult.detectedLocale + } + } + + addRequestMeta(req, '_nextRewroteUrl', newUrl) + addRequestMeta(req, '_nextDidRewrite', newUrl !== req.url) return { finished: false, - pathname: rewriteParsed.pathname, - query: { - ...parsedUrl.query, - ...rewriteParsed.query, - }, + pathname: newUrl, + query: parsedDestination.query, } } @@ -1575,7 +1591,7 @@ export default class Server { req: IncomingMessage res: ServerResponse pathname: string - query: ParsedUrlQuery + query: NextParsedUrlQuery } ): Promise { const userAgent = partialContext.req.headers['user-agent'] @@ -1636,8 +1652,8 @@ export default class Server { req: IncomingMessage, res: ServerResponse, pathname: string, - query: ParsedUrlQuery = {}, - parsedUrl?: UrlWithParsedQuery + query: NextParsedUrlQuery = {}, + parsedUrl?: NextUrlWithParsedQuery ): Promise { if (!pathname.startsWith('/')) { console.warn( @@ -1655,8 +1671,6 @@ export default class Server { pathname = '/' } - const url: any = req.url - // we allow custom servers to call render for all URLs // so check if we need to serve a static _next file or not. // we don't modify the URL for _next/data request but still @@ -1664,8 +1678,8 @@ export default class Server { if ( !this.minimalMode && !query._nextDataReq && - (url.match(/^\/_next\//) || - (this.hasStaticDir && url.match(/^\/static\//))) + (req.url?.match(/^\/_next\//) || + (this.hasStaticDir && req.url!.match(/^\/static\//))) ) { return this.handleRequest(req, res, parsedUrl) } @@ -1689,7 +1703,7 @@ export default class Server { protected async findPageComponents( pathname: string, - query: ParsedUrlQuery = {}, + query: NextParsedUrlQuery = {}, params: Params | null = null ): Promise { let paths = [ @@ -1729,12 +1743,12 @@ export default class Server { components, query: { ...(components.getStaticProps - ? { + ? ({ amp: query.amp, _nextDataReq: query._nextDataReq, __nextLocale: query.__nextLocale, __nextDefaultLocale: query.__nextDefaultLocale, - } + } as NextParsedUrlQuery) : query), ...(params || {}), }, @@ -1823,13 +1837,12 @@ export default class Server { typeof components.Document?.getInitialProps !== 'function' } - const locale = query.__nextLocale as string const defaultLocale = isSSG ? this.nextConfig.i18n?.defaultLocale - : (query.__nextDefaultLocale as string) + : query.__nextDefaultLocale - const { i18n } = this.nextConfig - const locales = i18n?.locales + const locale = query.__nextLocale + const locales = this.nextConfig.i18n?.locales let previewData: PreviewData let isPreviewMode = false @@ -1844,9 +1857,8 @@ export default class Server { // and we need to look up the path by the rewritten path let urlPathname = parseUrl(req.url || '').pathname || '/' - let resolvedUrlPathname = (req as any)._nextRewroteUrl - ? (req as any)._nextRewroteUrl - : urlPathname + let resolvedUrlPathname = + getRequestMeta(req, '_nextRewroteUrl') || urlPathname urlPathname = removePathTrailingSlash(urlPathname) resolvedUrlPathname = normalizeLocalePath( @@ -2315,7 +2327,7 @@ export default class Server { req: IncomingMessage, res: ServerResponse, pathname: string, - query: ParsedUrlQuery = {}, + query: NextParsedUrlQuery = {}, setHeaders = true ): Promise { if (setHeaders) { @@ -2467,17 +2479,18 @@ export default class Server { public async render404( req: IncomingMessage, res: ServerResponse, - parsedUrl?: UrlWithParsedQuery, + parsedUrl?: NextUrlWithParsedQuery, setHeaders = true ): Promise { - const url: any = req.url - const { pathname, query } = parsedUrl ? parsedUrl : parseUrl(url, true) - const { i18n } = this.nextConfig + const { pathname, query }: NextUrlWithParsedQuery = parsedUrl + ? parsedUrl + : parseUrl(req.url!, true) - if (i18n) { - query.__nextLocale = query.__nextLocale || i18n.defaultLocale + if (this.nextConfig.i18n) { + query.__nextLocale = + query.__nextLocale || this.nextConfig.i18n.defaultLocale query.__nextDefaultLocale = - query.__nextDefaultLocale || i18n.defaultLocale + query.__nextDefaultLocale || this.nextConfig.i18n.defaultLocale } res.statusCode = 404 diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index eed32ff812a8..0751b332cd2e 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -51,6 +51,7 @@ import { denormalizePagePath } from './denormalize-page-path' import type { FontManifest } from './font-utils' import type { LoadComponentsReturnType, ManifestItem } from './load-components' import { normalizePagePath } from './normalize-page-path' +import { getRequestMeta, NextParsedUrlQuery } from './request-meta' import { allowedStatusCodes, getRedirectStatus, @@ -277,7 +278,7 @@ export async function renderToHTML( req: IncomingMessage, res: ServerResponse, pathname: string, - query: ParsedUrlQuery, + query: NextParsedUrlQuery, renderOpts: RenderOpts ): Promise { // In dev we invalidate the cache by appending a timestamp to the resource URL. @@ -498,7 +499,7 @@ export async function renderToHTML( renderOpts.defaultLocale, renderOpts.domainLocales, isPreview, - (req as any).__nextIsLocaleDomain + getRequestMeta(req, '__nextIsLocaleDomain') ) const jsxStyleRegistry = createStyleRegistry() const ctx = { @@ -1167,7 +1168,7 @@ export async function renderToHTML( docComponentsRendered, dangerousAsPath: router.asPath, canonicalBase: - !renderOpts.ampPath && (req as any).__nextStrippedLocale + !renderOpts.ampPath && getRequestMeta(req, '__nextStrippedLocale') ? `${renderOpts.canonicalBase || ''}/${renderOpts.locale}` : renderOpts.canonicalBase, ampPath, diff --git a/packages/next/server/request-meta.ts b/packages/next/server/request-meta.ts new file mode 100644 index 000000000000..79866ad8913b --- /dev/null +++ b/packages/next/server/request-meta.ts @@ -0,0 +1,66 @@ +/* eslint-disable no-redeclare */ +import type { ParsedUrlQuery } from 'querystring' +import type { IncomingMessage } from 'http' +import type { UrlWithParsedQuery } from 'url' + +const NEXT_REQUEST_META = Symbol('NextRequestMeta') + +interface NextIncomingMessage extends IncomingMessage { + [NEXT_REQUEST_META]?: RequestMeta +} + +interface RequestMeta { + __NEXT_INIT_QUERY?: ParsedUrlQuery + __NEXT_INIT_URL?: string + __nextHadTrailingSlash?: boolean + __nextIsLocaleDomain?: boolean + __nextStrippedLocale?: boolean + _nextDidRewrite?: boolean + _nextHadBasePath?: boolean + _nextRewroteUrl?: string +} + +export function getRequestMeta( + req: NextIncomingMessage, + key?: undefined +): RequestMeta +export function getRequestMeta( + req: NextIncomingMessage, + key: K +): RequestMeta[K] +export function getRequestMeta( + req: NextIncomingMessage, + key?: K +): RequestMeta | RequestMeta[K] { + const meta = req[NEXT_REQUEST_META] || {} + return typeof key === 'string' ? meta[key] : meta +} + +export function setRequestMeta(req: NextIncomingMessage, meta: RequestMeta) { + req[NEXT_REQUEST_META] = meta + return getRequestMeta(req) +} + +export function addRequestMeta( + request: NextIncomingMessage, + key: K, + value: RequestMeta[K] +) { + const meta = getRequestMeta(request) + meta[key] = value + return setRequestMeta(request, meta) +} + +export type NextParsedUrlQuery = ParsedUrlQuery & { + __nextDefaultLocale?: string + __nextFallback?: 'true' + __nextLocale?: string + __nextSsgPath?: string + _nextBubbleNoFallback?: '1' + _nextDataReq?: '1' + amp?: '1' +} + +export interface NextUrlWithParsedQuery extends UrlWithParsedQuery { + query: NextParsedUrlQuery +} diff --git a/packages/next/server/router.ts b/packages/next/server/router.ts index f889ad0b0e18..fb0176c03274 100644 --- a/packages/next/server/router.ts +++ b/packages/next/server/router.ts @@ -1,12 +1,13 @@ import type { IncomingMessage, ServerResponse } from 'http' import type { ParsedUrlQuery } from 'querystring' -import type { UrlWithParsedQuery } from 'url' +import type { NextUrlWithParsedQuery } from './request-meta' import pathMatch from '../shared/lib/router/utils/path-match' import { removePathTrailingSlash } from '../client/normalize-trailing-slash' import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path' import { RouteHas } from '../lib/load-custom-routes' import { matchHas } from '../shared/lib/router/utils/prepare-destination' +import { getRequestMeta } from './request-meta' export const route = pathMatch() @@ -33,7 +34,7 @@ export type Route = { req: IncomingMessage, res: ServerResponse, params: Params, - parsedUrl: UrlWithParsedQuery + parsedUrl: NextUrlWithParsedQuery ) => Promise | RouteResult } @@ -122,7 +123,7 @@ export default class Router { async execute( req: IncomingMessage, res: ServerResponse, - parsedUrl: UrlWithParsedQuery + parsedUrl: NextUrlWithParsedQuery ): Promise { // memoize page check calls so we don't duplicate checks for pages const pageChecks: { [name: string]: Promise } = {} @@ -139,7 +140,7 @@ export default class Router { let parsedUrlUpdated = parsedUrl - const applyCheckTrue = async (checkParsedUrl: UrlWithParsedQuery) => { + const applyCheckTrue = async (checkParsedUrl: NextUrlWithParsedQuery) => { const originalFsPathname = checkParsedUrl.pathname const fsPathname = replaceBasePath(this.basePath, originalFsPathname!) @@ -264,7 +265,7 @@ export default class Router { ...(this.useFileSystemPublicRoutes ? [this.catchAllRoute] : []), ] const originallyHadBasePath = - !this.basePath || (req as any)._nextHadBasePath + !this.basePath || getRequestMeta(req, '_nextHadBasePath') for (const testRoute of allRoutes) { // if basePath is being used, the basePath will still be included @@ -308,14 +309,14 @@ export default class Router { } if ( - (req as any).__nextHadTrailingSlash && + getRequestMeta(req, '__nextHadTrailingSlash') && !currentPathname.endsWith('/') ) { currentPathname += '/' } } else { currentPathname = `${ - (req as any)._nextHadBasePath ? activeBasePath : '' + getRequestMeta(req, '_nextHadBasePath') ? activeBasePath : '' }${ activeBasePath && localePathResult.pathname === '/' ? '' @@ -340,7 +341,10 @@ export default class Router { // since we require basePath be present for non-custom-routes we // 404 here when we matched an fs route if (!keepBasePath) { - if (!originallyHadBasePath && !(req as any)._nextDidRewrite) { + if ( + !originallyHadBasePath && + !getRequestMeta(req, '_nextDidRewrite') + ) { if (requireBasePath) { // consider this a non-match so the 404 renders return false @@ -360,8 +364,8 @@ export default class Router { return true } - // since the fs route didn't match we need to re-add the basePath - // to continue checking rewrites with the basePath present + // since the fs route didn't finish routing we need to re-add the + // basePath to continue checking with the basePath present if (!keepBasePath) { parsedUrlUpdated.pathname = originalPathname } diff --git a/packages/next/shared/lib/router/utils/prepare-destination.ts b/packages/next/shared/lib/router/utils/prepare-destination.ts index b82b3f48ee71..7d6966c86cbd 100644 --- a/packages/next/shared/lib/router/utils/prepare-destination.ts +++ b/packages/next/shared/lib/router/utils/prepare-destination.ts @@ -1,28 +1,10 @@ import type { IncomingMessage } from 'http' -import type { ParsedUrlQuery } from 'querystring' -import { parseUrl } from './parse-url' -import * as pathToRegexp from 'next/dist/compiled/path-to-regexp' +import type { Key } from 'next/dist/compiled/path-to-regexp' +import type { NextParsedUrlQuery } from '../../../../server/request-meta' +import type { Params } from '../../../../server/router' import type { RouteHas } from '../../../../lib/load-custom-routes' - -type Params = { [param: string]: any } - -// ensure only a-zA-Z are used for param names for proper interpolating -// with path-to-regexp -export const getSafeParamName = (paramName: string) => { - let newParamName = '' - - for (let i = 0; i < paramName.length; i++) { - const charCode = paramName.charCodeAt(i) - - if ( - (charCode > 64 && charCode < 91) || // A-Z - (charCode > 96 && charCode < 123) // a-z - ) { - newParamName += paramName[i] - } - } - return newParamName -} +import { compile, pathToRegexp } from 'next/dist/compiled/path-to-regexp' +import { parseUrl } from './parse-url' export function matchHas( req: IncomingMessage, @@ -124,31 +106,23 @@ export function compileNonPath(value: string, params: Params): string { // the value needs to start with a forward-slash to be compiled // correctly - return pathToRegexp - .compile(`/${value}`, { validate: false })(params) - .substr(1) + return compile(`/${value}`, { validate: false })(params).substr(1) } -const escapeSegment = (str: string, segmentName: string) => - str.replace(new RegExp(`:${segmentName}`, 'g'), `__ESC_COLON_${segmentName}`) - -const unescapeSegments = (str: string) => str.replace(/__ESC_COLON_/gi, ':') - -export default function prepareDestination( - destination: string, - params: Params, - query: ParsedUrlQuery, +export function prepareDestination(args: { appendParamsToQuery: boolean -) { - // clone query so we don't modify the original - query = Object.assign({}, query) + destination: string + params: Params + query: NextParsedUrlQuery +}) { + const query = Object.assign({}, args.query) const hadLocale = query.__nextLocale delete query.__nextLocale delete query.__nextDefaultLocale - let escapedDestination = destination + let escapedDestination = args.destination - for (const param of Object.keys({ ...params, ...query })) { + for (const param of Object.keys({ ...args.params, ...query })) { escapedDestination = escapeSegment(escapedDestination, param) } @@ -158,17 +132,17 @@ export default function prepareDestination( `${parsedDestination.pathname!}${parsedDestination.hash || ''}` ) const destHostname = unescapeSegments(parsedDestination.hostname || '') - const destPathParamKeys: pathToRegexp.Key[] = [] - const destHostnameParamKeys: pathToRegexp.Key[] = [] - pathToRegexp.pathToRegexp(destPath, destPathParamKeys) - pathToRegexp.pathToRegexp(destHostname, destHostnameParamKeys) + const destPathParamKeys: Key[] = [] + const destHostnameParamKeys: Key[] = [] + pathToRegexp(destPath, destPathParamKeys) + pathToRegexp(destHostname, destHostnameParamKeys) const destParams: (string | number)[] = [] destPathParamKeys.forEach((key) => destParams.push(key.name)) destHostnameParamKeys.forEach((key) => destParams.push(key.name)) - const destPathCompiler = pathToRegexp.compile( + const destPathCompiler = compile( destPath, // we don't validate while compiling the destination since we should // have already validated before we got to this point and validating @@ -178,10 +152,8 @@ export default function prepareDestination( // params from a separate path-regex into another { validate: false } ) - const destHostnameCompiler = pathToRegexp.compile(destHostname, { - validate: false, - }) - let newUrl + + const destHostnameCompiler = compile(destHostname, { validate: false }) // update any params in query values for (const [key, strOrArray] of Object.entries(destQuery)) { @@ -189,16 +161,16 @@ export default function prepareDestination( // correctly if (Array.isArray(strOrArray)) { destQuery[key] = strOrArray.map((value) => - compileNonPath(unescapeSegments(value), params) + compileNonPath(unescapeSegments(value), args.params) ) } else { - destQuery[key] = compileNonPath(unescapeSegments(strOrArray), params) + destQuery[key] = compileNonPath(unescapeSegments(strOrArray), args.params) } } // add path params to query if it's not a redirect and not // already defined in destination query or path - let paramKeys = Object.keys(params) + let paramKeys = Object.keys(args.params) // remove internal param for i18n if (hadLocale) { @@ -206,30 +178,28 @@ export default function prepareDestination( } if ( - appendParamsToQuery && + args.appendParamsToQuery && !paramKeys.some((key) => destParams.includes(key)) ) { for (const key of paramKeys) { if (!(key in destQuery)) { - destQuery[key] = params[key] + destQuery[key] = args.params[key] } } } + let newUrl + try { - newUrl = destPathCompiler(params) + newUrl = destPathCompiler(args.params) const [pathname, hash] = newUrl.split('#') - parsedDestination.hostname = destHostnameCompiler(params) + parsedDestination.hostname = destHostnameCompiler(args.params) parsedDestination.pathname = pathname parsedDestination.hash = `${hash ? '#' : ''}${hash || ''}` delete (parsedDestination as any).search - } catch (err) { - if ( - (err as Error).message.match( - /Expected .*? to not repeat, but got an array/ - ) - ) { + } catch (err: any) { + if (err.message.match(/Expected .*? to not repeat, but got an array/)) { throw new Error( `To use a multi-match in the destination you must add \`*\` at the end of the param name to signify it should repeat. https://nextjs.org/docs/messages/invalid-multi-match` ) @@ -251,3 +221,34 @@ export default function prepareDestination( parsedDestination, } } + +/** + * Ensure only a-zA-Z are used for param names for proper interpolating + * with path-to-regexp + */ +function getSafeParamName(paramName: string) { + let newParamName = '' + + for (let i = 0; i < paramName.length; i++) { + const charCode = paramName.charCodeAt(i) + + if ( + (charCode > 64 && charCode < 91) || // A-Z + (charCode > 96 && charCode < 123) // a-z + ) { + newParamName += paramName[i] + } + } + return newParamName +} + +function escapeSegment(str: string, segmentName: string) { + return str.replace( + new RegExp(`:${segmentName}`, 'g'), + `__ESC_COLON_${segmentName}` + ) +} + +function unescapeSegments(str: string) { + return str.replace(/__ESC_COLON_/gi, ':') +} diff --git a/packages/next/shared/lib/router/utils/resolve-rewrites.ts b/packages/next/shared/lib/router/utils/resolve-rewrites.ts index b6551862abff..653a362ee0ba 100644 --- a/packages/next/shared/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/shared/lib/router/utils/resolve-rewrites.ts @@ -1,6 +1,6 @@ import { ParsedUrlQuery } from 'querystring' import pathMatch from './path-match' -import prepareDestination, { matchHas } from './prepare-destination' +import { matchHas, prepareDestination } from './prepare-destination' import { Rewrite } from '../../../../lib/load-custom-routes' import { removePathTrailingSlash } from '../../../../client/normalize-trailing-slash' import { normalizeLocalePath } from '../../i18n/normalize-locale-path' @@ -67,12 +67,12 @@ export default function resolveRewrites( // this is a proxied rewrite which isn't handled on the client return true } - const destRes = prepareDestination( - rewrite.destination, - params, - query, - true - ) + const destRes = prepareDestination({ + appendParamsToQuery: true, + destination: rewrite.destination, + params: params, + query: query, + }) parsedAs = destRes.parsedDestination asPath = destRes.newUrl Object.assign(query, destRes.parsedDestination.query) diff --git a/test/integration/middleware/with-i18n/next.config.js b/test/integration/middleware/with-i18n/next.config.js new file mode 100644 index 000000000000..75e2c5c2b603 --- /dev/null +++ b/test/integration/middleware/with-i18n/next.config.js @@ -0,0 +1,6 @@ +module.exports = { + i18n: { + locales: ['default', 'en', 'es', 'fr', 'zh'], + defaultLocale: 'default', + }, +} diff --git a/test/integration/middleware/with-i18n/pages/_middleware.js b/test/integration/middleware/with-i18n/pages/_middleware.js new file mode 100644 index 000000000000..407a751aa539 --- /dev/null +++ b/test/integration/middleware/with-i18n/pages/_middleware.js @@ -0,0 +1,20 @@ +import { NextResponse } from 'next/server' + +const PUBLIC_FILE = /\.(.*)$/ + +export function middleware(req) { + const locale = req.nextUrl.searchParams.get('my-locale') + const country = req.nextUrl.searchParams.get('country') || 'us' + + if (locale) { + req.nextUrl.locale = locale + } + + if ( + !PUBLIC_FILE.test(req.nextUrl.pathname) && + !req.nextUrl.pathname.includes('/api/') + ) { + req.nextUrl.pathname = `/test/${country}` + return NextResponse.rewrite(req.nextUrl) + } +} diff --git a/test/integration/middleware/with-i18n/pages/test/[country].js b/test/integration/middleware/with-i18n/pages/test/[country].js new file mode 100644 index 000000000000..dd09ad84a7e2 --- /dev/null +++ b/test/integration/middleware/with-i18n/pages/test/[country].js @@ -0,0 +1,22 @@ +export const getStaticPaths = async () => { + return { + fallback: 'blocking', + paths: [], + } +} + +export const getStaticProps = async ({ params: { country }, locale }) => { + return { + props: { country, locale }, + revalidate: false, + } +} + +export default function CountryPage({ locale, country }) { + return ( +
    +
  • {country}
    • +
  • {locale}
    • +
+ ) +} diff --git a/test/integration/middleware/with-i18n/test/index.test.js b/test/integration/middleware/with-i18n/test/index.test.js new file mode 100644 index 000000000000..658c2f147568 --- /dev/null +++ b/test/integration/middleware/with-i18n/test/index.test.js @@ -0,0 +1,64 @@ +/* eslint-env jest */ + +import { join } from 'path' +import cheerio from 'cheerio' +import { + fetchViaHTTP, + findPort, + killApp, + launchApp, + nextBuild, + nextStart, +} from 'next-test-utils' + +const context = {} +context.appDir = join(__dirname, '../') + +jest.setTimeout(1000 * 60 * 2) + +describe('Middleware i18n tests', () => { + describe('dev mode', () => { + beforeAll(async () => { + context.appPort = await findPort() + context.app = await launchApp(context.appDir, context.appPort) + }) + + afterAll(() => killApp(context.app)) + runTests() + }) + + describe('production mode', () => { + beforeAll(async () => { + await nextBuild(context.appDir, undefined, { + stderr: true, + stdout: true, + }) + + context.appPort = await findPort() + context.app = await nextStart(context.appDir, context.appPort) + }) + + afterAll(() => killApp(context.app)) + runTests() + }) +}) + +function runTests() { + it(`reads the locale from the pathname`, async () => { + const res = await fetchViaHTTP(context.appPort, '/fr', { country: 'spain' }) + + const html = await res.text() + const $ = cheerio.load(html) + expect($('#locale').text()).toBe('fr') + expect($('#country').text()).toBe('spain') + }) + + it(`rewrites from a locale correctly`, async () => { + const res = await fetchViaHTTP(context.appPort, '/', { 'my-locale': 'es' }) + + const html = await res.text() + const $ = cheerio.load(html) + expect($('#locale').text()).toBe('es') + expect($('#country').text()).toBe('us') + }) +}