Skip to content

fix(server): reject invalid HTTP methods with 400 in app route handlers#1048

Merged
james-elicx merged 1 commit intocloudflare:mainfrom
NathanDrake2406:fix/http-method-validation
May 4, 2026
Merged

fix(server): reject invalid HTTP methods with 400 in app route handlers#1048
james-elicx merged 1 commit intocloudflare:mainfrom
NathanDrake2406:fix/http-method-validation

Conversation

@NathanDrake2406
Copy link
Copy Markdown
Contributor

Summary

Vinext previously uppercased the request method and fell through to the normal 405 response for any unrecognized method. Next.js rejects non-standard methods with 400 Bad Request before dispatch.

Changes

  • Add isValidHTTPMethod() predicate to app-route-handler-runtime.ts, co-located with the existing ROUTE_HANDLER_HTTP_METHODS constant.
  • Return empty 400 in dispatchAppRouteHandler() before any auto-OPTIONS or 405 logic, matching Next.js behavior.
  • Port the Next.js test that verifies HEADER => 400.

Next.js source reference

Verification

  • vp test run tests/app-route-handler-dispatch.test.ts tests/app-route-handler-policy.test.ts tests/app-route-handler-runtime.test.ts — 25/25 passing
  • vp test run tests/app-router.test.ts -t "route handler" — 28/28 passing

Vinext previously uppercased the request method and fell through to the normal 405 response for any unrecognized method. Next.js rejects non-standard methods with 400 before dispatch.

- Add isValidHTTPMethod() predicate to app-route-handler-runtime.ts

- Return empty 400 in dispatchAppRouteHandler() before auto-OPTIONS/405

- Port Next.js test: HEADER => 400 Bad Request

Next.js source reference:

https://github.com/vercel/next.js/blob/canary/packages/next/src/server/route-modules/app-route/module.ts#L390-L392

https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app-routes/app-custom-routes.test.ts#L531-L538
Copilot AI review requested due to automatic review settings May 4, 2026 05:39
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 4, 2026

Open in StackBlitz

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

commit: 82fa445

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns vinext App Router route handler dispatch behavior with Next.js by rejecting non-standard HTTP methods with a 400 Bad Request response before any auto-OPTIONS or 405 handling occurs.

Changes:

  • Added an isValidHTTPMethod() type-guard based on ROUTE_HANDLER_HTTP_METHODS.
  • Updated dispatchAppRouteHandler() to return an empty 400 response for invalid methods before resolving the handler method.
  • Added a unit test (ported from Next.js) verifying "HEADER" is rejected with 400 and does not trigger cache/regeneration logic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/app-route-handler-dispatch.test.ts Adds coverage ensuring invalid HTTP methods return 400 and bypass route execution/cache/regeneration paths.
packages/vinext/src/server/app-route-handler-runtime.ts Introduces isValidHTTPMethod() co-located with the route handler method constants.
packages/vinext/src/server/app-route-handler-dispatch.ts Performs early invalid-method rejection with 400 before OPTIONS/405 resolution and further processing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@james-elicx
Copy link
Copy Markdown
Collaborator

/bigbonk review

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped fix. The Next.js source confirms the behavior — isHTTPMethod() in packages/next/src/server/web/http.ts is a case-sensitive check against the same 7 methods, and resolveHandler() in module.ts returns () => new Response(null, { status: 400 }) for invalid methods before any dispatch logic.

The placement in dispatchAppRouteHandler is correct — it's the shared dispatch path used by both the RSC entry (entries/app-rsc-entry.ts:579) and production server, so this covers both dev and prod without duplication.

Test is thorough: verifies 400 status, empty body, middleware header passthrough, context cleanup, and uses throwing stubs to prove no cache/regeneration/handler code runs. LGTM.

* @see https://github.com/vercel/next.js/blob/canary/packages/next/src/server/web/http.ts
*/
export function isValidHTTPMethod(maybeMethod: string): maybeMethod is RouteHandlerHttpMethod {
return (ROUTE_HANDLER_HTTP_METHODS as readonly string[]).includes(maybeMethod);
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.

Nit: Next.js uses .includes(maybeMethod as HTTP_METHOD) (casting the argument) rather than casting the array. Both work, but the Next.js pattern avoids widening the array type:

Suggested change
return (ROUTE_HANDLER_HTTP_METHODS as readonly string[]).includes(maybeMethod);
return ROUTE_HANDLER_HTTP_METHODS.includes(maybeMethod as RouteHandlerHttpMethod);

Non-blocking — the current version is functionally identical and both are safe since includes does === comparison.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 4, 2026

Review submitted. Approved with one minor nit about the array casting style in isValidHTTPMethod. The fix is correct — verified against the Next.js source that isHTTPMethod() performs the same case-sensitive check against the same 7 methods, and resolveHandler() returns a 400 for invalid methods before dispatch. The placement in dispatchAppRouteHandler covers both dev and prod paths since it's the shared dispatch function called from both the RSC entry and production server.

github run

@james-elicx james-elicx merged commit 390b460 into cloudflare:main May 4, 2026
28 of 29 checks passed
@NathanDrake2406 NathanDrake2406 deleted the fix/http-method-validation branch May 6, 2026 04:30
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.

3 participants