From 7ce5768372628394caa6e6f6efdf031dfaf5a5a0 Mon Sep 17 00:00:00 2001
From: An Tao <antao2002@gmail.com>
Date: Sat, 6 Feb 2021 15:16:44 +0800
Subject: [PATCH] Return 404 or 405 responses correctly (#705)

---
 examples/simple_example_test/main.cc | 55 +++++++++++++++++++++++++++-
 lib/src/HttpControllersRouter.cc     |  6 ++-
 lib/src/StaticFileRouter.cc          | 29 ++++++++++++---
 3 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/examples/simple_example_test/main.cc b/examples/simple_example_test/main.cc
index 03cbf6da..bab93511 100644
--- a/examples/simple_example_test/main.cc
+++ b/examples/simple_example_test/main.cc
@@ -256,6 +256,7 @@ void doTest(const HttpClientPtr &client,
                                 exit(1);
                             }
                         });
+
     // Post json again
     req = HttpRequest::newHttpJsonRequest(json);
     req->setMethod(drogon::Post);
@@ -283,7 +284,32 @@ void doTest(const HttpClientPtr &client,
                                 exit(1);
                             }
                         });
-
+    // Test 404
+    req = HttpRequest::newHttpJsonRequest(json);
+    req->setMethod(drogon::Get);
+    req->setPath("/api/v1/apitest/notFoundRouting");
+    client->sendRequest(req,
+                        [req, isHttps](ReqResult result,
+                                       const HttpResponsePtr &resp) {
+                            if (result == ReqResult::Ok)
+                            {
+                                if (resp->getStatusCode() == k404NotFound)
+                                {
+                                    outputGood(req, isHttps);
+                                }
+                                else
+                                {
+                                    LOG_DEBUG << resp->getBody();
+                                    LOG_ERROR << "Error!";
+                                    exit(1);
+                                }
+                            }
+                            else
+                            {
+                                LOG_ERROR << "Error!";
+                                exit(1);
+                            }
+                        });
     /// 1 Get /
     req = HttpRequest::newHttpRequest();
     req->setMethod(drogon::Get);
@@ -909,6 +935,33 @@ void doTest(const HttpClientPtr &client,
                                 exit(1);
                             }
                         });
+    // Test 405
+    req = HttpRequest::newHttpRequest();
+    req->setMethod(drogon::Post);
+    req->setPath("/drogon.jpg");
+    client->sendRequest(
+        req,
+        [req, client, isHttps, &pro](ReqResult result,
+                                     const HttpResponsePtr &resp) {
+            if (result == ReqResult::Ok)
+            {
+                if (resp->getStatusCode() == k405MethodNotAllowed)
+                {
+                    outputGood(req, isHttps);
+                }
+                else
+                {
+                    LOG_DEBUG << resp->getBody().length();
+                    LOG_ERROR << "Error!";
+                    exit(1);
+                }
+            }
+            else
+            {
+                LOG_ERROR << "Error!";
+                exit(1);
+            }
+        });
     /// Test file download
     req = HttpRequest::newHttpRequest();
     req->setMethod(drogon::Get);
diff --git a/lib/src/HttpControllersRouter.cc b/lib/src/HttpControllersRouter.cc
index f4c03299..ae7da4ec 100644
--- a/lib/src/HttpControllersRouter.cc
+++ b/lib/src/HttpControllersRouter.cc
@@ -33,8 +33,10 @@ void HttpControllersRouter::doWhenNoHandlerFound(
         !HttpAppFrameworkImpl::instance().getHomePage().empty())
     {
         req->setPath("/" + HttpAppFrameworkImpl::instance().getHomePage());
-        HttpAppFrameworkImpl::instance().forward(req, std::move(callback));
-        return;
+        // Just call the fileRouter_.route instead of forwarding. so comment out
+        // those sentences.
+        // HttpAppFrameworkImpl::instance().forward(req, std::move(callback));
+        // return;
     }
     fileRouter_.route(req, std::move(callback));
 }
diff --git a/lib/src/StaticFileRouter.cc b/lib/src/StaticFileRouter.cc
index f749a277..7427375e 100644
--- a/lib/src/StaticFileRouter.cc
+++ b/lib/src/StaticFileRouter.cc
@@ -65,11 +65,7 @@ void StaticFileRouter::route(
         callback(app().getCustomErrorHandler()(k403Forbidden));
         return;
     }
-    if (req->method() != Get)
-    {
-        callback(app().getCustomErrorHandler()(k405MethodNotAllowed));
-        return;
-    }
+
     auto lPath = path;
     std::transform(lPath.begin(), lPath.end(), lPath.begin(), tolower);
 
@@ -265,6 +261,11 @@ void StaticFileRouter::sendStaticFileResponse(
     {
         if (cachedResp)
         {
+            if (req->method() != Get)
+            {
+                callback(app().getCustomErrorHandler()(k405MethodNotAllowed));
+                return;
+            }
             if (static_cast<HttpResponseImpl *>(cachedResp.get())
                     ->getHeaderBy("last-modified") ==
                 req->getHeaderBy("if-modified-since"))
@@ -288,6 +289,12 @@ void StaticFileRouter::sendStaticFileResponse(
             {
                 fileExists = true;
                 LOG_TRACE << "last modify time:" << fileStat.st_mtime;
+                if (req->method() != Get)
+                {
+                    callback(
+                        app().getCustomErrorHandler()(k405MethodNotAllowed));
+                    return;
+                }
                 struct tm tm1;
 #ifdef _WIN32
                 gmtime_s(&tm1, &fileStat.st_mtime);
@@ -324,6 +331,11 @@ void StaticFileRouter::sendStaticFileResponse(
     }
     if (cachedResp)
     {
+        if (req->method() != Get)
+        {
+            callback(app().getCustomErrorHandler()(k405MethodNotAllowed));
+            return;
+        }
         LOG_TRACE << "Using file cache";
         HttpAppFrameworkImpl::instance().callCallback(req,
                                                       cachedResp,
@@ -340,6 +352,13 @@ void StaticFileRouter::sendStaticFileResponse(
             return;
         }
     }
+
+    if (req->method() != Get)
+    {
+        callback(app().getCustomErrorHandler()(k405MethodNotAllowed));
+        return;
+    }
+
     HttpResponsePtr resp;
     auto &acceptEncoding = req->getHeaderBy("accept-encoding");