Move http client to URL
Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606
It was found that splitting out the URI into encoded pieces in the early
phase removed some information we needed, namely whether or not a URI
was ipv6. This commit changes http client such that it passes all the
information through, with the correct type, rather than passing in
hostname, port, path, and ssl separately.
Opportunistically, because a number of log lines are changing, this uses
the opportunity to remove a number of calls to std::to_string, and rely
on std::format instead.
Now that we no longer use custom URI splitting code, the
ValidateAndSplitUrl() method can be removed, given that our validation
now happens in the URI class.
Tested: Aggregation works properly when satellite URIs are queried.
Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7
Signed-off-by: Ed Tanous <edtanous@google.com>
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 67f1800..ed02c40 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -356,13 +356,10 @@
Subscription(Subscription&&) = delete;
Subscription& operator=(Subscription&&) = delete;
- Subscription(const std::string& inHost, uint16_t inPort,
- const std::string& inPath, const std::string& inUriProto,
- boost::asio::io_context& ioc) :
- host(inHost),
- port(inPort), policy(std::make_shared<crow::ConnectionPolicy>()),
- path(inPath), uriProto(inUriProto)
+ Subscription(boost::urls::url_view url, boost::asio::io_context& ioc) :
+ policy(std::make_shared<crow::ConnectionPolicy>())
{
+ destinationUrl = url;
client.emplace(ioc, policy);
// Subscription constructor
policy->invalidResp = retryRespHandler;
@@ -384,12 +381,11 @@
return false;
}
- bool useSSL = (uriProto == "https");
// A connection pool will be created if one does not already exist
if (client)
{
- client->sendData(std::move(msg), host, port, path, useSSL,
- httpHeaders, boost::beast::http::verb::post);
+ client->sendData(std::move(msg), destinationUrl, httpHeaders,
+ boost::beast::http::verb::post);
return true;
}
@@ -565,8 +561,7 @@
private:
std::string subId;
uint64_t eventSeqNum = 1;
- std::string host;
- uint16_t port = 0;
+ boost::urls::url host;
std::shared_ptr<crow::ConnectionPolicy> policy;
crow::sse_socket::Connection* sseConn = nullptr;
std::optional<crow::HttpClient> client;
@@ -647,21 +642,17 @@
std::shared_ptr<persistent_data::UserSubscription> newSub =
it.second;
- std::string host;
- std::string urlProto;
- uint16_t port = 0;
- std::string path;
- bool status = crow::utility::validateAndSplitUrl(
- newSub->destinationUrl, urlProto, host, port, path);
+ boost::urls::result<boost::urls::url> url =
+ boost::urls::parse_absolute_uri(newSub->destinationUrl);
- if (!status)
+ if (!url)
{
BMCWEB_LOG_ERROR(
"Failed to validate and split destination url");
continue;
}
std::shared_ptr<Subscription> subValue =
- std::make_shared<Subscription>(host, port, path, urlProto, ioc);
+ std::make_shared<Subscription>(*url, ioc);
subValue->id = newSub->id;
subValue->destinationUrl = newSub->destinationUrl;
@@ -1012,20 +1003,6 @@
return idList;
}
- bool isDestinationExist(const std::string& destUrl) const
- {
- for (const auto& it : subscriptionsMap)
- {
- std::shared_ptr<Subscription> entry = it.second;
- if (entry->destinationUrl == destUrl)
- {
- BMCWEB_LOG_ERROR("Destination exist already{}", destUrl);
- return true;
- }
- }
- return false;
- }
-
bool sendTestEventLog()
{
for (const auto& it : subscriptionsMap)
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 2514fae..e5ad88a 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -712,8 +712,9 @@
}
// We need to strip the prefix from the request's path
- std::string targetURI(thisReq.target());
- size_t pos = targetURI.find(prefix + "_");
+ boost::urls::url targetURI(thisReq.target());
+ std::string path = thisReq.url().path();
+ size_t pos = path.find(prefix + "_");
if (pos == std::string::npos)
{
// If this fails then something went wrong
@@ -722,16 +723,20 @@
messages::internalError(asyncResp->res);
return;
}
- targetURI.erase(pos, prefix.size() + 1);
+ path.erase(pos, prefix.size() + 1);
std::function<void(crow::Response&)> cb =
std::bind_front(processResponse, prefix, asyncResp);
std::string data = thisReq.req.body();
- client.sendDataWithCallback(
- std::move(data), std::string(sat->second.host()),
- sat->second.port_number(), targetURI, false /*useSSL*/,
- thisReq.fields(), thisReq.method(), cb);
+ boost::urls::url url(sat->second);
+ url.set_path(path);
+ if (targetURI.has_query())
+ {
+ url.set_query(targetURI.query());
+ }
+ client.sendDataWithCallback(std::move(data), url, thisReq.fields(),
+ thisReq.method(), cb);
}
// Forward a request for a collection URI to each known satellite BMC
@@ -745,12 +750,15 @@
std::function<void(crow::Response&)> cb = std::bind_front(
processCollectionResponse, sat.first, asyncResp);
- std::string targetURI(thisReq.target());
+ boost::urls::url url(sat.second);
+ url.set_path(thisReq.url().path());
+ if (thisReq.url().has_query())
+ {
+ url.set_query(thisReq.url().query());
+ }
std::string data = thisReq.req.body();
- client.sendDataWithCallback(
- std::move(data), std::string(sat.second.host()),
- sat.second.port_number(), targetURI, false /*useSSL*/,
- thisReq.fields(), thisReq.method(), cb);
+ client.sendDataWithCallback(std::move(data), url, thisReq.fields(),
+ thisReq.method(), cb);
}
}
@@ -770,12 +778,13 @@
// is not already supported by the aggregating BMC
// TODO: Improve the processing so that we don't have to strip query
// params in this specific case
- std::string targetURI(thisReq.url().path());
+ boost::urls::url url(sat.second);
+ url.set_path(thisReq.url().path());
+
std::string data = thisReq.req.body();
- client.sendDataWithCallback(
- std::move(data), std::string(sat.second.host()),
- sat.second.port_number(), targetURI, false /*useSSL*/,
- thisReq.fields(), thisReq.method(), cb);
+
+ client.sendDataWithCallback(std::move(data), url, thisReq.fields(),
+ thisReq.method(), cb);
}
}
diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp
index b49e35a..89c2337 100644
--- a/redfish-core/lib/event_service.hpp
+++ b/redfish-core/lib/event_service.hpp
@@ -24,6 +24,7 @@
#include <boost/beast/http/fields.hpp>
#include <boost/system/error_code.hpp>
+#include <boost/url/parse.hpp>
#include <sdbusplus/unpack_properties.hpp>
#include <utils/dbus_utils.hpp>
@@ -317,19 +318,30 @@
}
}
- std::string host;
- std::string urlProto;
- uint16_t port = 0;
- std::string path;
-
- if (!crow::utility::validateAndSplitUrl(destUrl, urlProto, host, port,
- path))
+ boost::urls::result<boost::urls::url> url =
+ boost::urls::parse_absolute_uri(destUrl);
+ if (!url)
{
BMCWEB_LOG_WARNING("Failed to validate and split destination url");
messages::propertyValueFormatError(asyncResp->res, destUrl,
"Destination");
return;
}
+ url->normalize();
+ crow::utility::setProtocolDefaults(*url, protocol);
+ crow::utility::setPortDefaults(*url);
+
+ if (url->path().empty())
+ {
+ url->set_path("/");
+ }
+
+ if (url->has_userinfo())
+ {
+ messages::propertyValueFormatError(asyncResp->res, destUrl,
+ "Destination");
+ return;
+ }
if (protocol == "SNMPv2c")
{
@@ -381,20 +393,22 @@
asyncResp->res, "MetricReportDefinitions", "Protocol");
return;
}
+ if (url->scheme() != "snmp")
+ {
+ messages::propertyValueConflict(asyncResp->res, "Destination",
+ "Protocol");
+ return;
+ }
- addSnmpTrapClient(asyncResp, host, port);
+ addSnmpTrapClient(asyncResp, url->host_address(),
+ url->port_number());
return;
}
- if (path.empty())
- {
- path = "/";
- }
+ std::shared_ptr<Subscription> subValue =
+ std::make_shared<Subscription>(*url, app.ioContext());
- std::shared_ptr<Subscription> subValue = std::make_shared<Subscription>(
- host, port, path, urlProto, app.ioContext());
-
- subValue->destinationUrl = destUrl;
+ subValue->destinationUrl = std::move(*url);
if (subscriptionType)
{