diff options
author | Ed Tanous <edtanous@google.com> | 2022-09-22 01:28:04 +0300 |
---|---|---|
committer | Gunnar Mills <gmills@us.ibm.com> | 2022-09-22 23:37:39 +0300 |
commit | 4a0e1a0cf378e7bf4909c2ea8fc6e77e0d77ca6d (patch) | |
tree | 30e03e09b078d0bb2d1f6812852beb75f0569939 | |
parent | 656472d942f46194bcd6f59c6eca4658fee20c71 (diff) | |
download | bmcweb-4a0e1a0cf378e7bf4909c2ea8fc6e77e0d77ca6d.tar.xz |
Fix content-type return behavior for */*
An HTTP header of Accepts: */* throws a big wrench into our
implementation for a couple reasons. First, because it's the default in
a lot of commonly-used libraries, and second, because clients use it
when they certainly don't mean what the specification says it should
mean "ie, I accept ANY type".
This commit tries to address some of that, by making an explicit option
for content-type="ANY" and pushes it to the individual callers to handle
explicitly as if it were yet another type. In most protocols, there's a
"most common" representation, so protocols are free to use that, or to
explicitly handle it, and require that the user be explicit.
Tested:
Redfish Protocol Validator no longer locks up. (TBD, getting bugs filed
with protocol validator for this missing Accepts header).
For ServiceRoot
GET /redfish/v1 Accepts: application/json - returns json
GET /redfish/v1 Accepts: */* - returns json
GET /redfish/v1 Accepts: text/html - returns html
GET /redfish/v1 no-accepts header - returns json
Redfish-service-validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae6711ae587115d3e159a48a6fc46a903ed6c403
-rw-r--r-- | include/forward_unauthorized.hpp | 4 | ||||
-rw-r--r-- | include/http_utility.hpp | 17 | ||||
-rw-r--r-- | redfish-core/lib/log_services.hpp | 4 | ||||
-rw-r--r-- | test/include/http_utility_test.cpp | 35 |
4 files changed, 35 insertions, 25 deletions
diff --git a/include/forward_unauthorized.hpp b/include/forward_unauthorized.hpp index 75f2dae20f..2fc3ee45f5 100644 --- a/include/forward_unauthorized.hpp +++ b/include/forward_unauthorized.hpp @@ -14,8 +14,8 @@ inline void sendUnauthorized(std::string_view url, { // If it's a browser connecting, don't send the HTTP authenticate // header, to avoid possible CSRF attacks with basic auth - if (http_helpers::isContentTypeAllowed(accept, - http_helpers::ContentType::HTML)) + if (http_helpers::isContentTypeAllowed( + accept, http_helpers::ContentType::HTML, false /*allowWildcard*/)) { // If we have a webui installed, redirect to that login page if (hasWebuiRoute) diff --git a/include/http_utility.hpp b/include/http_utility.hpp index f091760515..cd1be3e2b1 100644 --- a/include/http_utility.hpp +++ b/include/http_utility.hpp @@ -21,6 +21,7 @@ namespace http_helpers enum class ContentType { NoMatch, + ANY, // Accepts: */* CBOR, HTML, JSON, @@ -70,10 +71,7 @@ inline ContentType getPreferedContentType(std::string_view header, // servers list if (encoding == "*/*") { - if (!preferedOrder.empty()) - { - return preferedOrder[0]; - } + return ContentType::ANY; } const auto* knownContentType = std::find_if(contentTypes.begin(), contentTypes.end(), @@ -98,10 +96,17 @@ inline ContentType getPreferedContentType(std::string_view header, return ContentType::NoMatch; } -inline bool isContentTypeAllowed(std::string_view header, ContentType type) +inline bool isContentTypeAllowed(std::string_view header, ContentType type, + bool allowWildcard) { auto types = std::to_array({type}); - return getPreferedContentType(header, types) == type; + ContentType allowed = getPreferedContentType(header, types); + if (allowed == ContentType::ANY) + { + return allowWildcard; + } + + return type == allowed; } inline std::string urlEncode(const std::string_view value) diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp index ffab320cec..d460794672 100644 --- a/redfish-core/lib/log_services.hpp +++ b/redfish-core/lib/log_services.hpp @@ -1675,7 +1675,7 @@ inline void requestRoutesDBusEventLogEntryDownload(App& app) } if (http_helpers::isContentTypeAllowed( req.getHeaderValue("Accept"), - http_helpers::ContentType::OctetStream)) + http_helpers::ContentType::OctetStream, true)) { asyncResp->res.result(boost::beast::http::status::bad_request); return; @@ -3495,7 +3495,7 @@ inline void requestRoutesPostCodesEntryAdditionalData(App& app) } if (http_helpers::isContentTypeAllowed( req.getHeaderValue("Accept"), - http_helpers::ContentType::OctetStream)) + http_helpers::ContentType::OctetStream, true)) { asyncResp->res.result(boost::beast::http::status::bad_request); return; diff --git a/test/include/http_utility_test.cpp b/test/include/http_utility_test.cpp index d1df6d1087..6878001e6a 100644 --- a/test/include/http_utility_test.cpp +++ b/test/include/http_utility_test.cpp @@ -13,38 +13,42 @@ namespace TEST(isContentTypeAllowed, PositiveTest) { - EXPECT_TRUE(isContentTypeAllowed("*/*, application/octet-stream", - ContentType::OctetStream)); + EXPECT_TRUE(isContentTypeAllowed("*/*", ContentType::HTML, true)); EXPECT_TRUE(isContentTypeAllowed("application/octet-stream", - ContentType::OctetStream)); - EXPECT_TRUE(isContentTypeAllowed("text/html", ContentType::HTML)); - EXPECT_TRUE(isContentTypeAllowed("application/json", ContentType::JSON)); - EXPECT_TRUE(isContentTypeAllowed("application/cbor", ContentType::CBOR)); + ContentType::OctetStream, false)); + EXPECT_TRUE(isContentTypeAllowed("text/html", ContentType::HTML, false)); EXPECT_TRUE( - isContentTypeAllowed("application/json, text/html", ContentType::HTML)); + isContentTypeAllowed("application/json", ContentType::JSON, false)); + EXPECT_TRUE( + isContentTypeAllowed("application/cbor", ContentType::CBOR, false)); + EXPECT_TRUE(isContentTypeAllowed("application/json, text/html", + ContentType::HTML, false)); } TEST(isContentTypeAllowed, NegativeTest) { + EXPECT_FALSE(isContentTypeAllowed("application/octet-stream", + ContentType::HTML, false)); + EXPECT_FALSE( + isContentTypeAllowed("application/html", ContentType::JSON, false)); EXPECT_FALSE( - isContentTypeAllowed("application/octet-stream", ContentType::HTML)); - EXPECT_FALSE(isContentTypeAllowed("application/html", ContentType::JSON)); - EXPECT_FALSE(isContentTypeAllowed("application/json", ContentType::CBOR)); - EXPECT_FALSE(isContentTypeAllowed("application/cbor", ContentType::HTML)); + isContentTypeAllowed("application/json", ContentType::CBOR, false)); + EXPECT_FALSE( + isContentTypeAllowed("application/cbor", ContentType::HTML, false)); EXPECT_FALSE(isContentTypeAllowed("application/json, text/html", - ContentType::OctetStream)); + ContentType::OctetStream, false)); } TEST(isContentTypeAllowed, ContainsAnyMimeTypeReturnsTrue) { EXPECT_TRUE( - isContentTypeAllowed("text/html, */*", ContentType::OctetStream)); + isContentTypeAllowed("text/html, */*", ContentType::OctetStream, true)); } TEST(isContentTypeAllowed, ContainsQFactorWeightingReturnsTrue) { - EXPECT_TRUE( - isContentTypeAllowed("text/html, */*;q=0.8", ContentType::OctetStream)); + EXPECT_TRUE(isContentTypeAllowed("text/html, */*;q=0.8", + ContentType::OctetStream, true)); } TEST(getPreferedContentType, PositiveTest) @@ -69,6 +73,7 @@ TEST(getPreferedContentType, PositiveTest) EXPECT_EQ(getPreferedContentType("application/json", cborJson), ContentType::JSON); + EXPECT_EQ(getPreferedContentType("*/*", cborJson), ContentType::ANY); } TEST(getPreferedContentType, NegativeTest) |