Skip to content

Commit b8a25b1

Browse files
committed
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
1 parent 39b5f06 commit b8a25b1

3 files changed

Lines changed: 66 additions & 2 deletions

File tree

packages/fresh/src/app_test.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,33 @@ Deno.test("App - wrong method match", async () => {
217217
expect(await res.text()).toEqual("ok");
218218
});
219219

220+
Deno.test("App - non-standard method does not crash handler", async () => {
221+
// Regression test for https://github.com/denoland/fresh/issues/3804
222+
// WebDAV scanners (and other clients) routinely send verbs outside the
223+
// seven methods Fresh's router declares (GET/POST/PATCH/PUT/DELETE/HEAD/
224+
// OPTIONS). Those used to surface as `methodMatch: true` with an
225+
// `undefined` item, throwing "handler2 is not a function" in production.
226+
const app = new App()
227+
.get("/", () => new Response("ok"))
228+
.get("/books/:id", (ctx) => new Response(ctx.params.id));
229+
230+
const server = new FakeServer(app.handler());
231+
232+
// Static route
233+
let res = await server.request(
234+
new Request("http://localhost/", { method: "PROPFIND" }),
235+
);
236+
expect(res.status).toEqual(405);
237+
expect(await res.text()).toEqual("Method Not Allowed");
238+
239+
// Dynamic route
240+
res = await server.request(
241+
new Request("http://localhost/books/123", { method: "PROPFIND" }),
242+
);
243+
expect(res.status).toEqual(405);
244+
expect(await res.text()).toEqual("Method Not Allowed");
245+
});
246+
220247
Deno.test("App - methods with middleware", async () => {
221248
const app = new App<{ text: string }>()
222249
.use((ctx) => {

packages/fresh/src/router.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ export class UrlPatternRouter<T> implements Router<T> {
139139
if (staticMatch !== undefined) {
140140
result.pattern = pathname;
141141

142-
let item = staticMatch.byMethod[method];
142+
// `byMethod[method]` is `undefined` for non-standard HTTP verbs
143+
// (e.g. PROPFIND from WebDAV scanners). Normalize to `null` so the
144+
// not-allowed-method path is taken instead of returning a bogus
145+
// `methodMatch: true` with `item: undefined`.
146+
let item: T | null = staticMatch.byMethod[method] ?? null;
143147
if (method === "HEAD" && item === null) {
144148
item = staticMatch.byMethod.GET;
145149
}
@@ -159,7 +163,7 @@ export class UrlPatternRouter<T> implements Router<T> {
159163

160164
result.pattern = route.pattern.pathname;
161165

162-
let item = route.byMethod[method];
166+
let item: T | null = route.byMethod[method] ?? null;
163167
if (method === "HEAD" && item === null) {
164168
item = route.byMethod.GET;
165169
}

packages/fresh/src/router_test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,36 @@ Deno.test("UrlPatternRouter - duplicate registration different methods", () => {
286286
const postRes = router.match("POST", new URL("/foo", "http://localhost"));
287287
expect(postRes.item).toBe(postHandler);
288288
});
289+
290+
Deno.test("UrlPatternRouter - non-standard method on static route", () => {
291+
const router = new UrlPatternRouter();
292+
const A = () => {};
293+
router.add("GET", "/foo", A);
294+
295+
// Methods outside the seven known verbs (here PROPFIND, common from
296+
// WebDAV scanners) must surface as `methodMatch: false` with a `null`
297+
// item rather than returning `undefined` and crashing the handler.
298+
// deno-lint-ignore no-explicit-any
299+
const res = router.match("PROPFIND" as any, new URL("/foo", "http://x"));
300+
expect(res).toEqual({
301+
params: Object.create(null),
302+
item: null,
303+
methodMatch: false,
304+
pattern: "/foo",
305+
});
306+
});
307+
308+
Deno.test("UrlPatternRouter - non-standard method on dynamic route", () => {
309+
const router = new UrlPatternRouter();
310+
const A = () => {};
311+
router.add("GET", "/books/:id", A);
312+
313+
// deno-lint-ignore no-explicit-any
314+
const res = router.match("PROPFIND" as any, new URL("/books/1", "http://x"));
315+
expect(res).toEqual({
316+
params: Object.create(null),
317+
item: null,
318+
methodMatch: false,
319+
pattern: "/books/:id",
320+
});
321+
});

0 commit comments

Comments
 (0)