diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a3fceee08..ce9bd1a608 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,8 +93,8 @@ given prefix, run `npm run pretest -- `. > Needs the `netlify-cli` installed and being logged in having access to Netlify Testing > Organization or providing your own site ID with NETLIFY_SITE_ID environment variable. -The e2e tests can be invoked with `npm run e2e` and perform a full e2e test. This means they do the -following: +The e2e tests can be invoked with `npm run test:e2e` and perform a full e2e test. This means they do +the following: 1. Building the next-runtime (just running `npm run build` in the repository) 2. Creating a temp directory and copying the provided fixture over to the directory. @@ -113,7 +113,7 @@ following: purposes. > [!TIP] If you'd like to always keep the deployment and the local fixture around for -> troubleshooting, run `E2E_PERSIST=1 npm run e2e`. +> troubleshooting, run `E2E_PERSIST=1 npm run test:e2e`. ### Next.js tests diff --git a/edge-runtime/lib/next-request.ts b/edge-runtime/lib/next-request.ts index e6a1bb95f8..cf5d12c56e 100644 --- a/edge-runtime/lib/next-request.ts +++ b/edge-runtime/lib/next-request.ts @@ -2,103 +2,89 @@ import type { Context } from '@netlify/edge-functions' import { addBasePath, - addTrailingSlash, + addLocale, normalizeDataUrl, normalizeLocalePath, + normalizeTrailingSlash, removeBasePath, } from './util.ts' -interface I18NConfig { - defaultLocale: string - localeDetection?: false - locales: string[] -} +import type { NextConfig } from 'next/dist/server/config-shared' +import type { NextRequest, RequestInit } from 'next/dist/server/web/spec-extension/request.js' -export interface RequestData { - geo?: { - city?: string - country?: string - region?: string - latitude?: string - longitude?: string - timezone?: string - } - headers: Record - ip?: string - method: string - nextConfig?: { - basePath?: string - i18n?: I18NConfig | null - trailingSlash?: boolean - skipMiddlewareUrlNormalize?: boolean - } - page?: { - name?: string - params?: { [key: string]: string } +export type NetlifyNextRequest = Partial> & + RequestInit & { + headers: HeadersInit } - url: string - body?: ReadableStream - detectedLocale?: string -} -const normalizeRequestURL = ( - originalURL: string, - nextConfig?: RequestData['nextConfig'], -): { url: string; detectedLocale?: string } => { - const url = new URL(originalURL) +const normalizeRequest = (url: URL, nextConfig?: NextConfig): URL => { + url.pathname = removeBasePath(url.pathname, nextConfig?.basePath) - let pathname = removeBasePath(url.pathname, nextConfig?.basePath) + // We want to run middleware for data requests and expose the URL of the + // corresponding pages, so we have to normalize the URLs before running + // the handler. + url.pathname = normalizeDataUrl(url.pathname) - // If it exists, remove the locale from the URL and store it - const { detectedLocale } = normalizeLocalePath(pathname, nextConfig?.i18n?.locales) + // Normalizing the trailing slash based on the `trailingSlash` configuration + // property from the Next.js config. + url.pathname = normalizeTrailingSlash(url.pathname, nextConfig?.trailingSlash) - if (!nextConfig?.skipMiddlewareUrlNormalize) { - // We want to run middleware for data requests and expose the URL of the - // corresponding pages, so we have to normalize the URLs before running - // the handler. - pathname = normalizeDataUrl(pathname) + url.pathname = addBasePath(url.pathname, nextConfig?.basePath) - // Normalizing the trailing slash based on the `trailingSlash` configuration - // property from the Next.js config. - if (nextConfig?.trailingSlash) { - pathname = addTrailingSlash(pathname) - } - } + return url +} - url.pathname = addBasePath(pathname, nextConfig?.basePath) +export const localizeRequest = ( + url: URL, + nextConfig?: NextConfig, +): { localizedUrl: URL; locale?: string } => { + url.pathname = removeBasePath(url.pathname, nextConfig?.basePath) + + // Detect the locale from the URL + const { detectedLocale } = normalizeLocalePath(url.pathname, nextConfig?.i18n?.locales) + + // Add the locale to the URL if not already present + url.pathname = addLocale(url.pathname, detectedLocale ?? nextConfig?.i18n?.defaultLocale) + + url.pathname = addBasePath(url.pathname, nextConfig?.basePath) return { - url: url.toString(), - detectedLocale, + localizedUrl: url, + locale: detectedLocale, } } export const buildNextRequest = ( request: Request, context: Context, - nextConfig?: RequestData['nextConfig'], -): RequestData => { - const { url, method, body, headers } = request - const { country, subdivision, city, latitude, longitude, timezone } = context.geo - const geo: RequestData['geo'] = { - city, - country: country?.code, - region: subdivision?.code, - latitude: latitude?.toString(), - longitude: longitude?.toString(), - timezone, - } + nextConfig?: NextConfig, +): NetlifyNextRequest => { + const { method, body, headers } = request + const { country, subdivision, city, latitude, longitude } = context.geo + const { i18n, basePath, trailingSlash } = nextConfig ?? {} - const { detectedLocale, url: normalizedUrl } = normalizeRequestURL(url, nextConfig) + const url = new URL(request.url) + const normalizedUrl = nextConfig?.skipMiddlewareUrlNormalize + ? url + : normalizeRequest(url, nextConfig) return { + url: normalizedUrl.toString(), headers: Object.fromEntries(headers.entries()), - geo, - url: normalizedUrl, - method, + geo: { + city, + country: country?.code, + region: subdivision?.code, + latitude: latitude?.toString(), + longitude: longitude?.toString(), + }, ip: context.ip, - body: body ?? undefined, - nextConfig, - detectedLocale, + method, + body, + nextConfig: { + i18n, + basePath, + trailingSlash, + }, } } diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts index 6e2366354b..daea42905e 100644 --- a/edge-runtime/lib/response.ts +++ b/edge-runtime/lib/response.ts @@ -4,14 +4,8 @@ import { HTMLRewriter } from '../vendor/deno.land/x/html_rewriter@v0.1.0-pre.17/ import { updateModifiedHeaders } from './headers.ts' import type { StructuredLogger } from './logging.ts' import { addMiddlewareHeaders, isMiddlewareRequest, isMiddlewareResponse } from './middleware.ts' -import { RequestData } from './next-request.ts' -import { - addBasePath, - normalizeDataUrl, - normalizeLocalePath, - normalizeTrailingSlash, - relativizeURL, -} from './util.ts' +import { NetlifyNextRequest } from './next-request.ts' +import { normalizeDataUrl, normalizeTrailingSlash, relativizeURL } from './util.ts' export interface FetchEventResult { response: Response @@ -20,20 +14,20 @@ export interface FetchEventResult { interface BuildResponseOptions { context: Context - logger: StructuredLogger + nextConfig?: NetlifyNextRequest['nextConfig'] + locale?: string request: Request result: FetchEventResult - nextConfig?: RequestData['nextConfig'] - requestLocale?: string + logger: StructuredLogger } export const buildResponse = async ({ context, - logger, + nextConfig, + locale, request, result, - nextConfig, - requestLocale, + logger, }: BuildResponseOptions): Promise => { logger .withFields({ is_nextresponse_next: result.response.headers.has('x-middleware-next') }) @@ -187,26 +181,12 @@ export const buildResponse = async ({ // respect trailing slash rules to prevent 308s rewriteUrl.pathname = normalizeTrailingSlash(rewriteUrl.pathname, nextConfig?.trailingSlash) - const target = normalizeLocalizedTarget({ target: rewriteUrl.toString(), request, nextConfig }) - if (target === request.url) { - logger.withFields({ rewrite_url: rewrite }).debug('Rewrite url is same as original url') - return - } + const target = rewriteUrl.toString() res.headers.set('x-middleware-rewrite', relativeUrl) request.headers.set('x-middleware-rewrite', target) return addMiddlewareHeaders(context.rewrite(target), res) } - // If we are redirecting a request that had a locale in the URL, we need to add it back in - if (redirect && requestLocale) { - redirect = normalizeLocalizedTarget({ target: redirect, request, nextConfig, requestLocale }) - if (redirect === request.url) { - logger.withFields({ rewrite_url: rewrite }).debug('Rewrite url is same as original url') - return - } - res.headers.set('location', redirect) - } - // Data requests shouldn't automatically redirect in the browser (they might be HTML pages): they're handled by the router if (redirect && isDataReq) { res.headers.delete('location') @@ -226,35 +206,3 @@ export const buildResponse = async ({ return res } - -/** - * Normalizes the locale in a URL. - */ -function normalizeLocalizedTarget({ - target, - request, - nextConfig, - requestLocale, -}: { - target: string - request: Request - nextConfig?: RequestData['nextConfig'] - requestLocale?: string -}) { - const targetUrl = new URL(target, request.url) - - const normalizedTarget = normalizeLocalePath(targetUrl.pathname, nextConfig?.i18n?.locales) - - const locale = normalizedTarget.detectedLocale ?? requestLocale - if ( - locale && - !normalizedTarget.pathname.startsWith(`/api/`) && - !normalizedTarget.pathname.startsWith(`/_next/static/`) - ) { - targetUrl.pathname = - addBasePath(`/${locale}${normalizedTarget.pathname}`, nextConfig?.basePath) || `/` - } else { - targetUrl.pathname = addBasePath(normalizedTarget.pathname, nextConfig?.basePath) || `/` - } - return targetUrl.toString() -} diff --git a/edge-runtime/lib/routing.ts b/edge-runtime/lib/routing.ts index 4619fda369..10cd261e41 100644 --- a/edge-runtime/lib/routing.ts +++ b/edge-runtime/lib/routing.ts @@ -7,8 +7,8 @@ import type { Key } from '../vendor/deno.land/x/path_to_regexp@v6.2.1/index.ts' -import { compile, pathToRegexp } from '../vendor/deno.land/x/path_to_regexp@v6.2.1/index.ts' import { getCookies } from '../vendor/deno.land/std@0.175.0/http/cookie.ts' +import { compile, pathToRegexp } from '../vendor/deno.land/x/path_to_regexp@v6.2.1/index.ts' /* ┌─────────────────────────────────────────────────────────────────────────┐ diff --git a/edge-runtime/lib/util.ts b/edge-runtime/lib/util.ts index 2bc11cd2e8..28fea98eaa 100644 --- a/edge-runtime/lib/util.ts +++ b/edge-runtime/lib/util.ts @@ -29,6 +29,20 @@ export const addBasePath = (path: string, basePath?: string) => { return path } +// add locale prefix if not present, allowing for locale fallbacks +export const addLocale = (path: string, locale?: string) => { + if ( + locale && + path.toLowerCase() !== `/${locale.toLowerCase()}` && + !path.toLowerCase().startsWith(`/${locale.toLowerCase()}/`) && + !path.startsWith(`/api/`) && + !path.startsWith(`/_next/static/`) + ) { + return `/${locale}${path}` + } + return path +} + // https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/i18n/normalize-locale-path.ts export interface PathLocale { diff --git a/edge-runtime/middleware.ts b/edge-runtime/middleware.ts index f0170b912d..96dbc8d9fa 100644 --- a/edge-runtime/middleware.ts +++ b/edge-runtime/middleware.ts @@ -5,7 +5,7 @@ import nextConfig from './next.config.json' with { type: 'json' } import { InternalHeaders } from './lib/headers.ts' import { logger, LogLevel } from './lib/logging.ts' -import { buildNextRequest, RequestData } from './lib/next-request.ts' +import { buildNextRequest, localizeRequest, NetlifyNextRequest } from './lib/next-request.ts' import { buildResponse, FetchEventResult } from './lib/response.ts' import { getMiddlewareRouteMatcher, @@ -13,7 +13,7 @@ import { type MiddlewareRouteMatch, } from './lib/routing.ts' -type NextHandler = (params: { request: RequestData }) => Promise +type NextHandler = (params: { request: NetlifyNextRequest }) => Promise const matchesMiddleware: MiddlewareRouteMatch = getMiddlewareRouteMatcher(matchers || []) @@ -31,8 +31,9 @@ export async function handleMiddleware( context: Context, nextHandler: NextHandler, ) { - const nextRequest = buildNextRequest(request, context, nextConfig) const url = new URL(request.url) + const query = searchParamsToUrlQuery(url.searchParams) + const { localizedUrl, locale } = localizeRequest(url, nextConfig) const reqLogger = logger .withLogLevel( request.headers.has(InternalHeaders.NFDebugLogging) ? LogLevel.Debug : LogLevel.Log, @@ -40,13 +41,17 @@ export async function handleMiddleware( .withFields({ url_path: url.pathname }) .withRequestID(request.headers.get(InternalHeaders.NFRequestID)) + // Convert the incoming request to a Next.js request, which includes + // normalizing the URL, adding geo and IP information and converting + // the headers to a plain object, among other things. + const nextRequest = buildNextRequest(request, context, nextConfig) + // While we have already checked the path when mapping to the edge function, // Next.js supports extra rules that we need to check here too, because we // might be running an edge function for a path we should not. If we find // that's the case, short-circuit the execution. - if (!matchesMiddleware(url.pathname, request, searchParamsToUrlQuery(url.searchParams))) { + if (!matchesMiddleware(localizedUrl.pathname, request, query)) { reqLogger.debug('Aborting middleware due to runtime rules') - return } @@ -54,11 +59,11 @@ export async function handleMiddleware( const result = await nextHandler({ request: nextRequest }) const response = await buildResponse({ context, - logger: reqLogger, + nextConfig: nextRequest.nextConfig, + locale, request, result, - requestLocale: nextRequest.detectedLocale, - nextConfig, + logger: reqLogger, }) return response diff --git a/src/build/functions/edge.ts b/src/build/functions/edge.ts index 77c79bf2e3..4c95ad1353 100644 --- a/src/build/functions/edge.ts +++ b/src/build/functions/edge.ts @@ -65,10 +65,7 @@ const writeHandlerFile = async (ctx: PluginContext, { matchers, name }: NextDefi // Writing a file with the matchers that should trigger this function. We'll // read this file from the function at runtime. - await writeFile( - join(handlerRuntimeDirectory, 'matchers.json'), - JSON.stringify(augmentMatchers(matchers, ctx)), - ) + await writeFile(join(handlerRuntimeDirectory, 'matchers.json'), JSON.stringify(matchers)) // The config is needed by the edge function to match and normalize URLs. To // avoid shipping and parsing a large file at runtime, let's strip it down to diff --git a/tests/fixtures/middleware-conditions/middleware.ts b/tests/fixtures/middleware-conditions/middleware.ts index d5a3c51045..fdb332cf8e 100644 --- a/tests/fixtures/middleware-conditions/middleware.ts +++ b/tests/fixtures/middleware-conditions/middleware.ts @@ -19,7 +19,7 @@ export const config = { source: '/hello', }, { - source: '/nl-NL/about', + source: '/nl/about', locale: false, }, ], diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts index 9aca477dfb..9ca3a633aa 100644 --- a/tests/integration/edge-handler.test.ts +++ b/tests/integration/edge-handler.test.ts @@ -261,18 +261,21 @@ describe("aborts middleware execution when the matcher conditions don't match th ctx.cleanup?.push(() => origin.stop()) - for (const path of ['/hello', '/en/hello', '/nl-NL/hello', '/nl-NL/about']) { + for (const path of ['/hello', '/en/hello', '/es/hello', '/nl/about']) { const response = await invokeEdgeFunction(ctx, { functions: ['___netlify-edge-handler-middleware'], origin, url: path, }) - expect(response.headers.has('x-hello-from-middleware-res'), `does match ${path}`).toBeTruthy() + expect( + response.headers.has('x-hello-from-middleware-res'), + `should match ${path}`, + ).toBeTruthy() expect(await response.text()).toBe('Hello from origin!') expect(response.status).toBe(200) } - for (const path of ['/hello/invalid', '/about', '/en/about']) { + for (const path of ['/hello/invalid', '/invalid/hello', '/about', '/en/about', '/es/about']) { const response = await invokeEdgeFunction(ctx, { functions: ['___netlify-edge-handler-middleware'], origin, @@ -280,7 +283,7 @@ describe("aborts middleware execution when the matcher conditions don't match th }) expect( response.headers.has('x-hello-from-middleware-res'), - `does not match ${path}`, + `should not match ${path}`, ).toBeFalsy() expect(await response.text()).toBe('Hello from origin!') expect(response.status).toBe(200)