fix(config): respect has/missing conditions in next.config headers

[tomasgferreira]

Fixes next.config.headers() conditional matching by adding has/missing support in matchHeaders, wiring request context through dev/prod paths, updating NextHeader types, and enabling coverage with new unit + e2e tests for header/cookie/query conditions (including backward compatibility when no context is provided).

[southpolesteve]

/bigbonk review

[ask-bonk]

Review: fix(config): respect has/missing conditions in next.config headers

Overall this is a solid, well-structured change. The has/missing condition plumbing is correctly wired through all four request-handling paths (Pages Router dev in index.ts, Pages Router prod in prod-server.ts, App Router dev in app-dev-server.ts, and the shared matchHeaders in config-matchers.ts). The tests are thorough and cover the key scenarios. A few things worth addressing:

Issues

1. Duplicate __buildRequestContext calls in app-dev-server — The request context is now built twice per request: once at line 1129 (inside the headers block) and once at line 1171 (for redirects/rewrites). These both call new URL(request.url) and re-parse cookies, which is wasteful. Since the request is immutable at this point, the context should be built once and reused. This is a minor efficiency nit but worth cleaning up since it's in a hot path.

2. Duplicated header-matching logic — The applyHeaders function in index.ts (lines 3463-3500) now contains the same regex + has/missing logic as matchHeaders in config-matchers.ts. The only difference is that applyHeaders calls res.setHeader() inline while matchHeaders returns an array. Consider refactoring applyHeaders to delegate to matchHeaders and then iterate the result to call setHeader, which would eliminate the duplication and ensure the two paths can't drift.

3. Legacy behavior test semantics — The "keeps legacy behavior without ctx" test (shims.test.ts:2710) verifies that when no ctx is passed, rules with has conditions are applied unconditionally. This is technically correct for backward compat, but it means a caller that forgets to pass ctx will silently ignore has/missing conditions. This design choice is fine for now, but it might be worth a brief code comment in matchHeaders explaining why ctx is optional and what happens when it's omitted.

Minor nits

See inline comments.

[ask-bonk]

This builds a RequestContext from the request, but __buildRequestContext(request) is called again at line 1171 for the same request. Since the request is immutable at this point, consider building the context once (e.g., at line 1171 before the redirect check) and passing it to both __applyConfigHeaders and __applyConfigRedirects.

Suggested change

	const __reqCtx = __buildRequestContext(request);
	const extraHeaders = __applyConfigHeaders(pathname, __reqCtx);

Then move the const __reqCtx = __buildRequestContext(request) declaration to before the if (__configHeaders.length ...) block at line 1125 so both code paths share the same context.

[ask-bonk]

This constructs a Request object solely to pass to requestContextFromRequest. Since you already have req.headers and url available, it may be cleaner to build the RequestContext directly without the intermediate Request allocation:

