Improve content type
We have a number of specialized content-type functions for varying
levels of degree, and most of them rely on quite a few strings. This
commit changes them to consolidate on two APIs.
isContentTypeSupported, which as the name implies, takes a single
content type, and returns a bool about whether or not that content type
is allowed.
getPreferedContentType, which takes an array of multiple options, and
fine the first one in the list that matches the clients expected string.
These two functions makes these functions more able to be reused in the
future, and don't require specialized entries for each possible type or
combination of types that we need to check for.
Tested: Unit tests passing. Pretty good coverage.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8b976d0cefec5f24e62fbbfae33d12cc803cb373
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 6494586..376eeec 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -454,12 +454,21 @@
}
if (res.body().empty() && !res.jsonValue.empty())
{
- if (http_helpers::requestPrefersHtml(req->getHeaderValue("Accept")))
+ using http_helpers::ContentType;
+ std::array<ContentType, 2> allowed{ContentType::HTML,
+ ContentType::JSON};
+ ContentType prefered =
+ getPreferedContentType(req->getHeaderValue("Accept"), allowed);
+
+ if (prefered == ContentType::HTML)
{
prettyPrintJson(res);
}
else
{
+ // Technically prefered could also be NoMatch here, but we'd
+ // like to default to something rather than return 400 for
+ // backward compatibility.
res.addHeader(boost::beast::http::field::content_type,
"application/json");
res.body() = res.jsonValue.dump(
diff --git a/include/forward_unauthorized.hpp b/include/forward_unauthorized.hpp
index a48b775..75f2dae 100644
--- a/include/forward_unauthorized.hpp
+++ b/include/forward_unauthorized.hpp
@@ -14,7 +14,8 @@
{
// If it's a browser connecting, don't send the HTTP authenticate
// header, to avoid possible CSRF attacks with basic auth
- if (http_helpers::requestPrefersHtml(accept))
+ if (http_helpers::isContentTypeAllowed(accept,
+ http_helpers::ContentType::HTML))
{
// 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 b7844ba..f091760 100644
--- a/include/http_utility.hpp
+++ b/include/http_utility.hpp
@@ -2,53 +2,63 @@
#include <boost/algorithm/string/classification.hpp>
#include <boost/algorithm/string/constants.hpp>
-#include <boost/algorithm/string/split.hpp>
#include <boost/iterator/iterator_facade.hpp>
#include <boost/type_index/type_index_facade.hpp>
#include <cctype>
#include <iomanip>
#include <ostream>
+#include <span>
#include <string>
#include <string_view>
#include <vector>
// IWYU pragma: no_include <ctype.h>
-// IWYU pragma: no_include <boost/algorithm/string/detail/classification.hpp>
namespace http_helpers
{
-inline std::vector<std::string> parseAccept(std::string_view header)
-{
- std::vector<std::string> encodings;
- // chrome currently sends 6 accepts headers, firefox sends 4.
- encodings.reserve(6);
- boost::split(encodings, header, boost::is_any_of(", "),
- boost::token_compress_on);
- return encodings;
-}
-
-inline bool requestPrefersHtml(std::string_view header)
+enum class ContentType
{
- for (const std::string& encoding : parseAccept(header))
+ NoMatch,
+ CBOR,
+ HTML,
+ JSON,
+ OctetStream,
+};
+
+struct ContentTypePair
+{
+ std::string_view contentTypeString;
+ ContentType contentTypeEnum;
+};
+
+constexpr std::array<ContentTypePair, 4> contentTypes{{
+ {"application/cbor", ContentType::CBOR},
+ {"application/json", ContentType::JSON},
+ {"application/octet-stream", ContentType::OctetStream},
+ {"text/html", ContentType::HTML},
+}};
+
+inline ContentType getPreferedContentType(std::string_view header,
+ std::span<ContentType> preferedOrder)
+{
+ size_t index = 0;
+ size_t lastIndex = 0;
+ while (lastIndex < header.size() + 1)
{
- if (encoding == "text/html")
+ index = header.find(',', lastIndex);
+ if (index == std::string_view::npos)
{
- return true;
+ index = header.size();
}
- if (encoding == "application/json")
- {
- return false;
- }
- }
- return false;
-}
+ std::string_view encoding = header.substr(lastIndex, index);
-inline bool isOctetAccepted(std::string_view header)
-{
- for (std::string_view encoding : parseAccept(header))
- {
+ if (!header.empty())
+ {
+ header.remove_prefix(1);
+ }
+ lastIndex = index + 1;
// ignore any q-factor weighting (;q=)
std::size_t separator = encoding.find(";q=");
@@ -56,12 +66,42 @@
{
encoding = encoding.substr(0, separator);
}
- if (encoding == "*/*" || encoding == "application/octet-stream")
+ // If the client allows any encoding, given them the first one on the
+ // servers list
+ if (encoding == "*/*")
{
- return true;
+ if (!preferedOrder.empty())
+ {
+ return preferedOrder[0];
+ }
}
+ const auto* knownContentType =
+ std::find_if(contentTypes.begin(), contentTypes.end(),
+ [encoding](const ContentTypePair& pair) {
+ return pair.contentTypeString == encoding;
+ });
+
+ if (knownContentType == contentTypes.end())
+ {
+ // not able to find content type in list
+ continue;
+ }
+
+ // Not one of the types requested
+ if (std::find(preferedOrder.begin(), preferedOrder.end(),
+ knownContentType->contentTypeEnum) == preferedOrder.end())
+ {
+ continue;
+ }
+ return knownContentType->contentTypeEnum;
}
- return false;
+ return ContentType::NoMatch;
+}
+
+inline bool isContentTypeAllowed(std::string_view header, ContentType type)
+{
+ auto types = std::to_array({type});
+ return getPreferedContentType(header, types) == type;
}
inline std::string urlEncode(const std::string_view value)
diff --git a/include/ut/http_utility_test.cpp b/include/ut/http_utility_test.cpp
index 1a33fd8..d1df6d1 100644
--- a/include/ut/http_utility_test.cpp
+++ b/include/ut/http_utility_test.cpp
@@ -11,37 +11,72 @@
namespace
{
-TEST(RequestPrefersHtml, ContainsHtmlReturnsTrue)
+TEST(isContentTypeAllowed, PositiveTest)
{
- EXPECT_TRUE(requestPrefersHtml("text/html, application/json"));
+ EXPECT_TRUE(isContentTypeAllowed("*/*, application/octet-stream",
+ ContentType::OctetStream));
+ 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));
+ EXPECT_TRUE(
+ isContentTypeAllowed("application/json, text/html", ContentType::HTML));
}
-TEST(RequestPrefersHtml, NoHtmlReturnsFalse)
+TEST(isContentTypeAllowed, NegativeTest)
{
- EXPECT_FALSE(requestPrefersHtml("*/*, application/octet-stream"));
- EXPECT_FALSE(requestPrefersHtml("application/json"));
+ 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));
+ EXPECT_FALSE(isContentTypeAllowed("application/json, text/html",
+ ContentType::OctetStream));
}
-TEST(IsOctetAccepted, ContainsOctetReturnsTrue)
+TEST(isContentTypeAllowed, ContainsAnyMimeTypeReturnsTrue)
{
- EXPECT_TRUE(isOctetAccepted("*/*, application/octet-stream"));
+ EXPECT_TRUE(
+ isContentTypeAllowed("text/html, */*", ContentType::OctetStream));
}
-TEST(IsOctetAccepted, ContainsAnyMimeTypeReturnsTrue)
+TEST(isContentTypeAllowed, ContainsQFactorWeightingReturnsTrue)
{
- EXPECT_TRUE(http_helpers::isOctetAccepted("text/html, */*"));
+ EXPECT_TRUE(
+ isContentTypeAllowed("text/html, */*;q=0.8", ContentType::OctetStream));
}
-TEST(IsOctetAccepted, ContainsQFactorWeightingReturnsTrue)
+TEST(getPreferedContentType, PositiveTest)
{
- EXPECT_TRUE(http_helpers::isOctetAccepted("text/html, */*;q=0.8"));
+ std::array<ContentType, 1> contentType{ContentType::HTML};
+ EXPECT_EQ(
+ getPreferedContentType("text/html, application/json", contentType),
+ ContentType::HTML);
+
+ std::array<ContentType, 2> htmlJson{ContentType::HTML, ContentType::JSON};
+ EXPECT_EQ(getPreferedContentType("text/html, application/json", htmlJson),
+ ContentType::HTML);
+
+ std::array<ContentType, 2> jsonHtml{ContentType::JSON, ContentType::HTML};
+ EXPECT_EQ(getPreferedContentType("text/html, application/json", jsonHtml),
+ ContentType::HTML);
+
+ std::array<ContentType, 2> cborJson{ContentType::CBOR, ContentType::JSON};
+ EXPECT_EQ(
+ getPreferedContentType("application/cbor, application::json", cborJson),
+ ContentType::CBOR);
+
+ EXPECT_EQ(getPreferedContentType("application/json", cborJson),
+ ContentType::JSON);
}
-TEST(IsOctetAccepted, NoOctetReturnsFalse)
+TEST(getPreferedContentType, NegativeTest)
{
- EXPECT_FALSE(isOctetAccepted("text/html, application/json"));
- EXPECT_FALSE(isOctetAccepted("application/json"));
+ std::array<ContentType, 1> contentType{ContentType::CBOR};
+ EXPECT_EQ(
+ getPreferedContentType("text/html, application/json", contentType),
+ ContentType::NoMatch);
}
-
} // namespace
} // namespace http_helpers
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 1b52d00..2a26ab9 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -1673,7 +1673,9 @@
{
return;
}
- if (!http_helpers::isOctetAccepted(req.getHeaderValue("Accept")))
+ if (http_helpers::isContentTypeAllowed(
+ req.getHeaderValue("Accept"),
+ http_helpers::ContentType::OctetStream))
{
asyncResp->res.result(boost::beast::http::status::bad_request);
return;
@@ -3562,7 +3564,9 @@
{
return;
}
- if (!http_helpers::isOctetAccepted(req.getHeaderValue("Accept")))
+ if (http_helpers::isContentTypeAllowed(
+ req.getHeaderValue("Accept"),
+ http_helpers::ContentType::OctetStream))
{
asyncResp->res.result(boost::beast::http::status::bad_request);
return;