Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/fresh/src/app_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
8 changes: 6 additions & 2 deletions packages/fresh/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
33 changes: 33 additions & 0 deletions packages/fresh/src/router_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
});
});
Loading