Skip to content

refactor(server): dedupe forbidden response construction - #1071

Merged
james-elicx merged 1 commit into
mainfrom
refactor/dedupe-forbidden-response
May 5, 2026
Merged

refactor(server): dedupe forbidden response construction#1071
james-elicx merged 1 commit into
mainfrom
refactor/dedupe-forbidden-response

Conversation

@james-elicx

Copy link
Copy Markdown
Member

Summary

  • Follow-up dedupe scan after fix(routing): decode matched params with decodeURIComponent #1049: the literal new Response("Forbidden", { status: 403, headers: { "Content-Type": "text/plain" } }) was repeated 7 times across two server files.
  • Adds a forbiddenResponse() helper in request-pipeline.ts and replaces the 3 byte-identical call sites in validateCsrfOrigin.
  • For the 4 occurrences inside generateDevOriginCheckCode() — which emits inline JS that runs in the RSC Vite environment and cannot import TS modules — introduces a local __forbidden() helper inside the generated code so the duplication is eliminated there too.
  • Pure refactor, no behavior change. All call sites were verified byte-equivalent before merging.

Touched files

  • packages/vinext/src/server/request-pipeline.ts — adds exported forbiddenResponse(); replaces 3 inline literals.
  • packages/vinext/src/server/dev-origin-check.ts — emits a local __forbidden() helper inside the codegen template; replaces 4 inline literals in the generated JS.

Test plan

  • pnpm vp test run tests/app-router.test.ts tests/pages-router.test.ts — 508 tests pass. (One unrelated afterAll cleanup-hook timeout under heavy concurrent load; reruns of the touched suite, including Pages Router allowedDevOrigins config, all pass.)
  • pnpm fmt:check — clean.
  • CI green on the draft PR.

🤖 Generated with Claude Code

Extract the repeated `new Response("Forbidden", { status: 403, ... })`
pattern from dev-origin-check.ts and request-pipeline.ts into a single
shared helper. Follow-up to #1049.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented May 5, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 501cfa7

@github-actions

github-actions Bot commented May 5, 2026

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

@james-elicx

Copy link
Copy Markdown
Member Author

/bigbonk review

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean dedupe. The split between an exported forbiddenResponse() for importable TS code and an inline __forbidden() inside the codegen template is the right call — the generated JS can't import .ts modules, so a local helper is the only option there.

Verified: all 7 original new Response("Forbidden", …) literals are accounted for (3 in request-pipeline.ts, 4 in dev-origin-check.ts), and the replacement is byte-equivalent.

No notes.

@ask-bonk

ask-bonk Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Approved PR #1071. The refactor is straightforward and correct:

  • forbiddenResponse() in request-pipeline.ts replaces 3 identical new Response("Forbidden", { status: 403, ... }) calls in validateCsrfOrigin.
  • __forbidden() inside the codegen template in dev-origin-check.ts replaces 4 identical literals in the generated JS that runs in the RSC Vite environment (can't import TS modules, so an inline helper is the right approach).

All 7 call sites accounted for, no behavior change.

github run

@james-elicx
james-elicx marked this pull request as ready for review May 5, 2026 18:23
@james-elicx
james-elicx merged commit bdc1f34 into main May 5, 2026
33 checks passed
@james-elicx
james-elicx deleted the refactor/dedupe-forbidden-response branch May 5, 2026 18:23
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.

1 participant