Skip to content

Commit

Permalink
fix: edge-middleware i18n matching (#2555)
Browse files Browse the repository at this point in the history
* fix: edge-middleware i18n matching

* test: update edge-runtime test

* test: do not localize data requests

---------

Co-authored-by: Rob Stanford <me@robstanford.com>
  • Loading branch information
pieh and orinokai authored Jul 29, 2024
1 parent 4ee9ef0 commit f02ef88
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 12 deletions.
28 changes: 28 additions & 0 deletions edge-runtime/lib/next-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Context } from '@netlify/edge-functions'

import {
addBasePath,
addLocale,
addTrailingSlash,
normalizeDataUrl,
normalizeLocalePath,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions edge-runtime/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 7 additions & 3 deletions edge-runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -31,25 +31,29 @@ 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,
)
.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({
Expand Down
5 changes: 1 addition & 4 deletions src/build/functions/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/middleware-conditions/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const config = {
source: '/hello',
},
{
source: '/nl-NL/about',
source: '/nl/about',
locale: false,
},
],
Expand Down
11 changes: 7 additions & 4 deletions tests/integration/edge-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,26 +261,29 @@ 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,
url: path,
})
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)
Expand Down

0 comments on commit f02ef88

Please sign in to comment.