fix: support ** wildcard in query parameters for route rules#4067
fix: support ** wildcard in query parameters for route rules#4067cernymatej wants to merge 1 commit intonitrojs:mainfrom
** wildcard in query parameters for route rules#4067Conversation
|
@cernymatej is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis change extends wildcard pattern handling in redirect and proxy rules by enabling "" substitution in non-trailing URL positions. The runtime logic now captures path segments and merges query parameters, while preset transformations apply additional string replacements to handle literal "" patterns across Netlify and Vercel configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/runtime/internal/route-rules.ts (2)
34-35: Remove inline explanatory comments in these runtime blocks.These comments narrate straightforward lines and can be dropped for consistency with repo style.
As per coding guidelines:
Do not add comments explaining what the line does unless prompted.Also applies to: 43-44, 47-48, 72-73, 81-82, 85-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/internal/route-rules.ts` around lines 34 - 35, Remove the inline explanatory comment lines in src/runtime/internal/route-rules.ts that narrate simple statements (e.g., the comment pair containing "Wildcard ** in a non-trailing position (e.g., "/target?param=**")" and "Use only pathname (not search) to avoid redirect loops" and the similar comment pairs at the other indicated locations). Locate these comment blocks near the route-rules runtime logic (around the code handling wildcard and pathname/search behavior) and delete the comment lines so the runtime blocks contain only the implementation code, leaving no explanatory comments as per the guideline.
33-50: Extract shared wildcard target resolution to avoid redirect/proxy drift.The non-trailing
**flow is duplicated in both branches. A small shared helper would keep behavior aligned and reduce future regressions.♻️ Suggested refactor sketch
+function resolveWildcardTarget( + target: string, + event: H3Event, + stripBase?: string +) { + let targetPath = event.url.pathname; + if (stripBase) { + targetPath = withoutBase(targetPath, stripBase); + } + targetPath = targetPath.replace(/^\//, ""); + if (!targetPath) { + return; + } + let resolved = target.replace("**", targetPath); + if (event.url.search) { + resolved = withQuery(resolved, Object.fromEntries(event.url.searchParams)); + } + return resolved; +}Also applies to: 71-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/internal/route-rules.ts` around lines 33 - 50, Duplicate logic that resolves non-trailing "**" targets (the block that computes targetPath, applies (m.options as any)._redirectStripBase via withoutBase, trims leading "/", skips redirect when empty, replaces "**" in target, and merges event.url.search via withQuery) should be extracted into a shared helper (e.g., resolveNonTrailingWildcardTarget(target, event, m)) and invoked from both branches where this flow appears (the block using target.includes("**") at the current location and the similar block around lines 71-88). Implement the helper to accept the original target string, the event (to read url.pathname and searchParams), and the route metadata/options (to read _redirectStripBase), perform the exact steps currently duplicated (strip base with withoutBase, remove leading slash, return null/undefined if empty to indicate skip, substitute "**" with computed path, and merge query via withQuery), and replace the duplicated blocks to call it and handle its null/undefined result identically to the existing early-return behavior.test/tests.ts (1)
316-325: Strengthen this regression with source-query passthrough assertions.The new runtime logic also merges incoming query params into the target; adding one assertion here would lock that behavior.
🧪 Suggested test extension
+ const wildcardQueryWithSourceQuery = await callHandler({ + url: "/rules/redirect/wildcard-query/FOO?extra=1", + }); + expect(wildcardQueryWithSourceQuery.headers.location).toBe("/target?param=FOO&extra=1"); + const wildcardQueryEmpty = await callHandler({ url: "/rules/redirect/wildcard-query/", }); expect(wildcardQueryEmpty.status).not.toBe(301); + expect(wildcardQueryEmpty.headers.location).toBeUndefined();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/tests.ts` around lines 316 - 325, Add assertions to strengthen the regression by verifying that source query parameters are merged into the redirect target: when calling callHandler for "/rules/redirect/wildcard-query/FOO" include a query string (e.g., "?src=bar") and assert that wildcardQuery.status is 301 and that wildcardQuery.headers.location contains both the target param from the rule (param=FOO) and the original query param (src=bar) in the resulting location; likewise, ensure the case for the empty path (wildcardQueryEmpty) still does not return 301 even when a source query is present. Use the existing variables callHandler, wildcardQuery, and wildcardQueryEmpty to locate where to add these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets/vercel/utils.ts`:
- Line 170: The generated Vercel route regex uses "(.*)" which allows empty
captures; change the logic that builds redirect/proxy Location strings (the
expressions using routeRules.redirect.to.replace("/**", "/$1").replace("**",
"$1") and the analogous proxy handling) to detect non-trailing "**" usages in
the destination and use "(.+)" instead of "(.*)" for those cases so the regex
requires at least one character; implement this by checking the destination for
non-trailing "**" and choosing replace patterns that insert "/$1" or "$1" based
on whether you need "/(.+)" vs "/(.*)" (and similarly for proxies) so runtime
parity is preserved.
---
Nitpick comments:
In `@src/runtime/internal/route-rules.ts`:
- Around line 34-35: Remove the inline explanatory comment lines in
src/runtime/internal/route-rules.ts that narrate simple statements (e.g., the
comment pair containing "Wildcard ** in a non-trailing position (e.g.,
"/target?param=**")" and "Use only pathname (not search) to avoid redirect
loops" and the similar comment pairs at the other indicated locations). Locate
these comment blocks near the route-rules runtime logic (around the code
handling wildcard and pathname/search behavior) and delete the comment lines so
the runtime blocks contain only the implementation code, leaving no explanatory
comments as per the guideline.
- Around line 33-50: Duplicate logic that resolves non-trailing "**" targets
(the block that computes targetPath, applies (m.options as
any)._redirectStripBase via withoutBase, trims leading "/", skips redirect when
empty, replaces "**" in target, and merges event.url.search via withQuery)
should be extracted into a shared helper (e.g.,
resolveNonTrailingWildcardTarget(target, event, m)) and invoked from both
branches where this flow appears (the block using target.includes("**") at the
current location and the similar block around lines 71-88). Implement the helper
to accept the original target string, the event (to read url.pathname and
searchParams), and the route metadata/options (to read _redirectStripBase),
perform the exact steps currently duplicated (strip base with withoutBase,
remove leading slash, return null/undefined if empty to indicate skip,
substitute "**" with computed path, and merge query via withQuery), and replace
the duplicated blocks to call it and handle its null/undefined result
identically to the existing early-return behavior.
In `@test/tests.ts`:
- Around line 316-325: Add assertions to strengthen the regression by verifying
that source query parameters are merged into the redirect target: when calling
callHandler for "/rules/redirect/wildcard-query/FOO" include a query string
(e.g., "?src=bar") and assert that wildcardQuery.status is 301 and that
wildcardQuery.headers.location contains both the target param from the rule
(param=FOO) and the original query param (src=bar) in the resulting location;
likewise, ensure the case for the empty path (wildcardQueryEmpty) still does not
return 301 even when a source query is present. Use the existing variables
callHandler, wildcardQuery, and wildcardQueryEmpty to locate where to add these
assertions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/presets/netlify/utils.tssrc/presets/vercel/utils.tssrc/runtime/internal/route-rules.tstest/fixture/nitro.config.tstest/presets/netlify.test.tstest/presets/vercel.test.tstest/tests.ts
| status: routeRules.redirect.status, | ||
| headers: { | ||
| Location: routeRules.redirect.to.replace("/**", "/$1"), | ||
| Location: routeRules.redirect.to.replace("/**", "/$1").replace("**", "$1"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file in question
find . -name "utils.ts" -path "*/presets/vercel/*" -type fRepository: nitrojs/nitro
Length of output: 86
🏁 Script executed:
# Once we have the file, read the relevant sections around lines 170 and 187
wc -l src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 88
🏁 Script executed:
# Read the section around line 170
sed -n '160,180p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 803
🏁 Script executed:
# Read the section around line 187
sed -n '177,197p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 786
🏁 Script executed:
# Get broader context for both occurrences
sed -n '150,200p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 1714
🌐 Web query:
Vercel Build Output API v3 routing regex patterns empty capture (.*) vs (.+)
💡 Result:
In Vercel Build Output API v3 (.vercel/output/config.json), routes[].src is a PCRE-compatible regex that matches the incoming pathname (no querystring), and you can reuse capture groups in dest as $1, $2, etc. [1]
(.*) vs (.+) in src
(.*)= “zero or more characters”- The capture can be empty.
- So
^/api/(.*)$matches both:/api/hello→$1 = "hello"/api/→$1 = ""(empty)
(.+)= “one or more characters”- The capture cannot be empty.
- So
^/api/(.+)$matches/api/hello, but not/api/.
Why it matters (common routing gotcha)
If your dest uses $1, then with (.*) you must be okay with $1 being empty sometimes (e.g. trailing-slash variants like /api/). If you require a segment after a prefix, use (.+) to avoid “empty capture” matches.
Vercel also documents that these legacy-style routes use PCRE regex semantics (including typical /(.*) catch-alls), which is the same routing syntax Build Output API v3 is using. [1][2]
Sources:
[1] Vercel — Build Output API v3: Configuration (routes, src, capture groups in dest) (vercel.com)
[2] Vercel — vercel.json legacy routes pattern matching examples (PCRE regex usage) (vercel.com)
Citations:
- 1: https://vercel.com/docs/build-output-api/v3/configuration?utm_source=openai
- 2: https://vercel.com/docs/project-configuration/vercel-json?utm_source=openai
🏁 Script executed:
# Search for empty path handling or related logic in the vercel preset
grep -n "empty\|skip\|capture\|(.+)\|(.*))" src/presets/vercel/utils.ts | head -30Repository: nitrojs/nitro
Length of output: 1940
🏁 Script executed:
# Check if there are any recent commits modifying these lines
git log --oneline -n 20 -- src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 143
🏁 Script executed:
# Look for the runtime empty-path skip behavior mentioned in the comment
rg "empty.*path\|path.*empty\|skip.*empty" src/ --type ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look at the context around line 90 where matchAll("", path) is called
sed -n '85,95p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 412
🏁 Script executed:
# Check what the function at line 464-465 is doing with the ** pattern
sed -n '460,475p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 542
🏁 Script executed:
# Search for the function that contains the buildRoute logic around line 428-465
sed -n '420,520p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 2667
🏁 Script executed:
# Check the commit that modified these lines to understand the intent
git show fc3797e --statRepository: nitrojs/nitro
Length of output: 48472
🏁 Script executed:
# Look at the actual diff of that commit
git show fc3797e src/presets/vercel/utils.ts | head -100Repository: nitrojs/nitro
Length of output: 3348
🏁 Script executed:
# Check if there are tests for these redirect/proxy rules
rg "redirect.*\.\*\*|proxy.*\.\*\*" test/presets/vercel.test.ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look for any existing test cases that exercise empty path behavior
rg "redirect\|proxy" test/presets/vercel.test.ts | head -20Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Get the vercel test file to understand what's being tested
wc -l test/presets/vercel.test.tsRepository: nitrojs/nitro
Length of output: 88
🏁 Script executed:
# Search for redirect or proxy rule tests
rg "routeRules\|redirect\|proxy" test/presets/vercel.test.ts | head -30Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look at what the generated routes should look like - search for routes config generation
sed -n '150,200p' test/presets/vercel.test.tsRepository: nitrojs/nitro
Length of output: 1845
🏁 Script executed:
# Search for test cases with redirect patterns
grep -n "redirect\|proxy" test/presets/vercel.test.ts | grep -E "to:|src:" | head -20Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look for the fixture config with routeRules to see what's being tested
grep -A 10 "routeRules" test/fixture/nitro.config.ts | head -40Repository: nitrojs/nitro
Length of output: 499
🏁 Script executed:
# Search for redirect tests with wildcards in vercel test
grep -n "redirect.*\*\*\|proxy.*\*\*\|/api/\*\*" test/presets/vercel.test.tsRepository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Look at broader sections of the vercel test to find redirect/proxy rule tests
sed -n '1,100p' test/presets/vercel.test.ts | grep -A 5 -B 5 "redirect\|proxy"Repository: nitrojs/nitro
Length of output: 825
🏁 Script executed:
# Check what the actual routes generated look like - search for assertion patterns
grep -n "routes.*redirect\|routes.*proxy\|src.*Location\|dest" test/presets/vercel.test.ts | head -20Repository: nitrojs/nitro
Length of output: 1166
🏁 Script executed:
# Look for the fixture config with redirect/proxy rules that include ** patterns
grep -B 5 -A 5 "redirect\|proxy" test/fixture/nitro.config.ts | grep -A 10 "\*\*"Repository: nitrojs/nitro
Length of output: 1078
🏁 Script executed:
# Check the exact lines 160-200 in utils.ts to understand the structure better
sed -n '160,200p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 1465
🏁 Script executed:
# Check the actual test assertions for these redirect routes
sed -n '1,150p' test/presets/vercel.test.ts | tail -80Repository: nitrojs/nitro
Length of output: 2545
🏁 Script executed:
# Search for assertions on the redirect routes in the test
rg "wildcard.*src.*Location|wildcard-query" test/presets/vercel.test.ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 235
🏁 Script executed:
# Look at what the test actually checks for these wildcard routes
sed -n '50,120p' test/presets/vercel.test.tsRepository: nitrojs/nitro
Length of output: 2222
🏁 Script executed:
# Search for any empty path guards or skip logic related to redirect/proxy
rg "empty.*path|skip.*empty|\$1.*empty" src/ --type ts -B 2 -A 2Repository: nitrojs/nitro
Length of output: 502
🏁 Script executed:
# Look at the runtime route matching logic to see if there's empty-path handling
sed -n '90,100p' src/presets/vercel/utils.tsRepository: nitrojs/nitro
Length of output: 561
🏁 Script executed:
# Check the actual fixture routes to see if empty paths are expected to be handled
grep -A 20 "rules/redirect/wildcard" test/fixture/nitro.config.tsRepository: nitrojs/nitro
Length of output: 931
🏁 Script executed:
# Check the runtime route-rules implementation to understand empty path behavior
find src/runtime -name "*route*" -type fRepository: nitrojs/nitro
Length of output: 92
🏁 Script executed:
# Search for the matchAll function and related route matching logic
rg "matchAll\|empty.*capture" src/ --type ts -B 3 -A 3 | head -40Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Read the route-rules runtime implementation
cat src/runtime/internal/route-rules.tsRepository: nitrojs/nitro
Length of output: 4467
Use (.+) instead of (.*) for non-trailing wildcard redirects and proxies to match runtime behavior.
Lines 170 and 187 generate Vercel routes using (.*) which allows empty captures. However, the runtime explicitly skips non-trailing ** redirects and proxies when the captured path is empty. This creates a parity gap where Vercel may attempt a redirect/proxy with an empty parameter that the runtime would skip.
Fix by checking if the redirect/proxy destination contains non-trailing ** and use (.+) instead of (.*) in those cases to require at least one captured character:
Suggested fix
- let route = {
- src: path.replace("/**", "/(.*)"),
- };
+ const hasNonTrailingWildcard = routeRules.redirect?.to.includes("**") && !routeRules.redirect.to.endsWith("/**");
+ let route = {
+ src: path.replace("/**", hasNonTrailingWildcard ? "/(.+)" : "/(.*)"),
+ };
- const route: Record<string, any> = {
- src: path.replace("/**", "/(.*)"),
- dest: proxy.to.replace("/**", "/$1").replace("**", "$1"),
- };
+ const hasNonTrailingWildcard = proxy.to.includes("**") && !proxy.to.endsWith("/**");
+ const route: Record<string, any> = {
+ src: path.replace("/**", hasNonTrailingWildcard ? "/(.+)" : "/(.*)"),
+ dest: proxy.to.replace("/**", "/$1").replace("**", "$1"),
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/presets/vercel/utils.ts` at line 170, The generated Vercel route regex
uses "(.*)" which allows empty captures; change the logic that builds
redirect/proxy Location strings (the expressions using
routeRules.redirect.to.replace("/**", "/$1").replace("**", "$1") and the
analogous proxy handling) to detect non-trailing "**" usages in the destination
and use "(.+)" instead of "(.*)" for those cases so the regex requires at least
one character; implement this by checking the destination for non-trailing "**"
and choosing replace patterns that insert "/$1" or "$1" based on whether you
need "/(.+)" vs "/(.*)" (and similarly for proxies) so runtime parity is
preserved.
1 participant
📚 Description
This fixes an issue where using the
**wildcard in query parameters for route rule redirects did not work. Previously, only ending route slugs were supported.Now, it is possible to do the following:
I'm not very familiar with the route redirect presets for providers like Vercel, etc. I implemented that part based on the existing code, so please let me know if there are any issues 🙏