[PR #258] [MERGED] refactor: extract shared request handling from server entry points #422

New issue

Closed

opened 2026-05-06 12:39:44 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 12:39:44 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/258
Author: @yunus25jmi1
Created: 3/5/2026
Status: ✅ Merged
Merged: 3/9/2026
Merged by: @james-elicx

Base: main ← Head: refactor/shared-request-handling

📝 Commits (8)

b59724d refactor: extract shared request handling into config-matchers and request-pipeline
bd15f7d fix: address bonk review - remove dead applyTo param, dangling comment
44139b0 Merge remote-tracking branch 'upstream/main' into refactor/shared-request-handling
48e4924 fix: remove dangling comments (parseCookies stub, sanitizeDestination stub)
3632312 fix: remove bare block around __tsRedirect, add multiple trailing slash test
a2d7ec7 fix: stricter /api prefix check for trailing slash normalization (matches only /api or /api/), add tests for edge cases
d41dcb3 Merge remote-tracking branch 'origin/main' into refactor/shared-request-handling
83f48b6 test: update entry-template snapshots after merging main

📊 Changes

8 files changed (+777 additions, -2404 deletions)

View changed files

📝 packages/vinext/src/deploy.ts (+1 -1)
📝 packages/vinext/src/index.ts (+19 -188)
📝 packages/vinext/src/server/app-dev-server.ts (+41 -317)
📝 packages/vinext/src/server/prod-server.ts (+1 -1)
➕ packages/vinext/src/server/request-pipeline.ts (+224 -0)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+201 -1867)
📝 tests/app-router.test.ts (+28 -30)
➕ tests/request-pipeline.test.ts (+262 -0)

📄 Description

Summary

Deduplicates ~250 lines of inline request-handling logic from the generated App Router RSC entry (app-dev-server.ts) and Pages Router server entry (index.ts) by importing from the canonical config-matchers.ts and a new request-pipeline.ts module.

Changes

New file: `server/request-pipeline.ts`

Shared request lifecycle utilities extracted for reuse across entry points:

guardProtocolRelativeUrl() — blocks //evil.com protocol-relative URL attacks
stripBasePath() — removes configured basePath prefix from pathnames
normalizeTrailingSlash() — redirects to canonical trailing-slash form
validateCsrfOrigin() — CSRF origin validation for server actions
isOriginAllowed() — wildcard subdomain matching for allowed origins
validateImageUrl() — image optimization URL validation
processMiddlewareHeaders() — strips internal x-middleware-* headers

`app-dev-server.ts` (~250 lines removed)

Replaced 14 inline __-prefixed functions with imports from config-matchers.ts and request-pipeline.ts:

Config matching: matchRedirect, matchRewrite, matchHeaders, sanitizeDestination
External proxying: isExternalUrl, proxyExternalRequest
Request context: requestContextFromRequest
Security: validateCsrfOrigin, validateImageUrl

`index.ts`

Replaced inline parseCookies in generated server entry template with import from config-matchers.ts
Replaced manual RequestContext construction with requestContextFromRequest()
Removed duplicate sanitizeDestinationLocal(), using imported sanitizeDestination()
Removed duplicate matchConfigPattern + extractConstraint (~100 lines), re-exporting from config-matchers.ts

Test updates

Updated app-router.test.ts assertions to reference the new imported function names instead of old inline __-prefixed names.
Added request-pipeline.test.ts with 33 unit tests covering all 6 exported functions.

Behavioral Change

normalizeTrailingSlash now uses relative Location headers instead of absolute URLs. The old inline code used Response.redirect(new URL(..., request.url), 308) which produces an absolute Location header; the new code uses new Response(null, { status: 308, headers: { Location: relativePath } }). This is valid per RFC 7231 §7.1.2 and aligns dev behavior with prod-server.ts (which already uses relative Location headers).

Security Fixes

matchConfigPattern: eliminates chained .replace() divergence (CodeQL flagged incomplete sanitization) in favor of config-matchers single-pass tokenizer
proxyExternalRequest: uses AbortController+setTimeout (broader runtime support) instead of AbortSignal.timeout()
escapeHeaderSource: uses proper config-matchers implementation instead of simplified chained .replace()

Verification

Typecheck: pnpm run typecheck passes clean
Shims tests: 575/575 passed
App Router tests: 209/209 passed
Request pipeline tests: 33/33 passed

