Skip to content

fix: handle non-standard HTTP methods in router#3808

Merged
bartlomieju merged 2 commits into
mainfrom
orch/issue-63
May 26, 2026
Merged

fix: handle non-standard HTTP methods in router#3808
bartlomieju merged 2 commits into
mainfrom
orch/issue-63

Conversation

@lunadogbot
Copy link
Copy Markdown
Contributor

Summary

Fixes #3804.

Production apps crashed with TypeError: handler2 is not a function when a request arrived with a non-standard HTTP method such as PROPFIND (common from WebDAV scanners hitting public Fresh sites).

The router's match() method indexed byMethod with the request method and treated the result with item !== null. RouteByMethod<T> only declares entries for the seven known methods (GET/POST/PATCH/PUT/DELETE/HEAD/OPTIONS), so for any other verb byMethod[method] was undefined — which slipped past the !== null check and surfaced as methodMatch: true with item: undefined, crashing the request pipeline.

Normalize the lookup with ?? null in both the static and dynamic branches so unknown verbs route through the existing DEFAULT_NOT_ALLOWED_METHOD path and return a 405.

Tests

  • Two unit tests in router_test.ts for PROPFIND against static and dynamic routes — assert methodMatch: false, item: null.
  • One integration test in app_test.tsx exercising the full app handler with PROPFIND on both a static and a dynamic route — asserts a 405 response and that nothing throws.

deno task ok passes locally (fmt, lint, type check, full test suite, docs check).

Test plan

  • Unit tests for non-standard method on static + dynamic routes
  • Integration test for the full request pipeline returning 405
  • deno task ok passes locally

Closes bartlomieju/orchid-inbox#63

The router's static and dynamic match branches assumed
`byMethod[method]` was either `T` or `null`, but the type's
key set only covers the seven standard HTTP verbs. A request
with a non-standard method (e.g. PROPFIND from WebDAV scanners)
caused `byMethod[method]` to be `undefined`, which slipped past
the `item !== null` check and surfaced as `methodMatch: true`
with `item: undefined`, crashing the handler with
`TypeError: handler2 is not a function`.

Normalize the lookup with `?? null` in both branches so unknown
verbs route through the existing `DEFAULT_NOT_ALLOWED_METHOD`
path and return a 405 instead of throwing.

Fixes #3804
@lunadogbot
Copy link
Copy Markdown
Contributor Author

The canary CI failures are unrelated to this PR — they're a Deno canary regression around node: scheme resolution during Vite SSR module loading.

All 9 failing tests are in packages/plugin-vite/tests/dev_server_test.ts, packages/plugin-vite/tests/build_test.ts, and www/main_test.ts (which uses buildVite). The error is:

TypeError: Unsupported scheme "node" for module "node:assert". Supported schemes: blob, data, file, http, https, jsr, npm
  at async eval (file:///...node_modules/.deno/rollup@4.55.1/node_modules/rollup/dist/es/rollup.js:14:8)

The same canary failures appear on unrelated PR #3807 (no router changes). This PR's v2.x suite passes on all three platforms, and the new router tests (UrlPatternRouter - non-standard method on static route and ... dynamic route) pass locally on stable Deno.

This PR only touches packages/fresh/src/router.ts and the two test files — no Vite plugin changes.

@bartlomieju bartlomieju enabled auto-merge (squash) May 26, 2026 14:22
@bartlomieju bartlomieju merged commit d738f22 into main May 26, 2026
9 checks passed
@bartlomieju bartlomieju deleted the orch/issue-63 branch May 26, 2026 14: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.

Fresh built handlers2 error

2 participants