From f02ef880f27bdc28f9e699757e8df9d2a1203438 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 29 Jul 2024 11:57:41 +0200 Subject: [PATCH] fix: edge-middleware i18n matching (#2555) * fix: edge-middleware i18n matching * test: update edge-runtime test * test: do not localize data requests --------- Co-authored-by: Rob Stanford --- edge-runtime/lib/next-request.ts | 28 +++++++++++++++++++ edge-runtime/lib/util.ts | 14 ++++++++++ edge-runtime/middleware.ts | 10 +++++-- src/build/functions/edge.ts | 5 +--- .../middleware-conditions/middleware.ts | 2 +- tests/integration/edge-handler.test.ts | 11 +++++--- 6 files changed, 58 insertions(+), 12 deletions(-) diff --git a/edge-runtime/lib/next-request.ts b/edge-runtime/lib/next-request.ts index e6a1bb95f8..e8e1624c72 100644 --- a/edge-runtime/lib/next-request.ts +++ b/edge-runtime/lib/next-request.ts @@ -2,6 +2,7 @@ import type { Context } from '@netlify/edge-functions' import { addBasePath, + addLocale, addTrailingSlash, normalizeDataUrl, normalizeLocalePath, @@ -73,6 +74,33 @@ const normalizeRequestURL = ( } } +export const localizeRequest = ( + url: URL, + nextConfig?: { + basePath?: string + i18n?: I18NConfig | null + }, +): { localizedUrl: URL; locale?: string } => { + const localizedUrl = new URL(url) + localizedUrl.pathname = removeBasePath(localizedUrl.pathname, nextConfig?.basePath) + + // Detect the locale from the URL + const { detectedLocale } = normalizeLocalePath(localizedUrl.pathname, nextConfig?.i18n?.locales) + + // Add the locale to the URL if not already present + localizedUrl.pathname = addLocale( + localizedUrl.pathname, + detectedLocale ?? nextConfig?.i18n?.defaultLocale, + ) + + localizedUrl.pathname = addBasePath(localizedUrl.pathname, nextConfig?.basePath) + + return { + localizedUrl, + locale: detectedLocale, + } +} + export const buildNextRequest = ( request: Request, context: Context, diff --git a/edge-runtime/lib/util.ts b/edge-runtime/lib/util.ts index 2bc11cd2e8..26677b47d1 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/`) + ) { + 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..73a37a6007 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, RequestData } from './lib/next-request.ts' import { buildResponse, FetchEventResult } from './lib/response.ts' import { getMiddlewareRouteMatcher, @@ -31,8 +31,8 @@ export async function handleMiddleware( context: Context, nextHandler: NextHandler, ) { - const nextRequest = buildNextRequest(request, context, nextConfig) const url = new URL(request.url) + const reqLogger = logger .withLogLevel( request.headers.has(InternalHeaders.NFDebugLogging) ? LogLevel.Debug : LogLevel.Log, @@ -40,16 +40,20 @@ export async function handleMiddleware( .withFields({ url_path: url.pathname }) .withRequestID(request.headers.get(InternalHeaders.NFRequestID)) + const { localizedUrl } = localizeRequest(url, 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, searchParamsToUrlQuery(url.searchParams)) + ) { reqLogger.debug('Aborting middleware due to runtime rules') return } + const nextRequest = buildNextRequest(request, context, nextConfig) try { const result = await nextHandler({ request: nextRequest }) const response = await buildResponse({ 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..78949d2d6c 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', '/nl/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 ['/invalid/hello', '/hello/invalid', '/about', '/en/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)