Closes #254

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/258 **Author:** [@yunus25jmi1](https://github.com/yunus25jmi1) **Created:** 3/5/2026 **Status:** ✅ Merged **Merged:** 3/9/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `refactor/shared-request-handling` --- ### 📝 Commits (8) - [`b59724d`](https://github.com/cloudflare/vinext/commit/b59724dec9ceeec497a89e37e9b988aecac63a6d) refactor: extract shared request handling into config-matchers and request-pipeline - [`bd15f7d`](https://github.com/cloudflare/vinext/commit/bd15f7df0f971011138413633f1fd162bc8af40b) fix: address bonk review - remove dead applyTo param, dangling comment - [`44139b0`](https://github.com/cloudflare/vinext/commit/44139b028bb7f762caaeca86a7905057e5a1ba39) Merge remote-tracking branch 'upstream/main' into refactor/shared-request-handling - [`48e4924`](https://github.com/cloudflare/vinext/commit/48e492489dbd09f7d0fb32aaf0a122daa0c2f664) fix: remove dangling comments (parseCookies stub, sanitizeDestination stub) - [`3632312`](https://github.com/cloudflare/vinext/commit/36323126fc94406c36da6a23cdfa1fe822ced536) fix: remove bare block around __tsRedirect, add multiple trailing slash test - [`a2d7ec7`](https://github.com/cloudflare/vinext/commit/a2d7ec7ecbb8b0d0f446279bc19c7953ef9a7658) fix: stricter /api prefix check for trailing slash normalization (matches only /api or /api/), add tests for edge cases - [`d41dcb3`](https://github.com/cloudflare/vinext/commit/d41dcb3f2ee13315c48bd017506942390463953b) Merge remote-tracking branch 'origin/main' into refactor/shared-request-handling - [`83f48b6`](https://github.com/cloudflare/vinext/commit/83f48b63685f210979b56c26d3fec43fa0e1f9f9) test: update entry-template snapshots after merging main ### 📊 Changes **8 files changed** (+777 additions, -2404 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/deploy.ts` (+1 -1) 📝 `packages/vinext/src/index.ts` (+19 -188) 📝 `packages/vinext/src/server/app-dev-server.ts` (+41 -317) 📝 `packages/vinext/src/server/prod-server.ts` (+1 -1) ➕ `packages/vinext/src/server/request-pipeline.ts` (+224 -0) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+201 -1867) 📝 `tests/app-router.test.ts` (+28 -30) ➕ `tests/request-pipeline.test.ts` (+262 -0) </details> ### 📄 Description ## Summary Deduplicates ~250 lines of inline request-handling logic from the generated App Router RSC entry (`app-dev-server.ts`) and Pages Router server entry (`index.ts`) by importing from the canonical `config-matchers.ts` and a new `request-pipeline.ts` module. ## Changes ### New file: `server/request-pipeline.ts` Shared request lifecycle utilities extracted for reuse across entry points: - `guardProtocolRelativeUrl()` — blocks `//evil.com` protocol-relative URL attacks - `stripBasePath()` — removes configured basePath prefix from pathnames - `normalizeTrailingSlash()` — redirects to canonical trailing-slash form - `validateCsrfOrigin()` — CSRF origin validation for server actions - `isOriginAllowed()` — wildcard subdomain matching for allowed origins - `validateImageUrl()` — image optimization URL validation - `processMiddlewareHeaders()` — strips internal `x-middleware-*` headers ### `app-dev-server.ts` (~250 lines removed) Replaced 14 inline `__`-prefixed functions with imports from `config-matchers.ts` and `request-pipeline.ts`: - Config matching: `matchRedirect`, `matchRewrite`, `matchHeaders`, `sanitizeDestination` - External proxying: `isExternalUrl`, `proxyExternalRequest` - Request context: `requestContextFromRequest` - Security: `validateCsrfOrigin`, `validateImageUrl` ### `index.ts` - Replaced inline `parseCookies` in generated server entry template with import from `config-matchers.ts` - Replaced manual `RequestContext` construction with `requestContextFromRequest()` - Removed duplicate `sanitizeDestinationLocal()`, using imported `sanitizeDestination()` - Removed duplicate `matchConfigPattern` + `extractConstraint` (~100 lines), re-exporting from `config-matchers.ts` ### Test updates - Updated `app-router.test.ts` assertions to reference the new imported function names instead of old inline `__`-prefixed names. - Added `request-pipeline.test.ts` with 33 unit tests covering all 6 exported functions. ## Behavioral Change `normalizeTrailingSlash` now uses relative `Location` headers instead of absolute URLs. The old inline code used `Response.redirect(new URL(..., request.url), 308)` which produces an absolute `Location` header; the new code uses `new Response(null, { status: 308, headers: { Location: relativePath } })`. This is valid per RFC 7231 §7.1.2 and aligns dev behavior with `prod-server.ts` (which already uses relative `Location` headers). ## Security Fixes - **matchConfigPattern**: eliminates chained `.replace()` divergence (CodeQL flagged incomplete sanitization) in favor of config-matchers single-pass tokenizer - **proxyExternalRequest**: uses `AbortController`+`setTimeout` (broader runtime support) instead of `AbortSignal.timeout()` - **escapeHeaderSource**: uses proper config-matchers implementation instead of simplified chained `.replace()` ## Verification - Typecheck: `pnpm run typecheck` passes clean - Shims tests: 575/575 passed - App Router tests: 209/209 passed - Request pipeline tests: 33/33 passed Closes #254 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>