Redesign 405 handling to not use notFoundHandler.

mattbishop · Aug 10, 2021 · 84e8cba · 84e8cba
1 parent b0710e0
commit 84e8cba
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 146 deletions.
diff --git a/lib/index.js b/lib/index.js
@@ -25,35 +25,25 @@ function captureRouteMethod(caseSensitive, ignoreTrailingSlash, routeMethods, ma
     routeMethods.set(url, urlMethods);
     matcherMethods.set(urlMatcher, urlMethods);
 }
-function handleNotFound(matcherMethods, request, reply) {
-    const { url, method } = request;
-    const methods = findUrlMethods(matcherMethods, url);
-    let statusCode, message, error;
-    if (methods) {
-        statusCode = 405;
-        message = `${method} ${url} not allowed`;
-        error = "Method Not Allowed";
-        reply.header("allow", methods);
-    }
-    else {
-        statusCode = 404;
-        message = `Route ${method}:${url} not found`;
-        error = "Not Found";
-    }
-    reply
-        .code(statusCode)
-        .send({
-        message,
-        error,
-        statusCode
-    });
-}
-function addAllowHeader(routeMethods, request, reply, done) {
-    // @ts-ignore config does not have url in it's type declaration
-    const { url } = reply.context.config;
-    const methods = routeMethods.get(url);
+function handleRequest(routeMethods, matcherMethods, send405, request, reply, done) {
+    const { url, method, 
+    // @ts-ignore request does not have context in it's type declaration
+    context: { config: { url: path } } } = request;
+    const methods = path
+        ? routeMethods.get(path)
+        : findUrlMethods(matcherMethods, url);
     if (methods) {
         reply.header("allow", methods);
+        if (send405 && !methods.includes(method)) {
+            // send 405
+            reply
+                .code(405)
+                .send({
+                statusCode: 405,
+                message: `${method} ${url} not allowed`,
+                error: "Method Not Allowed"
+            });
+        }
     }
     done();
 }
@@ -65,15 +55,12 @@ function findUrlMethods(matcherMethods, url) {
     }
 }
 function plugin(fastify, opts, done) {
+    const { send405 = true } = opts;
+    const { caseSensitive = true, ignoreTrailingSlash = false } = fastify.initialConfig;
     const routeMethods = new Map();
     const matcherMethods = new Map();
-    const { caseSensitive = true, ignoreTrailingSlash = false } = fastify.initialConfig;
     fastify.addHook("onRoute", (o) => captureRouteMethod(caseSensitive, ignoreTrailingSlash, routeMethods, matcherMethods, o));
-    fastify.addHook("onRequest", (q, p, d) => addAllowHeader(routeMethods, q, p, d));
-    const { send405 = true } = opts;
-    if (send405) {
-        fastify.setNotFoundHandler((q, p) => handleNotFound(matcherMethods, q, p));
-    }
+    fastify.addHook("onRequest", (q, p, d) => handleRequest(routeMethods, matcherMethods, send405, q, p, d));
     done();
 }
 exports.default = fastify_plugin_1.default(plugin, {

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "fastify-allow",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "description": "Fastify plugin that adds an Allow header with all registered methods to GET and HEAD responses. See https://datatracker.ietf.org/doc/html/rfc7231#section-7.4.1",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",

diff --git a/src/index.ts b/src/index.ts
@@ -16,7 +16,7 @@ function captureRouteMethod(caseSensitive:        boolean,
                             routeMethods:         RouteMethodsMap,
                             matcherMethods:       MatcherMethodsMap,
                             routeOptions:         FastifyPluginOptions) {
-  const {method, url} = routeOptions
+  const { method, url } = routeOptions
   const pattern = url.replace(/\/:[^/]+/g, "/[^/]+")
   const flags = caseSensitive ? "" : "i"
   const trailingSlash = ignoreTrailingSlash ? "/?" : ""
@@ -40,43 +40,40 @@ function captureRouteMethod(caseSensitive:        boolean,
 }
 
 
-function handleNotFound(matcherMethods: MatcherMethodsMap,
-                        request:        FastifyRequest,
-                        reply:          FastifyReply) {
-  const {url, method} = request
-  const methods = findUrlMethods(matcherMethods, url)
-  let statusCode, message, error
-
-  if (methods) {
-    statusCode = 405
-    message = `${method} ${url} not allowed`
-    error = "Method Not Allowed"
-    reply.header("allow", methods)
-  } else {
-    statusCode = 404
-    message = `Route ${method}:${url} not found`
-    error = "Not Found"
-  }
-
-  reply
-    .code(statusCode)
-    .send({
-        message,
-        error,
-        statusCode
-      })
-}
+function handleRequest(routeMethods:    RouteMethodsMap,
+                       matcherMethods:  MatcherMethodsMap,
+                       send405:         boolean,
+                       request:         FastifyRequest,
+                       reply:           FastifyReply,
+                       done:            () => void) {
+  const {
+    url,
+    method,
+    // @ts-ignore request does not have context in it's type declaration
+    context: {
+      config: {
+        url: path
+      }
+    }
+  } = request
 
+  const methods = path
+    ? routeMethods.get(path)
+    : findUrlMethods(matcherMethods, url)
 
-function addAllowHeader(routeMethods: RouteMethodsMap,
-                        request:      FastifyRequest,
-                        reply:        FastifyReply,
-                        done:         () => void) {
-  // @ts-ignore config does not have url in it's type declaration
-  const {url} = reply.context.config
-  const methods = routeMethods.get(url)
   if (methods) {
     reply.header("allow", methods)
+
+    if (send405 && !methods.includes(method)) {
+      // send 405
+      reply
+        .code(405)
+        .send({
+          statusCode: 405,
+          message: `${method} ${url} not allowed`,
+          error: "Method Not Allowed"
+        })
+    }
   }
   done()
 }
@@ -95,15 +92,12 @@ function findUrlMethods(matcherMethods: MatcherMethodsMap,
 function plugin(fastify:  FastifyInstance,
                 opts:     AllowOptions,
                 done:     () => void) {
+  const { send405 = true } = opts
+  const { caseSensitive = true, ignoreTrailingSlash = false } = fastify.initialConfig
   const routeMethods = new Map()
   const matcherMethods = new Map()
-  const {caseSensitive = true, ignoreTrailingSlash = false} = fastify.initialConfig
   fastify.addHook("onRoute", (o) => captureRouteMethod(caseSensitive, ignoreTrailingSlash, routeMethods, matcherMethods, o))
-  fastify.addHook("onRequest", (q, p, d) => addAllowHeader(routeMethods, q, p, d))
-  const { send405 = true } = opts
-  if (send405) {
-    fastify.setNotFoundHandler((q, p) => handleNotFound(matcherMethods, q, p))
-  }
+  fastify.addHook("onRequest", (q, p, d) => handleRequest(routeMethods, matcherMethods, send405, q, p, d))
   done()
 }
 

diff --git a/test/test.ts b/test/test.ts
@@ -3,42 +3,6 @@ import Fastify, {FastifyReply, FastifyRequest, LightMyRequestResponse} from "fas
 import allowPlugin, {AllowOptions} from "../src"
 
 
-test("register without options", (t) => {
-  t.plan(2)
-  const app = Fastify()
-  app.setNotFoundHandler = () => {
-    t.pass("notFoundHandler set")
-    return app
-  }
-  app.register(allowPlugin)
-  app.ready(t.error)
-})
-
-test("register with send405 options set to true", (t) => {
-  t.plan(2)
-  const app = Fastify()
-  app.setNotFoundHandler = () => {
-    t.pass("notFoundHandler set")
-    return app
-  }
-  const opts: AllowOptions = {send405: true}
-  app.register(allowPlugin, opts)
-  app.ready(t.error)
-})
-
-test("register with send405 options set to false", (t) => {
-  t.plan(1)
-  const app = Fastify()
-  app.setNotFoundHandler = () => {
-    t.fail("notFoundHandler should not be set")
-    return app
-  }
-  const opts: AllowOptions = {send405: false}
-  app.register(allowPlugin, opts)
-  app.ready(t.error)
-})
-
-
 test("request tests, default fastify options", (testGroup) => {
   const app = Fastify()
   const opts: AllowOptions = {send405: true}