From b8a25b1b9bf269aab8075b852d9e9d3ad33c1ca4 Mon Sep 17 00:00:00 2001 From: lunadogbot Date: Thu, 14 May 2026 05:18:36 +0000 Subject: [PATCH] fix: handle non-standard HTTP methods in router 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 --- packages/fresh/src/app_test.tsx | 27 +++++++++++++++++++++++++ packages/fresh/src/router.ts | 8 ++++++-- packages/fresh/src/router_test.ts | 33 +++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/fresh/src/app_test.tsx b/packages/fresh/src/app_test.tsx index 67563b9f469..c872a83f12e 100644 --- a/packages/fresh/src/app_test.tsx +++ b/packages/fresh/src/app_test.tsx @@ -217,6 +217,33 @@ Deno.test("App - wrong method match", async () => { expect(await res.text()).toEqual("ok"); }); +Deno.test("App - non-standard method does not crash handler", async () => { + // Regression test for https://github.com/denoland/fresh/issues/3804 + // WebDAV scanners (and other clients) routinely send verbs outside the + // seven methods Fresh's router declares (GET/POST/PATCH/PUT/DELETE/HEAD/ + // OPTIONS). Those used to surface as `methodMatch: true` with an + // `undefined` item, throwing "handler2 is not a function" in production. + const app = new App() + .get("/", () => new Response("ok")) + .get("/books/:id", (ctx) => new Response(ctx.params.id)); + + const server = new FakeServer(app.handler()); + + // Static route + let res = await server.request( + new Request("http://localhost/", { method: "PROPFIND" }), + ); + expect(res.status).toEqual(405); + expect(await res.text()).toEqual("Method Not Allowed"); + + // Dynamic route + res = await server.request( + new Request("http://localhost/books/123", { method: "PROPFIND" }), + ); + expect(res.status).toEqual(405); + expect(await res.text()).toEqual("Method Not Allowed"); +}); + Deno.test("App - methods with middleware", async () => { const app = new App<{ text: string }>() .use((ctx) => { diff --git a/packages/fresh/src/router.ts b/packages/fresh/src/router.ts index f487bffe8b8..c0bbfa9293a 100644 --- a/packages/fresh/src/router.ts +++ b/packages/fresh/src/router.ts @@ -139,7 +139,11 @@ export class UrlPatternRouter implements Router { if (staticMatch !== undefined) { result.pattern = pathname; - let item = staticMatch.byMethod[method]; + // `byMethod[method]` is `undefined` for non-standard HTTP verbs + // (e.g. PROPFIND from WebDAV scanners). Normalize to `null` so the + // not-allowed-method path is taken instead of returning a bogus + // `methodMatch: true` with `item: undefined`. + let item: T | null = staticMatch.byMethod[method] ?? null; if (method === "HEAD" && item === null) { item = staticMatch.byMethod.GET; } @@ -159,7 +163,7 @@ export class UrlPatternRouter implements Router { result.pattern = route.pattern.pathname; - let item = route.byMethod[method]; + let item: T | null = route.byMethod[method] ?? null; if (method === "HEAD" && item === null) { item = route.byMethod.GET; } diff --git a/packages/fresh/src/router_test.ts b/packages/fresh/src/router_test.ts index 5198f540f77..479f2f8439c 100644 --- a/packages/fresh/src/router_test.ts +++ b/packages/fresh/src/router_test.ts @@ -286,3 +286,36 @@ Deno.test("UrlPatternRouter - duplicate registration different methods", () => { const postRes = router.match("POST", new URL("/foo", "http://localhost")); expect(postRes.item).toBe(postHandler); }); + +Deno.test("UrlPatternRouter - non-standard method on static route", () => { + const router = new UrlPatternRouter(); + const A = () => {}; + router.add("GET", "/foo", A); + + // Methods outside the seven known verbs (here PROPFIND, common from + // WebDAV scanners) must surface as `methodMatch: false` with a `null` + // item rather than returning `undefined` and crashing the handler. + // deno-lint-ignore no-explicit-any + const res = router.match("PROPFIND" as any, new URL("/foo", "http://x")); + expect(res).toEqual({ + params: Object.create(null), + item: null, + methodMatch: false, + pattern: "/foo", + }); +}); + +Deno.test("UrlPatternRouter - non-standard method on dynamic route", () => { + const router = new UrlPatternRouter(); + const A = () => {}; + router.add("GET", "/books/:id", A); + + // deno-lint-ignore no-explicit-any + const res = router.match("PROPFIND" as any, new URL("/books/1", "http://x")); + expect(res).toEqual({ + params: Object.create(null), + item: null, + methodMatch: false, + pattern: "/books/:id", + }); +});