Skip to content

fix(config): substitute matched params into headers() keys and values - #4087

Merged
james-elicx merged 1 commit into
mainfrom
fix/header-value-param-substitution
Oct 19, 2026
Merged

james-elicx merged 1 commit into
mainfrom
fix/header-value-param-substitution

Conversation

@james-elicx

Copy link
Copy Markdown
Member

Summary

headers() rules now substitute matched params into header keys and values, like Next.js. Before this change, { source: "/blog/:slug", headers: [{ key: "x-slug", value: ":slug" }] } sent the literal text :slug.

The port follows Next.js 16.2.7 (resolve-routes → compileNonPath / safeCompile in prepare-destination.ts / route-match-utils.ts):

  • Where params come from: the path-to-regexp source match plus has named groups, merged in the same order as matchHas. Unnamed source params are dropped (removeUnnamedParams).
  • Raw captures: values are the raw path text, so /blog/a%20b gives a%20b. Nothing is encoded or validated, matching path-to-regexp compile with validate: false.
  • Repeats: */+ params are arrays and are joined with the template's prefix/suffix (:path* gives a/b).
  • Only captured :name markers are substituted. Every other path-to-regexp metacharacter in the template stays literal, and safeCompile's adjacent-param retry is ported.
  • Fast path: rules whose header keys and values contain no : skip substitution entirely.

Known limitations

When Next.js's compile throws (for example, a repeated param referenced without */+, or an undefined has capture), or when the substituted pair is not a valid HTTP header, Next.js fails the request with a 500. vinext omits that one header instead and warns once per rule. Web Headers would otherwise throw, or silently accept some control characters, so dropping the header is the predictable choice.

Tests

  • tests/shims.test.ts: "matchHeaders param substitution parity with Next.js", with cases checked against real Next.js behaviour.
  • tests/e2e/app-router/config-redirect.spec.ts on the existing app-basic fixture covers:
    • a prerendered /blog/:slug page with params in both key and value;
    • a percent-encoded capture;
    • a :path* + has named-group rule.

Next.js compiles each header key and value with compileNonPath using the
source params (raw path text) merged with matchHas captures. vinext sent the
templates verbatim, and a param in a header key made Headers throw.

Where Next.js fails the request (a template it cannot compile, or a
substituted value that is not a valid header), vinext omits only that header.
@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review this PR. You have an 8-minute time limit. Head SHA: 664de28

@pkg-pr-new

pkg-pr-new Bot commented Oct 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

@vinext/cloudflare

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

create-vinext-app

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

@vinext/types

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

vinext

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

@cloudflare/workers-response-store

npm i https://pkg.pr.new/@cloudflare/workers-response-store@4087

commit: 664de28

Comment thread packages/vinext/src/config/header-params.ts Dismissed
@ask-bonk

ask-bonk Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

LGTM!

github run

@github-actions

Copy link
Copy Markdown
Contributor
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
response-store-demo preview production
workers-cache preview production
kv preview production
static-assets-cache preview production
static-assets-pages preview production
static-assets-pages-i18n preview production
static-export preview production
web preview production

@github-actions

Copy link
Copy Markdown
Contributor

Performance benchmarks

Compared 664de28 against base 55dcb4c using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 7 within ±1.5%

Scenario Framework Baseline Current Change
Client bundle size (gzip) vinext 154.2 KB 154.2 KB ⚫ +0.0%
Client entry size (gzip) vinext 141.7 KB 141.7 KB ⚫ +0.0%
Dev server cold start vinext 2.17 s 2.16 s ⚫ -0.5%
Dev server cold start (large Pages Router app) vinext 18.75 s 18.69 s ⚫ -0.3%
Production build time vinext 2.62 s 2.61 s ⚫ -0.4%
RSC entry closure size (gzip) vinext 141.9 KB 141.9 KB ⚫ -0.0%
Server bundle size (gzip) vinext 249.4 KB 250.5 KB ⚫ +0.4%

View detailed results and traces

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

@james-elicx
james-elicx marked this pull request as ready for review October 18, 2026 23:21
@clanky-the-clanker

