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 <edtanous@google.com>
Change-Id: Ib0afd23d46bb5b88f89cf1e3f4e0606a48ae47ca
Signed-off-by: Carson Labrado <clabrado@google.com>
diff --git a/http/routing.hpp b/http/routing.hpp
index 8852b30..870a62c 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -39,6 +39,7 @@
// 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 @@
size_t methodsBitfield{
1 << static_cast<size_t>(boost::beast::http::verb::get)};
static_assert(std::numeric_limits<decltype(methodsBitfield)>::digits >
- notFoundIndex,
+ methodNotAllowedIndex,
"Not enough bits to store bitfield");
std::vector<redfish::Privileges> privilegesSet;
@@ -464,6 +465,13 @@
return *self;
}
+ self_t& methodNotAllowed()
+ {
+ self_t* self = static_cast<self_t*>(this);
+ self->methodsBitfield = 1U << methodNotAllowedIndex;
+ return *self;
+ }
+
self_t& privileges(
const std::initializer_list<std::initializer_list<const char*>>& p)
{
@@ -1120,7 +1128,7 @@
{
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 @@
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<unsigned, RoutingParams> 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 @@
// Make sure it's safe to deference the array at that index
static_assert(maxVerbIndex <
std::tuple_size_v<decltype(perMethods)>);
-
- const PerMethod& perMethod = perMethods[perMethodIndex];
- std::pair<unsigned, RoutingParams> 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 @@
static_cast<boost::beast::http::verb>(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 @@
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<unsigned, RoutingParams> 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 @@
{}
};
- std::array<PerMethod, notFoundIndex + 1> perMethods;
+ std::array<PerMethod, methodNotAllowedIndex + 1> perMethods;
std::vector<std::unique_ptr<BaseRule>> allRules;
};
} // namespace crow
diff --git a/http/ut/router_test.cpp b/http/ut/router_test.cpp
index b43b659..9b5d9be 100644
--- a/http/ut/router_test.cpp
+++ b/http/ut/router_test.cpp
@@ -90,5 +90,37 @@
}
EXPECT_TRUE(notFoundCalled);
}
+
+TEST(Router, 405)
+{
+ // Callback handler that does nothing
+ auto nullCallback = [](const Request&,
+ const std::shared_ptr<bmcweb::AsyncResp>&) {};
+ bool called = false;
+ auto notAllowedCallback =
+ [&called](const Request&, const std::shared_ptr<bmcweb::AsyncResp>&) {
+ 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<getParameterTag(url)>(std::string(url))
+ .methods(boost::beast::http::verb::get)(nullCallback);
+ router.newRuleTagged<getParameterTag(url)>("/foo/<path>")
+ .methodNotAllowed()(notAllowedCallback);
+ router.validate();
+ {
+ std::shared_ptr<bmcweb::AsyncResp> asyncResp =
+ std::make_shared<bmcweb::AsyncResp>();
+
+ router.handle(req, asyncResp);
+ }
+ EXPECT_TRUE(called);
+}
} // namespace
} // namespace crow