Handle HEAD and Allow headers per the spec
The Redfish specification calls out that the Allow header should be
returned for all resources to give a client an indication of what
actions are allowed on that resource. The router internally has all
this data, so this patchset allows the router to construct an allow
header value, as well as return early on a HEAD request.
This was reverted once here:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/53637
Due to a redfish validator failure. With the previous patches
workaround, this error has now been resolved.
Tested:
Called curl with various parameters and observed the Allow header
curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url
HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST
HEAD /redfish/v1 returned Allow: GET
HEAD /redfish/v1/SessionService returned Allow: GET, PATCH
POST /redfish/v1 returned Allow: GET (method not allowed)
GET /redfish/v1 returned Allow: GET
GET /redfish/v1/SessionService returned Allow: GET, PATCH
Redfish-Protocol-Validator now reports more tests passing.
Prior to this patch:
Pass: 255, Warning: 0, Fail: 27, Not tested: 45
After this patch:
Pass: 262, Warning: 0, Fail: 21, Not tested: 43
Diff: 7 more tests passing
All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and
RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing
Included unit tests passing.
Redfish service validator is now passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ibd52a7c2babe19020a0e27fa1ac79a9d33463f25
diff --git a/http/routing.hpp b/http/routing.hpp
index 5f29761..7bac283 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -12,6 +12,7 @@
#include "websocket.hpp"
#include <async_resp.hpp>
+#include <boost/beast/ssl/ssl_stream.hpp>
#include <boost/container/flat_map.hpp>
#include <cerrno>
@@ -1208,6 +1209,30 @@
}
}
+ std::string buildAllowHeader(Request& req)
+ {
+ std::string allowHeader;
+ // Check to see if this url exists at any verb
+ for (size_t perMethodIndex = 0; perMethodIndex < perMethods.size();
+ perMethodIndex++)
+ {
+ const PerMethod& p = perMethods[perMethodIndex];
+ const std::pair<unsigned, RoutingParams>& found2 =
+ p.trie.find(req.url);
+ if (found2.first == 0)
+ {
+ continue;
+ }
+ if (!allowHeader.empty())
+ {
+ allowHeader += ", ";
+ }
+ allowHeader += boost::beast::http::to_string(
+ static_cast<boost::beast::http::verb>(perMethodIndex));
+ }
+ return allowHeader;
+ }
+
void handle(Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
@@ -1219,25 +1244,31 @@
PerMethod& perMethod = perMethods[static_cast<size_t>(req.method())];
Trie& trie = perMethod.trie;
std::vector<BaseRule*>& rules = perMethod.rules;
+ std::string allowHeader = buildAllowHeader(req);
+ if (!allowHeader.empty())
+ {
+ asyncResp->res.addHeader(boost::beast::http::field::allow,
+ allowHeader);
+
+ // If this is a header request, we're done.
+ if (req.method() == boost::beast::http::verb::head)
+ {
+ return;
+ }
+ }
const std::pair<unsigned, RoutingParams>& found = trie.find(req.url);
unsigned ruleIndex = found.first;
-
if (ruleIndex == 0U)
{
- // Check to see if this url exists at any verb
- for (const PerMethod& p : perMethods)
+ if (!allowHeader.empty())
{
- const std::pair<unsigned, RoutingParams>& found2 =
- p.trie.find(req.url);
- if (found2.first > 0)
- {
- asyncResp->res.result(
- boost::beast::http::status::method_not_allowed);
- return;
- }
+ asyncResp->res.result(
+ boost::beast::http::status::method_not_allowed);
+ return;
}
+
BMCWEB_LOG_DEBUG << "Cannot match rules " << req.url;
asyncResp->res.result(boost::beast::http::status::not_found);
return;
diff --git a/http/ut/router_test.cpp b/http/ut/router_test.cpp
new file mode 100644
index 0000000..da0f72c
--- /dev/null
+++ b/http/ut/router_test.cpp
@@ -0,0 +1,37 @@
+#include "http_request.hpp"
+#include "routing.hpp"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+TEST(Router, AllowHeader)
+{
+ // Callback handler that does nothing
+ auto nullCallback = [](const crow::Request&,
+ const std::shared_ptr<bmcweb::AsyncResp>&) {};
+
+ crow::Router router;
+ std::error_code ec;
+
+ constexpr const std::string_view url = "/foo";
+
+ crow::Request req{{boost::beast::http::verb::get, url, 11}, ec};
+
+ // No route should return no methods.
+ router.validate();
+ EXPECT_EQ(router.buildAllowHeader(req), "");
+
+ router
+ .newRuleTagged<crow::black_magic::getParameterTag(url)>(
+ std::string(url))
+ .methods(boost::beast::http::verb::get)(nullCallback);
+ router.validate();
+ EXPECT_EQ(router.buildAllowHeader(req), "GET");
+
+ router
+ .newRuleTagged<crow::black_magic::getParameterTag(url)>(
+ std::string(url))
+ .methods(boost::beast::http::verb::patch)(nullCallback);
+ router.validate();
+ EXPECT_EQ(router.buildAllowHeader(req), "GET, PATCH");
+}
\ No newline at end of file