Copy link
Copy Markdown

💬 Approved with comments — The port of Next.js compileNonPath param substitution is correct and well tested, and the one deliberate divergence (omitting the header instead of failing the request with a 500) is documented. The only remaining note is an optional performance improvement.

Findings: 1 info

The decision is advisory: this comment never approves, blocks or requests changes on the pull request.

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #4087.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## INFO (1)

### [Performance] Header templates are escaped, parsed and compiled again on every matching request, even though the result depends only on the template and the param names. Caching or a `:name` precheck would keep this off the hot path.
- **File:** packages/vinext/src/config/header-params.ts:29

⚡ Performance: 💬 1 info

One optional note: header templates are re-parsed on every matching request.

Recommendation: 💬 Optional Nits

  • [INFO] Header templates are escaped, parsed and compiled again on every matching request, even though the result depends only on the template and the param names. Caching or a :name precheck would keep this off the hot path. (packages/vinext/src/config/header-params.ts:29)
View full review

Header templates are recompiled on every matching request (packages/vinext/src/config/header-params.ts:29-50)

compileHeaderParamTemplate runs for every header whose key or value contains :, on every request that matches a source with named params. That includes ordinary values such as Link: <https://...> or a CSP containing URLs. Each call builds up to four RegExp objects per param, runs the escape replaces and re-parses the template with parseMiddlewarePath. The escaped template and its tokens depend only on the header string and the set of param names, not on the param values. The repository guidelines ask to hoist deterministic parsing off the hot path, so two options:

  • Cache the parsed tokens per (template, param-name set). The cache is bounded by the size of the config.
  • Add a cheap precheck that returns the template unchanged when no :${name} for any param appears in it.

The cost is a few microseconds per header and the fix is optional.


✅ All other checks passed

🔒 Security: ✅ LGTM

Values from has captures (cookies, query strings, request headers) can end up in response headers, as they can in Next.js. Before any substituted pair is emitted, isValidHeaderPair requires the key to be an HTTP token and the value to contain no CR, LF, NUL or other control characters, so request data cannot inject extra headers. Param names placed into new RegExp(...) come from path-to-regexp names or regex named groups, which are identifier characters only, or from a has key stripped to letters only. They cannot change the regex pattern.


📖 AGENTS.md: ✅ LGTM

This pull request does not change agent workflows, commands or architecture, so AGENTS.md does not need an update.


📋 Code Quality: ✅ LGTM

I read the changed code but did not run the tests or build it.

  • Capture indexing: compileHeaderSource keeps only the tokens that have a pattern, so keys[index - 1] lines up with the regex capture groups. Unnamed (numeric) keys are skipped, which matches removeUnnamedParams.
  • Repeats: a */+ capture is split on prefix + suffix. A group that did not participate is undefined and is skipped, as in path-to-regexp's regexpToFunction.
  • has captures: these follow matchHas. A condition without a value exposes the matched value under its key with non-letters stripped. A value regex exposes its named groups. A host regex without named groups exposes the matched host as host. When a query key appears more than once, the regex is tested against the last value, as in Next.js.
  • Fast path: rules whose header keys and values contain no : push their headers unchanged, so existing behaviour is preserved.
  • Invalid output: a substituted pair is checked with isValidHeaderPair before it is emitted. A header whose key and value come out unchanged skips the check, which is safe because the original pair was already valid.
  • Divergence: Next.js fails the request with a 500 when compiling throws or the result is not a valid header. vinext omits only that header and warns once per header entry. The pull request description documents this and a unit test covers it.
  • Tests: the unit tests check results against real Next.js behaviour, and the e2e tests on the app-basic fixture cover the three scenarios in the description.

📝 Documentation: ✅ LGTM

The new module has JSDoc that links to the Next.js sources it ports. The pull request description explains the known limitation.


Reviewed commit: 664de286

@james-elicx
james-elicx merged commit 5f42a9e into main Oct 19, 2026
105 checks passed
@james-elicx
james-elicx deleted the fix/header-value-param-substitution branch October 19, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants