summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEd Tanous <edtanous@google.com>2022-09-22 01:28:04 +0300
committerGunnar Mills <gmills@us.ibm.com>2022-09-22 23:37:39 +0300
commit4a0e1a0cf378e7bf4909c2ea8fc6e77e0d77ca6d (patch)
tree30e03e09b078d0bb2d1f6812852beb75f0569939
parent656472d942f46194bcd6f59c6eca4658fee20c71 (diff)
downloadbmcweb-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.hpp4
-rw-r--r--include/http_utility.hpp17
-rw-r--r--redfish-core/lib/log_services.hpp4
-rw-r--r--test/include/http_utility_test.cpp35
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)