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

Fixes #3804.

Co-authored-by: lunadogbot <lunadogbot@users.noreply.github.com>
Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

Author: lunadogbot

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) => {

packages/fresh/src/router.ts

@@ -139,7 +139,11 @@ export class UrlPatternRouter<T> implements Router<T> {
     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<T> implements Router<T> {
 
       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;
       }

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",
+  });
+});