fix: narrow down middleware i18n locale matcher to concrete locales (#2768)

* test: add test cases to i18n middleware with exclusions * fix: narrow down middleware i18n locale matcher to concrete locales

Author: pieh

src/build/functions/edge.ts

@@ -36,13 +36,23 @@ const augmentMatchers = (
  matchers: NextDefinition['matchers'],
  ctx: PluginContext,
 ): NextDefinition['matchers'] => {
- if (!ctx.buildConfig.i18n) {
+ const i18NConfig = ctx.buildConfig.i18n
+ if (!i18NConfig) {
  return matchers
  }
  return matchers.flatMap((matcher) => {
  if (matcher.originalSource && matcher.locale !== false) {
  return [
- matcher,
+ matcher.regexp
+ ? {
+ ...matcher,
+ // https://github.com/vercel/next.js/blob/5e236c9909a768dc93856fdfad53d4f4adc2db99/packages/next/src/build/analysis/get-page-static-info.ts#L332-L336
+ // Next is producing pretty broad matcher for i18n locale. Presumably rest of their infrastructure protects this broad matcher
+ // from matching on non-locale paths. For us this becomes request entry point, so we need to narrow it down to just defined locales
+ // otherwise users might get unexpected matches on paths like `/api*`
+ regexp: matcher.regexp.replace(/\[\^\/\.]+/g, `(${i18NConfig.locales.join('|')})`),
+ }
+ : matcher,
  {
  ...matcher,
  regexp: pathToRegexp(matcher.originalSource).source,

tests/e2e/edge-middleware.test.ts

@@ -69,3 +69,150 @@ test('json data rewrite works', async ({ middlewarePages }) => {
 
  expect(data.pageProps.message).toBeDefined()
 })
+
+// those tests use `fetch` instead of `page.goto` intentionally to avoid potential client rendering
+// hiding any potential edge/server issues
+test.describe('Middleware with i18n and excluded paths', () => {
+ const DEFAULT_LOCALE = 'en'
+
+ /** helper function to extract JSON data from page rendering data with `<pre>{JSON.stringify(data)}</pre>` */
+ function extractDataFromHtml(html: string): Record<string, any> {
+ const match = html.match(/<pre>(?<rawInput>[^<]+)<\/pre>/)
+ if (!match || !match.groups?.rawInput) {
+ console.error('<pre> not found in html input', {
+ html,
+ })
+ throw new Error('Failed to extract data from HTML')
+ }
+
+ const { rawInput } = match.groups
+ const unescapedInput = rawInput.replaceAll('&quot;', '"')
+ try {
+ return JSON.parse(unescapedInput)
+ } catch (originalError) {
+ console.error('Failed to parse JSON', {
+ originalError,
+ rawInput,
+ unescapedInput,
+ })
+ }
+ throw new Error('Failed to extract data from HTML')
+ }
+
+ // those tests hit paths ending with `/json` which has special handling in middleware
+ // to return JSON response from middleware itself
+ test.describe('Middleware response path', () => {
+ test('should match on non-localized not excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/json`)
+
+ expect(response.headers.get('x-test-used-middleware')).toBe('true')
+ expect(response.status).toBe(200)
+
+ const { nextUrlPathname, nextUrlLocale } = await response.json()
+
+ expect(nextUrlPathname).toBe('/json')
+ expect(nextUrlLocale).toBe(DEFAULT_LOCALE)
+ })
+
+ test('should match on localized not excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/json`)
+
+ expect(response.headers.get('x-test-used-middleware')).toBe('true')
+ expect(response.status).toBe(200)
+
+ const { nextUrlPathname, nextUrlLocale } = await response.json()
+
+ expect(nextUrlPathname).toBe('/json')
+ expect(nextUrlLocale).toBe('fr')
+ })
+ })
+
+ // those tests hit paths that don't end with `/json` while still satisfying middleware matcher
+ // so middleware should pass them through to origin
+ test.describe('Middleware passthrough', () => {
+ test('should match on non-localized not excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/html`)
+
+ expect(response.headers.get('x-test-used-middleware')).toBe('true')
+ expect(response.status).toBe(200)
+ expect(response.headers.get('content-type')).toMatch(/text\/html/)
+
+ const html = await response.text()
+ const { locale, params } = extractDataFromHtml(html)
+
+ expect(params).toMatchObject({ catchall: ['html'] })
+ expect(locale).toBe(DEFAULT_LOCALE)
+ })
+
+ test('should match on localized not excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/html`)
+
+ expect(response.headers.get('x-test-used-middleware')).toBe('true')
+ expect(response.status).toBe(200)
+ expect(response.headers.get('content-type')).toMatch(/text\/html/)
+
+ const html = await response.text()
+ const { locale, params } = extractDataFromHtml(html)
+
+ expect(params).toMatchObject({ catchall: ['html'] })
+ expect(locale).toBe('fr')
+ })
+ })
+
+ // those tests hit paths that don't satisfy middleware matcher, so should go directly to origin
+ // without going through middleware
+ test.describe('Middleware skipping (paths not satisfying middleware matcher)', () => {
+ test('should NOT match on non-localized excluded API path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/api/html`)
+
+ expect(response.headers.get('x-test-used-middleware')).not.toBe('true')
+ expect(response.status).toBe(200)
+
+ const { params } = await response.json()
+
+ expect(params).toMatchObject({ catchall: ['html'] })
+ })
+
+ test('should NOT match on non-localized excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/excluded`)
+
+ expect(response.headers.get('x-test-used-middleware')).not.toBe('true')
+ expect(response.status).toBe(200)
+ expect(response.headers.get('content-type')).toMatch(/text\/html/)
+
+ const html = await response.text()
+ const { locale, params } = extractDataFromHtml(html)
+
+ expect(params).toMatchObject({ catchall: ['excluded'] })
+ expect(locale).toBe(DEFAULT_LOCALE)
+ })
+
+ test('should NOT match on localized excluded page path', async ({
+ middlewareI18nExcludedPaths,
+ }) => {
+ const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/excluded`)
+
+ expect(response.headers.get('x-test-used-middleware')).not.toBe('true')
+ expect(response.status).toBe(200)
+ expect(response.headers.get('content-type')).toMatch(/text\/html/)
+
+ const html = await response.text()
+ const { locale, params } = extractDataFromHtml(html)
+
+ expect(params).toMatchObject({ catchall: ['excluded'] })
+ expect(locale).toBe('fr')
+ })
+ })
+})

tests/fixtures/middleware-i18n-excluded-paths/middleware.ts

@@ -0,0 +1,36 @@
+import { NextResponse } from 'next/server'
+import type { NextRequest } from 'next/server'
+
+export async function middleware(request: NextRequest) {
+ const url = request.nextUrl
+
+ // if path ends with /json we create response in middleware, otherwise we pass it through
+ // to next server to get page or api response from it
+ const response = url.pathname.includes('/json')
+ ? NextResponse.json({
+ requestUrlPathname: new URL(request.url).pathname,
+ nextUrlPathname: request.nextUrl.pathname,
+ nextUrlLocale: request.nextUrl.locale,
+ })
+ : NextResponse.next()
+
+ response.headers.set('x-test-used-middleware', 'true')
+
+ return response
+}
+
+// matcher copied from example in https://nextjs.org/docs/pages/building-your-application/routing/middleware#matcher
+// with `excluded` segment added to exclusion
+export const config = {
+ matcher: [
+ /*
+ * Match all request paths except for the ones starting with:
+ * - api (API routes)
+ * - excluded (for testing localized routes and not just API routes)
+ * - _next/static (static files)
+ * - _next/image (image optimization files)
+ * - favicon.ico, sitemap.xml, robots.txt (metadata files)
+ */
+ '/((?!api|excluded|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt).*)',
+ ],
+}

tests/fixtures/middleware-i18n-excluded-paths/next-env.d.ts

@@ -0,0 +1,5 @@
+/// <reference types="next" />
+/// <reference types="next/image-types/global" />
+
+// NOTE: This file should not be edited
+// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information.

tests/fixtures/middleware-i18n-excluded-paths/next.config.js

@@ -0,0 +1,10 @@
+module.exports = {
+ output: 'standalone',
+ eslint: {
+ ignoreDuringBuilds: true,
+ },
+ i18n: {
+ locales: ['en', 'fr'],
+ defaultLocale: 'en',
+ },
+}

tests/fixtures/middleware-i18n-excluded-paths/package.json

@@ -0,0 +1,20 @@
+{
+ "name": "middleware-i18n-excluded-paths",
+ "version": "0.1.0",
+ "private": true,
+ "scripts": {
+ "postinstall": "next build",
+ "dev": "next dev",
+ "build": "next build"
+ },
+ "dependencies": {
+ "next": "latest",
+ "react": "18.2.0",
+ "react-dom": "18.2.0"
+ },
+ "devDependencies": {
+ "@types/node": "^17.0.12",
+ "@types/react": "18.2.47",
+ "typescript": "^5.2.2"
+ }
+}

tests/fixtures/middleware-i18n-excluded-paths/pages/[[...catchall]].tsx

@@ -0,0 +1,21 @@
+import type { GetStaticPaths, GetStaticProps } from 'next'
+
+export default function CatchAll({ params, locale }) {
+ return <pre>{JSON.stringify({ params, locale }, null, 2)}</pre>
+}
+
+export const getStaticPaths: GetStaticPaths = () => {
+ return {
+ paths: [],
+ fallback: 'blocking',
+ }
+}
+
+export const getStaticProps: GetStaticProps = ({ params, locale }) => {
+ return {
+ props: {
+ params,
+ locale,
+ },
+ }
+}

tests/fixtures/middleware-i18n-excluded-paths/pages/api/[[...catchall]].ts

@@ -0,0 +1,11 @@
+import type { NextApiRequest, NextApiResponse } from 'next'
+
+type ResponseData = {
+ params: {
+ catchall?: string[]
+ }
+}
+
+export default function handler(req: NextApiRequest, res: NextApiResponse<ResponseData>) {
+ res.status(200).json({ params: req.query })
+}

tests/fixtures/middleware-i18n-excluded-paths/tsconfig.json

@@ -0,0 +1,19 @@
+{
+ "compilerOptions": {
+ "target": "ES2017",
+ "lib": ["dom", "dom.iterable", "esnext"],
+ "allowJs": true,
+ "skipLibCheck": true,
+ "strict": false,
+ "noEmit": true,
+ "incremental": true,
+ "module": "esnext",
+ "esModuleInterop": true,
+ "moduleResolution": "node",
+ "resolveJsonModule": true,
+ "isolatedModules": true,
+ "jsx": "preserve"
+ },
+ "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
+ "exclude": ["node_modules"]
+}

tests/prepare.mjs

@@ -23,6 +23,7 @@ const e2eOnlyFixtures = new Set([
  'after',
  'cli-before-regional-blobs-support',
  'dist-dir',
+ 'middleware-i18n-excluded-paths',
  // There is also a bug on Windows on Node.js 18.20.6, that cause build failures on this fixture
  // see https://github.com/opennextjs/opennextjs-netlify/actions/runs/13268839161/job/37043172448?pr=2749#step:12:78
  'middleware-og',