Suggested change

	const requestForCtx = new Request(new URL(url, `http://${req.headers.host || "localhost"}`), {
	const reqUrl = new URL(url, `http://${req.headers.host || "localhost"}`);

Then construct the context manually:

const reqCtx: RequestContext = {
  headers: new Headers(
    Object.fromEntries(
      Object.entries(req.headers)
        .filter(([, v]) => v !== undefined)
        .map(([k, v]) => [k, Array.isArray(v) ? v.join(", ") : String(v)])
    )
  ),
  cookies: parseCookies(req.headers.cookie as string | null),
  query: reqUrl.searchParams,
  host: req.headers.host || "localhost",
};

This avoids the new Request() allocation (which also copies the body stream descriptor). Not a correctness issue, just a minor efficiency improvement.

[ask-bonk]

This applyHeaders function now contains the same regex matching + has/missing checking logic as matchHeaders in config-matchers.ts. Consider refactoring to reuse matchHeaders:

function applyHeaders(
  pathname: string,
  res: any,
  headers: NextHeader[],
  ctx?: RequestContext,
): void {
  const matched = matchHeaders(pathname, headers, ctx);
  for (const header of matched) {
    res.setHeader(header.key, header.value);
  }
}

This eliminates ~30 lines of duplicated pattern matching logic and ensures the two code paths can't drift out of sync — which is exactly the kind of parity issue flagged in AGENTS.md.

[ask-bonk]

Worth a brief comment explaining the design: when ctx is not provided, has/missing conditions are silently ignored and all path-matched rules apply unconditionally. This is intentional for backward compatibility but could be surprising to future readers.

Suggested change

	if (ctx && (rule.has || rule.missing)) {
	// When no request context is available, skip has/missing checks
	// and apply all path-matched rules unconditionally (backward compat).
	if (ctx && (rule.has || rule.missing)) {

[ask-bonk]

Nice test case — verifying that preview=0 doesn't match the value: "1" condition is a good edge case. One thing that might be worth testing in a follow-up: a has query condition with no value (i.e., just checking for the key's presence regardless of value). That would exercise the condition.value !== undefined branch in checkSingleCondition.

[ask-bonk]

Review posted. Here's a summary of the feedback:

Key findings

1. Correctness is good — The has/missing condition plumbing is correctly wired through all four request-handling paths (Pages Router dev, Pages Router prod, App Router dev, and shared matchHeaders). The type updates to NextHeader are correct and backward-compatible.

2. Duplicate __buildRequestContext in app-dev-server — The request context is built twice per request (line 1129 for headers, line 1171 for redirects). Should be built once and shared.

3. Duplicated header-matching logic in index.ts — The applyHeaders function (lines 3463-3500) reimplements the same regex + has/missing logic that matchHeaders in config-matchers.ts already provides. applyHeaders should delegate to matchHeaders to eliminate ~30 lines of duplicated code and prevent drift between the two paths.

4. Missing code comment — The ctx parameter being optional means has/missing conditions are silently ignored when no context is passed. This is intentional for backward compat but deserves a comment.

5. Tests are solid — Good coverage of has-header, missing-cookie, has-query, and legacy-no-ctx scenarios. Suggested a follow-up test for has query with no value (key-existence-only check).

github run

[southpolesteve]

/bigbonk one last review before I merge please

[pkg-pr-new]

Open in StackBlitz

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

commit: 2199eb5

[ask-bonk]

Review: fix(config): respect has/missing conditions in next.config headers

This PR correctly wires has/missing condition support through all request-handling paths for next.config.headers(). The implementation is clean and addresses a real gap in Next.js API compatibility.

What's good

1. Proper deduplication in index.ts — All three local wrappers (applyHeaders, applyRedirects, applyRewrites) now delegate to their shared counterparts from config-matchers.ts. This eliminates the ~30 lines of duplicated regex+matching logic that existed before and prevents drift between Pages Router dev and the shared matchers. This was flagged in the previous review and has been addressed.

2. Single __buildRequestContext call in app-dev-server.ts — The request context is built once at line 1193 (inside handler) and passed to both _handleRequest (for redirects/rewrites) and __applyConfigHeaders (for headers). The previous review's concern about duplicate construction has been addressed.

3. Correct backward compatibility — The ctx parameter being optional in matchHeaders means existing callers that don't pass context still work. Rules with has/missing are applied unconditionally when no context is provided, which is a reasonable default.

4. Good test coverage — Unit tests cover has-header, missing-cookie, has-query, and legacy-no-ctx scenarios. E2E tests cover the same conditions against the actual dev server with the app-basic fixture.

5. All four request paths are covered — Pages Router dev (index.ts), Pages Router prod (prod-server.ts), App Router dev (app-dev-server.ts), and the shared matchHeaders in config-matchers.ts all wire through ctx. This is exactly the kind of parity check AGENTS.md requires.

Minor observations (non-blocking)

See inline comments for a couple of minor nits. None are blocking.

[ask-bonk]

Nit: You're building the RequestContext manually here (parsing headers, cookies, query) rather than using the existing requestContextFromRequest helper from config-matchers.ts. This is intentional since you're working with a Node.js IncomingMessage (not a Web Request), so constructing a new Request() just to pass it to requestContextFromRequest would be wasteful.

That said, this pattern of converting Node.js headers to Headers + parsing cookies manually is somewhat boilerplate-heavy. If this pattern recurs elsewhere, it might be worth adding a requestContextFromNodeRequest(req, url) helper to config-matchers.ts. Not needed for this PR though.

[ask-bonk]

The previous review suggested adding a test for has query with no value (key-existence-only check), which would exercise the condition.value !== undefined branch in checkSingleCondition. Would be a nice addition in a follow-up — not blocking this PR.

[ask-bonk]

This __applyConfigHeaders function duplicates the escaping logic from escapeHeaderSource in config-matchers.ts. This is expected since the app-dev-server generates self-contained virtual module code that can't import from config-matchers.ts at runtime. Just flagging that if escapeHeaderSource ever changes (e.g., to handle new metacharacters), this inline version needs to be updated in lockstep. A code comment noting this dependency would help future maintainers.

Also worth noting: the inline escaping here uses the simpler ___GROUP_N___ placeholder approach, while escapeHeaderSource uses Unicode PUA sentinels (\uE000). Both achieve the same result but the approaches are slightly different, which could theoretically diverge on edge cases with very unusual source patterns. Probably fine in practice.

[ask-bonk]

Review submitted successfully. Here's a summary:

Approved — This is a solid PR that correctly wires has/missing condition support through all four request-handling paths (Pages Router dev, Pages Router prod, App Router dev, and the shared matchHeaders).

Key findings:

• The previous review's concerns (duplicate __buildRequestContext, duplicated matching logic in index.ts) have been addressed in this revision
• All applyHeaders/applyRedirects/applyRewrites in index.ts now properly delegate to shared matchers from config-matchers.ts
• Backward compatibility is maintained — ctx is optional, so existing callers without context still work
• Good test coverage across unit and E2E tests

I left 3 non-blocking inline observations:

1. A note about potentially extracting a requestContextFromNodeRequest helper if the manual Node.js header conversion pattern recurs
2. Suggestion for a follow-up test covering key-existence-only has query conditions (no value)
3. A note about the escaping logic in __applyConfigHeaders (app-dev-server codegen) needing to stay in sync with escapeHeaderSource in config-matchers.ts, since they use slightly different placeholder approaches

github run