fix(middleware): align encoded path matching

[james-elicx]

Summary

• evaluate middleware matchers against the encoded pathname before a single decoded fallback
• preserve Next.js basePath and i18n locale ordering, including domain locale selection
• cover App Router pages, route handlers, RSC requests, and Pages Router paths across supported runtimes

Validation

• vp test run tests/middleware-runtime.test.ts tests/middleware-runtime-trailing-slash.test.ts tests/pages-data-route.test.ts tests/pages-data-url.test.ts
• vp test run tests/middleware-matcher-auth.test.ts
• vp test run tests/shims.test.ts -t matchesMiddleware
• vp check packages/vinext/src/server/middleware-runtime.ts packages/vinext/src/server/middleware-matcher.ts tests/middleware-runtime.test.ts tests/shims.test.ts

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2802

npm i https://pkg.pr.new/create-vinext-app@2802

npm i https://pkg.pr.new/@vinext/types@2802

npm i https://pkg.pr.new/vinext@2802

commit: d337861

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared d337861 against base dfc979c using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.6 KB	134.6 KB	⚫ 0.0%
Client entry size (gzip)	vinext	122.2 KB	122.2 KB	⚫ +0.0%
Dev server cold start	vinext	2.90 s	2.86 s	⚫ -1.1%
Production build time	vinext	3.12 s	3.15 s	⚫ +0.9%
RSC entry closure size (gzip)	vinext	113.8 KB	113.8 KB	⚫ +0.1%
Server bundle size (gzip)	vinext	191.3 KB	191.4 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(middleware): align encoded path matching

I reviewed the diff, traced the new matcher logic through both entry points (server/prod-server.ts and server/app-middleware.ts), and re-ran the middleware suites locally. All 128 tests in middleware-runtime, middleware-runtime-trailing-slash, and middleware-matcher-auth pass, plus shims (matcher patterns), pages-data-*, and the App Router middleware integration tests (42). vp check is clean on both modified source files.

This is a solid, well-tested correctness fix. The core changes are sound:

Correct

• Evaluating the encoded pathname first, then a single full decode fallback, matches the Next.js next-server behavior and the segment-decode approach that could turn %0A/%2F into structurally different characters. The line-terminator (%0A, %0D, %E2%80%A8, %E2%80%A9) and delimiter (%2F, %3F, %23) cases are directly ported and exercised.
• Dropping the removeTrailingSlash(pattern) normalization in matchPattern is the right call. The compiled regex already appends an optional terminal [/#?]?, so /api/admin/ (literal trailing slash required) correctly no longer matches /api/admin. The updated shims.test.ts and middleware-runtime-trailing-slash.test.ts expectations reflect real path-to-regexp semantics rather than the previous artificial widening.
• The MiddlewareLocaleMatchContext (literal/internal/defaulted) cleanly encodes locale provenance so it is computed once and shared across all matchers in an array — the case-insensitive literal-locale detection stays consistent with stripLocalePrefix, and locale: false correctly bypasses locale handling.
• Both entry points delegate to executeMiddleware, so dev/prod/App Router/RSC parity is preserved. /_next/static and /_next/image returning false under i18n matches Next.js (and /_next/data is normalized to the page path before this code runs, so it is unaffected).

Points worth confirming (not blockers)

1. Matcher now reads the raw request URL, not the caller-provided normalizedPathname. The new logic derives encodedRequestPathname from new URL(options.request.url).pathname and does its own basePath handling via prepareMatcherPathname, whereas the old code matched against basePathStrippedPathname (derived from normalizedPathname). For App Router this shifts the source of truth from cleanPathname to the original request URL. The hadBasePath === true branch and the App Router integration tests cover this, but it is the highest-risk behavioral shift in the PR — worth a second look to confirm no caller relied on a pre-normalized normalizedPathname diverging from the raw URL (e.g. a caller that rewrote the pathname before invoking middleware). matchPathname/basePathStrippedPathname is now used only for the NextFetchEvent page, no longer for eligibility.

2. matchPattern root special-case asymmetry. The guard if (pattern === "/" && (pathname === "//" || pathname === "/?" || pathname === "/#")) return false; blocks a bare terminal delimiter only for the root pattern, while /api/admin/? intentionally still matches /api/admin/. This is deliberate and test-backed (matchPattern("/?", "/") -> false vs matchPattern("/api/admin/?", "/api/admin/") -> true), but a one-line comment explaining why root is special would help future readers. Real requests never reach matchPattern with ////? (normalizePath collapses them), so this only affects the direct unit-test surface — fine.

3. prepareMatcherPathname endsWith("?")/endsWith("#") branch. This only triggers when a decoded candidate (decodeURIComponent) produces <basePath>?/<basePath>#; URL.pathname never contains those raw. The intent is subtle — the existing comment covers the encoded-basePath rationale, but explicitly noting these come from the decode fallback would aid readability.

Overall: correct, matches Next.js semantics, comprehensively tested, and preserves multi-runtime parity. Approving with the note on point #1 as the main thing to double-check.

[ask-bonk]

Review posted to PR #2802.

Summary

I reviewed PR #2802 (fix(middleware): align encoded path matching) by tracing the new matcher logic through both entry points and re-running the middleware test suites locally (128 middleware tests + shims + pages-data + 42 App Router integration tests all pass; vp check clean).

Verdict: solid, well-tested correctness fix. The core changes are sound:

• Encoded-first, single-decode-fallback matching matches Next.js next-server behavior and avoids segment-decode structural corruption (line terminators, encoded slashes).
• Removing removeTrailingSlash(pattern) in matchPattern is correct — the compiled regex already provides an optional terminal [/#?]?, so /api/admin/ no longer over-matches /api/admin. Test expectations were updated to reflect real path-to-regexp semantics.
• MiddlewareLocaleMatchContext cleanly encodes locale provenance once, shared across array matchers, and is consistent with stripLocalePrefix.
• Parity preserved — both Pages and App Router delegate to executeMiddleware; /_next/static//_next/image returning false under i18n matches Next.js.

Non-blocking points flagged for the author:

1. The matcher now reads the raw request URL (new URL(options.request.url).pathname) rather than the caller-provided normalizedPathname — the highest-risk behavioral shift, worth confirming no caller relied on a pre-normalized path diverging from the raw URL.
2. matchPattern root-pattern special-case asymmetry (deliberate + tested, but a one-line comment would help).
3. prepareMatcherPathname's endsWith("?")/endsWith("#") branch only fires on the decode fallback — a clarifying comment would aid readability.

No correctness defects or regressions found.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(middleware): align encoded path matching

Re-reviewed the current head (d337861). I traced both entry points, re-ran the middleware suites (middleware-runtime, middleware-matcher-auth, middleware-runtime-trailing-slash, shims matcher patterns, pages-router encoded — all green), and confirmed vp check is clean on both source files. I verified the two highest-risk claims directly against Next.js canary source rather than trusting the comments:

Confirmed correct against Next.js source

1. Case-insensitive locale detection. stripLocalePrefix and the runtime literal-locale detection now lowercase both sides. This matches normalizeLocalePath in packages/next/src/shared/lib/i18n/normalize-locale-path.ts, which lowercases the segment and the locale list before comparing and returns the case-sensitive locale. The /EN/foo / /Fr/foo → true and /FR/about shim cases are correct. Note the runtime's own case-insensitive hasLiteralLocale check (middleware-runtime.ts:403-407) correctly pre-empts the still-case-sensitive normalizeDefaultLocalePathname (pages-i18n.ts:92, locales.includes(parts[1])), so uppercase locales never fall into the internal/defaulted classification path — no divergence there, but that case-sensitivity gap in normalizeDefaultLocalePathname is worth a follow-up since it is relied on indirectly.

2. Trailing-slash source semantics. Dropping removeTrailingSlash(pattern) in matchPattern is correct: middlewarePathTokensToRegExp (middleware-path-to-regexp.ts:279) appends exactly [/#?]?$, so matcher: "/api/admin/" compiles to ^/api/admin/[/#?]?$ and correctly no longer matches /api/admin. The updated shims.test.ts expectations (matchesMiddleware("/api/admin", "/api/admin/") → false) reflect real path-to-regexp behavior, not the previous artificial widening. This is a deliberate behavioral change — any user whose matcher source carries a trailing slash while their requests don't will now see middleware stop firing. It matches Next.js, so it's the right call, but it's the kind of change that deserves a changelog/release note callout.

Other checks

• Encoded-first + single full-decodeURIComponent fallback matches next-server; the line-terminator (%0A/%0D/%E2%80%A8/%E2%80%A9) and delimiter (%2F/%3F/%23) ports are exercised, and %5C/%252F correctly do not overmatch.
• MiddlewareLocaleMatchContext is computed once and shared across array matchers; internal → string matchers return false while locale:false matchers still match internal paths (test-backed, matches Next.js).
• matchObjectMatcher defaulted branch inserts the domain-aware default locale for locale:false sources — the fr.example.com domain test confirms domain-locale selection.
• New optional localeContext param keeps matchesMiddleware/matchPattern backward-compatible for the only external caller (server/middleware.ts re-export).

One thing to double-check (non-blocking, same as prior review's point #1)

Eligibility now derives encodedRequestPathname from new URL(options.request.url).pathname (the raw request URL) and does its own basePath handling, while normalizedPathname/basePathStrippedPathname are now used only to build NextFetchEvent.page (middleware-runtime.ts:466). In app-middleware.ts the caller passes normalizedPathname: cleanPathname, and cleanPathname can be a rewritten pathname (app-middleware.ts:293) on the forwarded-context second pass. So on that path the eligibility check and the NextFetchEvent.page are now sourced from different values (raw URL vs rewritten clean path). The App Router integration tests pass, so this appears intended, but it's worth a sentence in the PR description confirming the raw-URL source of truth is desired for the forwarded-rewrite re-entry case.

Verdict: Correct, matches Next.js semantics, comprehensively tested, preserves multi-runtime parity. No correctness defects found. Approving with the trailing-slash behavior-change note (worth a release-note callout) and the forwarded-rewrite source-of-truth confirmation.

[ask-bonk]

Review posted to PR #2802.

Summary

I re-reviewed PR #2802 (fix(middleware): align encoded path matching) at head d337861, traced both entry points, and re-ran the middleware suites (all green) with vp check clean. Unlike the prior review, I verified the two highest-risk claims directly against Next.js canary source:

Verified correct against Next.js source:

1. Case-insensitive locale detection — matches normalizeLocalePath (lowercases both sides, returns case-sensitive locale). Also confirmed the runtime's own case-insensitive hasLiteralLocale check pre-empts a still-case-sensitive normalizeDefaultLocalePathname (pages-i18n.ts:92), so no divergence in practice — but that gap is worth a follow-up.
2. Trailing-slash semantics — dropping removeTrailingSlash(pattern) is correct because middlewarePathTokensToRegExp appends exactly [/#?]?$. Flagged that matcher: "/api/admin/" no longer matching /api/admin is a deliberate behavioral change deserving a release-note callout.

Non-blocking items flagged:

• The forwarded-rewrite re-entry path in app-middleware.ts now sources eligibility (raw request URL) and NextFetchEvent.page (rewritten cleanPathname) from different values — appears intended, worth confirming in the PR description.
• The normalizeDefaultLocalePathname case-sensitivity gap as a future follow-up.

No correctness defects or regressions found. Approving with notes.

github run