From 759cf1055aaf9be84ea08631578a1f3c712ecc61 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Sun, 31 Jul 2022 16:36:52 -0700 Subject: Add Method Not Allowed (405) handler Similar to the 404 handler, add a 405 handler for registering custom 405 handlers for a given tree. The primary use case is for protocols like redfish that support specific messages for 405 handlers that don't have an empty body. Tested: Unit tests pass. PATCH /redfish/v1 returns 405 Method Not Allowed POST /redfish/v1/Chassis returns 405 Method Not Allowed POST /redfish/v1/Chassis/foo returns 405 Method Not Allowed PATCH /redfish/v1/foo/bar returns 404 Not Found GET /redfish/v1 returns ServiceRoot GET /redfish/v1/Chassis returns Chassis collection Signed-off-by: Ed Tanous Change-Id: Ib0afd23d46bb5b88f89cf1e3f4e0606a48ae47ca Signed-off-by: Carson Labrado --- http/routing.hpp | 84 +++++++++++++++++++++++++++++++------------------ http/ut/router_test.cpp | 32 +++++++++++++++++++ 2 files changed, 85 insertions(+), 31 deletions(-) diff --git a/http/routing.hpp b/http/routing.hpp index 8852b304ab..870a62c9ed 100644 --- a/http/routing.hpp +++ b/http/routing.hpp @@ -39,6 +39,7 @@ static constexpr size_t maxVerbIndex = // to keep the BaseRule as a single bitfield (thus keeping the struct small) // while still having a way to declare a route a "not found" route. static constexpr const size_t notFoundIndex = maxVerbIndex + 1; +static constexpr const size_t methodNotAllowedIndex = notFoundIndex + 1; class BaseRule { @@ -109,7 +110,7 @@ class BaseRule size_t methodsBitfield{ 1 << static_cast(boost::beast::http::verb::get)}; static_assert(std::numeric_limits::digits > - notFoundIndex, + methodNotAllowedIndex, "Not enough bits to store bitfield"); std::vector privilegesSet; @@ -464,6 +465,13 @@ struct RuleParameterTraits return *self; } + self_t& methodNotAllowed() + { + self_t* self = static_cast(this); + self->methodsBitfield = 1U << methodNotAllowedIndex; + return *self; + } + self_t& privileges( const std::initializer_list>& p) { @@ -1120,7 +1128,7 @@ class Router { return; } - for (size_t method = 0, methodBit = 1; method <= notFoundIndex; + for (size_t method = 0, methodBit = 1; method <= methodNotAllowedIndex; method++, methodBit <<= 1) { if ((ruleObject->methodsBitfield & methodBit) > 0U) @@ -1175,7 +1183,30 @@ class Router FindRoute route; }; - FindRouteResponse findRoute(const Request& req) const + FindRoute findRouteByIndex(std::string_view url, size_t index) const + { + FindRoute route; + if (index >= perMethods.size()) + { + BMCWEB_LOG_CRITICAL << "Bad index???"; + return route; + } + const PerMethod& perMethod = perMethods[index]; + std::pair found = perMethod.trie.find(url); + if (found.first >= perMethod.rules.size()) + { + throw std::runtime_error("Trie internal structure corrupted!"); + } + // Found a 404 route, switch that in + if (found.first != 0U) + { + route.rule = perMethod.rules[found.first]; + route.params = std::move(found.second); + } + return route; + } + + FindRouteResponse findRoute(Request& req) const { FindRouteResponse findRoute; @@ -1187,11 +1218,8 @@ class Router // Make sure it's safe to deference the array at that index static_assert(maxVerbIndex < std::tuple_size_v); - - const PerMethod& perMethod = perMethods[perMethodIndex]; - std::pair found2 = - perMethod.trie.find(req.url); - if (found2.first == 0) + FindRoute route = findRouteByIndex(req.url, perMethodIndex); + if (route.rule == nullptr) { continue; } @@ -1203,8 +1231,7 @@ class Router static_cast(perMethodIndex)); if (perMethodIndex == reqMethodIndex) { - findRoute.route.rule = perMethod.rules[found2.first]; - findRoute.route.params = std::move(found2.second); + findRoute.route = route; } } return findRoute; @@ -1290,31 +1317,26 @@ class Router FindRouteResponse foundRoute = findRoute(req); - // Couldn't find a normal route with any verb, try looking for a 404 - // route - if (foundRoute.allowHeader.empty()) + if (foundRoute.route.rule == nullptr) { - if (foundRoute.route.rule == nullptr) + // Couldn't find a normal route with any verb, try looking for a 404 + // route + if (foundRoute.allowHeader.empty()) { - const PerMethod& perMethod = perMethods[notFoundIndex]; - std::pair found = - perMethod.trie.find(req.url); - if (found.first >= perMethod.rules.size()) - { - throw std::runtime_error( - "Trie internal structure corrupted!"); - } - // Found a 404 route, switch that in - if (found.first != 0U) - { - foundRoute.route.rule = perMethod.rules[found.first]; - foundRoute.route.params = std::move(found.second); - } + foundRoute.route = findRouteByIndex(req.url, notFoundIndex); + } + else + { + // See if we have a method not allowed (405) handler + foundRoute.route = + findRouteByIndex(req.url, methodNotAllowedIndex); } } - else + + // Fill in the allow header if it's valid + if (!foundRoute.allowHeader.empty()) { - // Found at least one valid route, fill in the allow header + asyncResp->res.addHeader(boost::beast::http::field::allow, foundRoute.allowHeader); } @@ -1491,7 +1513,7 @@ class Router {} }; - std::array perMethods; + std::array perMethods; std::vector> allRules; }; } // namespace crow diff --git a/http/ut/router_test.cpp b/http/ut/router_test.cpp index b43b65944b..9b5d9bec98 100644 --- a/http/ut/router_test.cpp +++ b/http/ut/router_test.cpp @@ -90,5 +90,37 @@ TEST(Router, 404) } EXPECT_TRUE(notFoundCalled); } + +TEST(Router, 405) +{ + // Callback handler that does nothing + auto nullCallback = [](const Request&, + const std::shared_ptr&) {}; + bool called = false; + auto notAllowedCallback = + [&called](const Request&, const std::shared_ptr&) { + called = true; + }; + + Router router; + std::error_code ec; + + constexpr const std::string_view url = "/foo/bar"; + + Request req{{boost::beast::http::verb::patch, url, 11}, ec}; + + router.newRuleTagged(std::string(url)) + .methods(boost::beast::http::verb::get)(nullCallback); + router.newRuleTagged("/foo/") + .methodNotAllowed()(notAllowedCallback); + router.validate(); + { + std::shared_ptr asyncResp = + std::make_shared(); + + router.handle(req, asyncResp); + } + EXPECT_TRUE(called); +} } // namespace } // namespace crow -- cgit v1.2.3