Improve HttpHeaders in EventService
This commit moves the internal data structures to use
boost::beast::http::fields as its internal data structure. fields is a
hyper-optimized map implementation for http headers, and has a lot of
nice escaping properties. It is what boost::beast::http::request uses
under the covers, so this has some niceties in reducing the amount of
code, and means we can completely remove the headers structure, and
simply rely on req. When this conversion was done, now the type safety
of the incoming data needs to have better checking, as loading into the
keys has new requirements (like values must be strings), so that type
conversion code for to and from json was added, and the POST and PATCH
handler updated to put into the new structure.
Tested:
curl -vvvv --insecure -u root:0penBmc
"https://192.168.7.2:443/redfish/v1/EventService/Subscriptions" -X POST
-d
"{\"Destination\":\"http://192.168.7.2:443/\",\"Context\":\"Public\",\"Protocol\":\"Redfish\",\"HttpHeaders\":[{\"Foo\":\"Bar\"}]}"
returned 200.
Tested various "bad" headers, and observed the correct type errors.
Issued: systemctl restart bmcweb.
Subscription restored properly verified with.
GET https://localhost:8001/redfish/v1/EventService/Subscriptions/183211400
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I331f65e1a3960f1812c9baac27dbdcb1d54f112c
diff --git a/http/http_client.hpp b/http/http_client.hpp
index ab20eb0..53040ae 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -68,7 +68,6 @@
std::optional<
boost::beast::http::response_parser<boost::beast::http::string_body>>
parser;
- std::vector<std::pair<std::string, std::string>> headers;
boost::circular_buffer_space_optimized<std::string> requestDataQueue{};
ConnState state;
@@ -141,11 +140,6 @@
req.target(uri);
req.method(boost::beast::http::verb::post);
- // Set headers
- for (const auto& [key, value] : headers)
- {
- req.set(key, value);
- }
req.set(boost::beast::http::field::host, host);
req.keep_alive(true);
@@ -425,10 +419,9 @@
return;
}
- void setHeaders(
- const std::vector<std::pair<std::string, std::string>>& httpHeaders)
+ void setHeaders(const boost::beast::http::fields& httpHeaders)
{
- headers = httpHeaders;
+ req.base() = boost::beast::http::header<true>(httpHeaders);
}
void setRetryConfig(const uint32_t retryAttempts,
diff --git a/include/event_service_store.hpp b/include/event_service_store.hpp
index 345b690..dcc99f1 100644
--- a/include/event_service_store.hpp
+++ b/include/event_service_store.hpp
@@ -1,6 +1,7 @@
#pragma once
#include "logging.hpp"
+#include <boost/beast/http/fields.hpp>
#include <boost/container/flat_map.hpp>
#include <nlohmann/json.hpp>
@@ -19,7 +20,7 @@
std::vector<std::string> registryMsgIds;
std::vector<std::string> registryPrefixes;
std::vector<std::string> resourceTypes;
- std::vector<nlohmann::json> httpHeaders; // key-value pair
+ boost::beast::http::fields httpHeaders;
std::vector<std::string> metricReportDefinitions;
static std::shared_ptr<UserSubscription>
@@ -146,13 +147,15 @@
const auto& obj = element.value();
for (const auto& val : obj.items())
{
- const auto value =
- val.value().get_ptr<const nlohmann::json::object_t*>();
+ const std::string* value =
+ val.value().get_ptr<const std::string*>();
if (value == nullptr)
{
+ BMCWEB_LOG_ERROR << "Failed to parse value for key"
+ << val.key();
continue;
}
- subvalue->httpHeaders.emplace_back(*value);
+ subvalue->httpHeaders.set(val.key(), *value);
}
}
else if (element.key() == "MetricReportDefinitions")
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index 4178d7d..d7230cb 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -1,6 +1,7 @@
#pragma once
#include <app.hpp>
+#include <boost/beast/http/fields.hpp>
#include <boost/container/flat_map.hpp>
#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid_generators.hpp>
@@ -253,20 +254,31 @@
<< "The subscription type is SSE, so skipping.";
continue;
}
+ nlohmann::json::object_t headers;
+ for (const boost::beast::http::fields::value_type& header :
+ subValue->httpHeaders)
+ {
+ // Note, these are technically copies because nlohmann doesn't
+ // support key lookup by std::string_view. At least the
+ // following code can use move
+ // https://github.com/nlohmann/json/issues/1529
+ std::string name(header.name_string());
+ headers[std::move(name)] = header.value();
+ }
+
subscriptions.push_back({
{"Id", subValue->id},
{"Context", subValue->customText},
{"DeliveryRetryPolicy", subValue->retryPolicy},
{"Destination", subValue->destinationUrl},
{"EventFormatType", subValue->eventFormatType},
- {"HttpHeaders", subValue->httpHeaders},
+ {"HttpHeaders", std::move(headers)},
{"MessageIds", subValue->registryMsgIds},
{"Protocol", subValue->protocol},
{"RegistryPrefixes", subValue->registryPrefixes},
{"ResourceTypes", subValue->resourceTypes},
{"SubscriptionType", subValue->subscriptionType},
{"MetricReportDefinitions", subValue->metricReportDefinitions},
-
});
}
persistentFile << data;
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 3f398d7..079b28a 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -400,17 +400,7 @@
{
if (conn != nullptr)
{
- std::vector<std::pair<std::string, std::string>> reqHeaders;
- for (const auto& header : httpHeaders)
- {
- for (const auto& item : header.items())
- {
- std::string key = item.key();
- std::string val = item.value();
- reqHeaders.emplace_back(std::pair(key, val));
- }
- }
- conn->setHeaders(reqHeaders);
+ conn->setHeaders(httpHeaders);
conn->sendData(msg);
this->eventSeqNum++;
}
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index 8609862..b802280 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -17,6 +17,7 @@
#include "event_service_manager.hpp"
#include <app.hpp>
+#include <boost/beast/http/fields.hpp>
#include <registries/privilege_registry.hpp>
namespace redfish
@@ -343,7 +344,22 @@
if (headers)
{
- subValue->httpHeaders = *headers;
+ for (const nlohmann::json& headerChunk : *headers)
+ {
+ for (const auto& item : headerChunk.items())
+ {
+ const std::string* value =
+ item.value().get_ptr<const std::string*>();
+ if (value == nullptr)
+ {
+ messages::propertyValueFormatError(
+ asyncResp->res, item.value().dump(2, true),
+ "HttpHeaders/" + item.key());
+ return;
+ }
+ subValue->httpHeaders.set(item.key(), *value);
+ }
+ }
}
if (regPrefixes)
@@ -577,7 +593,25 @@
if (headers)
{
- subValue->httpHeaders = *headers;
+ boost::beast::http::fields fields;
+ for (const nlohmann::json& headerChunk : *headers)
+ {
+ for (auto& it : headerChunk.items())
+ {
+ const std::string* value =
+ it.value().get_ptr<const std::string*>();
+ if (value == nullptr)
+ {
+ messages::propertyValueFormatError(
+ asyncResp->res,
+ it.value().dump(2, ' ', true),
+ "HttpHeaders/" + it.key());
+ return;
+ }
+ fields.set(it.key(), *value);
+ }
+ }
+ subValue->httpHeaders = fields;
}
if (retryPolicy)