Skip to content

refactor: extract shared request handling from server entry points #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yunus25jmi1 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

refactor: extract shared request handling from server entry points #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b59724d
refactor: extract shared request handling into config-matchers and re…
yunus25jmi1 Mar 5, 2026
bd15f7d
fix: address bonk review - remove dead applyTo param, dangling comment
yunus25jmi1 Mar 8, 2026
44139b0
Merge remote-tracking branch 'upstream/main' into refactor/shared-req…
yunus25jmi1 Mar 8, 2026
48e4924
fix: remove dangling comments (parseCookies stub, sanitizeDestination…
yunus25jmi1 Mar 8, 2026
3632312
fix: remove bare block around __tsRedirect, add multiple trailing sla…
yunus25jmi1 Mar 8, 2026
a2d7ec7
fix: stricter /api prefix check for trailing slash normalization (mat…
yunus25jmi1 Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/vinext/src/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ export default {
}

// ── 2. Trailing slash normalization ────────────────────────────
if (pathname !== "/" && !pathname.startsWith("/api")) {
if (pathname !== "/" && pathname !== "/api" && !pathname.startsWith("/api/")) {
const hasTrailing = pathname.endsWith("/");
if (trailingSlash && !hasTrailing) {
return new Response(null, {
Expand Down
205 changes: 19 additions & 186 deletions packages/vinext/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import { findInstrumentationFile, runInstrumentation } from "./server/instrument
import { PHASE_PRODUCTION_BUILD, PHASE_DEVELOPMENT_SERVER } from "./shims/constants.js";
import { validateDevRequest } from "./server/dev-origin-check.js";
import {
safeRegExp,
isExternalUrl,
proxyExternalRequest,
parseCookies,
matchHeaders,
matchRedirect,
matchRewrite,
requestContextFromRequest,
sanitizeDestination,
type RequestContext,
} from "./config/config-matchers.js";
import { scanMetadataFiles } from "./server/metadata-routes.js";
Expand Down Expand Up @@ -821,6 +821,7 @@ import { runWithHeadState } from "vinext/head-state";
import { safeJsonStringify } from "vinext/html";
import { getSSRFontLinks as _getSSRFontLinks, getSSRFontStyles as _getSSRFontStylesGoogle, getSSRFontPreloads as _getSSRFontPreloadsGoogle } from "next/font/google";
import { getSSRFontStyles as _getSSRFontStylesLocal, getSSRFontPreloads as _getSSRFontPreloadsLocal } from "next/font/local";
import { parseCookies } from ${JSON.stringify(path.resolve(__dirname, "config/config-matchers.js").replace(/\\/g, "/"))};
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good change. The generated Pages Router entry now imports parseCookies from the canonical config-matchers.ts instead of inlining its own copy. The import uses path.resolve(__dirname, ...) which is consistent with the established CJS-compat pattern in this file (vs. the fileURLToPath(new URL(..., import.meta.url)) ESM pattern in app-dev-server.ts).

${instrumentationImportCode}
${middlewareImportCode}

Expand Down Expand Up @@ -1090,16 +1091,6 @@ function parseCookieLocaleFromHeader(cookieHeader) {
return null;
}

function parseCookies(cookieHeader) {
const cookies = {};
if (!cookieHeader) return cookies;
for (const part of cookieHeader.split(";")) {
const [key, ...rest] = part.split("=");
if (key) cookies[key.trim()] = rest.join("=").trim();
}
return cookies;
}

// Lightweight req/res facade for getServerSideProps and API routes.
// Next.js pages expect ctx.req/ctx.res with Node-like shapes.
function createReqRes(request, url, query, body) {
Expand Down Expand Up @@ -2867,7 +2858,7 @@ hydrate();

// Normalize trailing slash based on next.config.js trailingSlash setting.
// Redirect to the canonical form if needed.
if (nextConfig && pathname !== "/" && !pathname.startsWith("/api")) {
if (nextConfig && pathname !== "/" && pathname !== "/api" && !pathname.startsWith("/api/")) {
const hasTrailing = pathname.endsWith("/");
if (nextConfig.trailingSlash && !hasTrailing) {
// trailingSlash: true — redirect /about → /about/
Expand Down Expand Up @@ -2992,6 +2983,8 @@ hydrate();

// Build request context once for has/missing condition checks
// across headers, redirects, and rewrites.
// Convert Node.js IncomingMessage headers to a Web Request for
// requestContextFromRequest(), which uses the standard Web API.
const reqUrl = new URL(url, `http://${req.headers.host || "localhost"}`);
const reqCtxHeaders = new Headers(
Object.fromEntries(
Expand All @@ -3000,12 +2993,9 @@ hydrate();
.map(([k, v]) => [k, Array.isArray(v) ? v.join(", ") : String(v)])
),
);
const reqCtx: RequestContext = {
headers: reqCtxHeaders,
cookies: parseCookies(reqCtxHeaders.get("cookie")),
query: reqUrl.searchParams,
host: reqCtxHeaders.get("host") ?? reqUrl.host,
};
const reqCtx: RequestContext = requestContextFromRequest(
new Request(reqUrl, { headers: reqCtxHeaders }),
);

// Apply custom headers from next.config.js
if (nextConfig?.headers.length) {
Expand Down Expand Up @@ -3853,50 +3843,14 @@ function getNextPublicEnvDefines(): Record<string, string> {
return defines;
}

/**
* If the current position in `str` starts with a parenthesized group, consume
* it and advance `re.lastIndex` past the closing `)`. Returns the group
* contents or null if no group is present.
*/
function extractConstraint(str: string, re: RegExp): string | null {
if (str[re.lastIndex] !== "(") return null;
const start = re.lastIndex + 1;
let depth = 1;
let i = start;
while (i < str.length && depth > 0) {
if (str[i] === "(") depth++;
else if (str[i] === ")") depth--;
i++;
}
if (depth !== 0) return null;
re.lastIndex = i;
return str.slice(start, i - 1);
}
// matchConfigPattern is imported from config-matchers.ts and re-exported
// for tests and other consumers that import it from vinext's main entry.
// The duplicate local implementation and its extractConstraint helper
// have been removed in favor of the canonical config-matchers.ts version
// which uses a single-pass tokenizer (fixing the chained .replace()
// divergence that CodeQL flagged as incomplete sanitization).
export { matchConfigPattern } from "./config/config-matchers.js";

/**
* Match a Next.js route pattern (e.g. "/blog/:slug", "/docs/:path*") against a pathname.
* Returns matched params or null.
*
* Supports:
* :param — matches a single path segment
* :param* — matches zero or more segments (catch-all)
* :param+ — matches one or more segments
* (regex) — inline regex patterns in the source
*/
/**
* Strip server-only data-fetching exports from a page module's source code.
* Returns the transformed code, or null if no changes were made.
*
* Handles:
* - export (async) function getServerSideProps(...) { ... }
* - export const getStaticProps = async (...) => { ... }
* - export const getServerSideProps = someHelper;
*/
/**
* Skip past balanced brackets/parens/braces starting at `pos` (which should
* point to the opening bracket). Returns the position AFTER the closing bracket.
* Handles nested brackets, string literals, and comments.
*/
/**
* Strip server-only data-fetching exports (getServerSideProps,
* getStaticProps, getStaticPaths) from page modules for the client
Expand Down Expand Up @@ -3979,125 +3933,6 @@ function stripServerExports(code: string): string | null {
return s.toString();
}

export function matchConfigPattern(
pathname: string,
pattern: string,
): Record<string, string> | null {
// If the pattern contains regex groups like (\\d+) or (.*), use regex matching.
// Also enter this branch when a catch-all parameter (:param* or :param+) is
// followed by a literal suffix (e.g. "/:path*.md"). Without this, the suffix
// pattern falls through to the simple segment matcher which incorrectly treats
// the whole segment (":path*.md") as a named parameter and matches everything.
if (
pattern.includes("(") ||
pattern.includes("\\") ||
/:[\w-]+[*+][^/]/.test(pattern) ||
/:[\w-]+\./.test(pattern)
) {
try {
// Extract named params and their constraints from the pattern.
// :param(constraint) -> use constraint as the regex group
// :param -> ([^/]+)
// :param* -> (.*)
// :param+ -> (.+)
// Param names may contain hyphens (e.g. :auth-method, :sign-in).
const paramNames: string[] = [];
// Single-pass conversion with procedural suffix handling. The tokenizer
// matches only simple, non-overlapping tokens; quantifier/constraint
// suffixes after :param are consumed procedurally to avoid polynomial
// backtracking in the regex engine.
let regexStr = "";
const tokenRe = /:([\w-]+)|[.]|[^:.]+/g; // lgtm[js/redos] — alternatives are non-overlapping (`:` and `.` excluded from `[^:.]+`)
let tok: RegExpExecArray | null;
while ((tok = tokenRe.exec(pattern)) !== null) {
if (tok[1] !== undefined) {
const name = tok[1];
const rest = pattern.slice(tokenRe.lastIndex);
// Check for quantifier (* or +) with optional constraint
if (rest.startsWith("*") || rest.startsWith("+")) {
const quantifier = rest[0];
tokenRe.lastIndex += 1;
const constraint = extractConstraint(pattern, tokenRe);
paramNames.push(name);
if (constraint !== null) {
regexStr += `(${constraint})`;
} else {
regexStr += quantifier === "*" ? "(.*)" : "(.+)";
}
} else {
// Check for inline constraint without quantifier
const constraint = extractConstraint(pattern, tokenRe);
paramNames.push(name);
regexStr += constraint !== null ? `(${constraint})` : "([^/]+)";
}
} else if (tok[0] === ".") {
regexStr += "\\.";
} else {
regexStr += tok[0];
}
}
const re = safeRegExp("^" + regexStr + "$");
if (!re) return null;
const match = re.exec(pathname);
if (!match) return null;
const params: Record<string, string> = {};
for (let i = 0; i < paramNames.length; i++) {
params[paramNames[i]] = match[i + 1] ?? "";
}
return params;
} catch {
// Fall through to segment-based matching
}
}

// Check for catch-all patterns (:param* or :param+) without regex groups
// Param names may contain hyphens (e.g. :sign-in*, :sign-up+).
const catchAllMatch = pattern.match(/:([\w-]+)(\*|\+)$/);
if (catchAllMatch) {
const prefix = pattern.slice(0, pattern.lastIndexOf(":"));
const paramName = catchAllMatch[1];
const isPlus = catchAllMatch[2] === "+";

if (!pathname.startsWith(prefix.replace(/\/$/, ""))) return null;

const rest = pathname.slice(prefix.replace(/\/$/, "").length);
// For :path+ we need at least one segment (non-empty after the prefix)
if (isPlus && (!rest || rest === "/")) return null;
// For :path* zero segments is fine
let restValue = rest.startsWith("/") ? rest.slice(1) : rest;
// NOTE: Do NOT decodeURIComponent here. The pathname is already decoded at
// the entry point. Decoding again would create a double-decode vector.
return { [paramName]: restValue };
}

// Simple segment-based matching for exact patterns and :param
const parts = pattern.split("/");
const pathParts = pathname.split("/");

if (parts.length !== pathParts.length) return null;

const params: Record<string, string> = {};
for (let i = 0; i < parts.length; i++) {
if (parts[i].startsWith(":")) {
params[parts[i].slice(1)] = pathParts[i];
} else if (parts[i] !== pathParts[i]) {
return null;
}
}
return params;
}

/**
* Sanitize a redirect/rewrite destination by collapsing leading slashes and
* backslashes to a single "/" for non-external URLs. Browsers interpret "\"
* as "/" in URL contexts, so "\/evil.com" becomes "//evil.com" (protocol-relative).
*/
function sanitizeDestinationLocal(dest: string): string {
if (dest.startsWith("http://") || dest.startsWith("https://")) return dest;
dest = dest.replace(/^[\\/]+/, "/");
return dest;
}

/**
* Apply redirect rules from next.config.js.
* Returns true if a redirect was applied.
Expand All @@ -4111,17 +3946,15 @@ function applyRedirects(
const result = matchRedirect(pathname, redirects, ctx);
if (result) {
// Sanitize to prevent open redirect via protocol-relative URLs
const dest = sanitizeDestinationLocal(result.destination);
const dest = sanitizeDestination(result.destination);
res.writeHead(result.permanent ? 308 : 307, { Location: dest });
res.end();
return true;
}
return false;
}

/**
* Proxy an external rewrite in the Node.js dev server context.
*
/*
* Converts the Node.js IncomingMessage into a Web Request, calls
* proxyExternalRequest(), and pipes the response back to the Node.js
* ServerResponse.
Expand Down Expand Up @@ -4196,7 +4029,7 @@ function applyRewrites(
const dest = matchRewrite(pathname, rewrites, ctx);
if (dest) {
// Sanitize to prevent open redirect via protocol-relative URLs
return sanitizeDestinationLocal(dest);
return sanitizeDestination(dest);
}
return null;
}
Expand Down
Loading
Loading