Add common url segments parser
This change is adding helper template function, which can be used both
to validate and read segments from segments_view returned by boost_url
parser. Number of segments is also validated - in case when argument
count differs from them, false will be returned. In case when we want to
validate only existence of a segment, special argument can be passed in
its place: 'anySegment'.
Reasoning why url_view was chosen instead of strings:
- This way code generation is kept minimal.
- There are multiple parse functions in boost_url with different rules,
but all of them return url_view. This solution should accommodate
every use case.
Testing done:
- Unit tests are added, passing.
- Refactored part of telemetry to use this new approach, no regression
spotted during simple POST/GET tests.
Change-Id: I677a34e1ee570d33f2322a80dc1629f88273e0d5
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
diff --git a/http/ut/utility_test.cpp b/http/ut/utility_test.cpp
index 2f19fca..88743e0 100644
--- a/http/ut/utility_test.cpp
+++ b/http/ut/utility_test.cpp
@@ -127,6 +127,61 @@
EXPECT_EQ(std::string_view(url.data(), url.size()), "/%2f/bad&tring");
}
+TEST(Utility, readUrlSegments)
+{
+ using crow::utility::readUrlSegments;
+
+ boost::urls::result<boost::urls::url_view> parsed =
+ boost::urls::parse_relative_ref("/redfish/v1/Chassis#/Fans/0/Reading");
+
+ EXPECT_TRUE(readUrlSegments(*parsed, "redfish", "v1", "Chassis"));
+
+ EXPECT_FALSE(readUrlSegments(*parsed, "FOOBAR", "v1", "Chassis"));
+
+ EXPECT_FALSE(readUrlSegments(*parsed, "redfish", "v1"));
+
+ EXPECT_FALSE(
+ readUrlSegments(*parsed, "redfish", "v1", "Chassis", "FOOBAR"));
+
+ std::string out1;
+ std::string out2;
+ std::string out3;
+ EXPECT_TRUE(readUrlSegments(*parsed, "redfish", "v1", std::ref(out1)));
+ EXPECT_EQ(out1, "Chassis");
+
+ out1 = out2 = out3 = "";
+ EXPECT_TRUE(readUrlSegments(*parsed, std::ref(out1), std::ref(out2),
+ std::ref(out3)));
+ EXPECT_EQ(out1, "redfish");
+ EXPECT_EQ(out2, "v1");
+ EXPECT_EQ(out3, "Chassis");
+
+ out1 = out2 = out3 = "";
+ EXPECT_TRUE(readUrlSegments(*parsed, "redfish", std::ref(out1), "Chassis"));
+ EXPECT_EQ(out1, "v1");
+
+ out1 = out2 = out3 = "";
+ EXPECT_TRUE(readUrlSegments(*parsed, std::ref(out1), "v1", std::ref(out2)));
+ EXPECT_EQ(out1, "redfish");
+ EXPECT_EQ(out2, "Chassis");
+
+ EXPECT_FALSE(readUrlSegments(*parsed, "too", "short"));
+
+ EXPECT_FALSE(readUrlSegments(*parsed, "too", "long", "too", "long"));
+
+ EXPECT_FALSE(
+ readUrlSegments(*parsed, std::ref(out1), "v2", std::ref(out2)));
+
+ EXPECT_FALSE(readUrlSegments(*parsed, "redfish", std::ref(out1),
+ std::ref(out2), std::ref(out3)));
+
+ parsed = boost::urls::parse_relative_ref("/absolute/url");
+ EXPECT_TRUE(readUrlSegments(*parsed, "absolute", "url"));
+
+ parsed = boost::urls::parse_relative_ref("not/absolute/url");
+ EXPECT_FALSE(readUrlSegments(*parsed, "not", "absolute", "url"));
+}
+
TEST(Utility, ValidateAndSplitUrlPositive)
{
using crow::utility::validateAndSplitUrl;
@@ -152,8 +207,8 @@
validateAndSplitUrl("https://foo.com/bar", urlProto, host, port, path));
EXPECT_EQ(port, "443");
- // If http push eventing is allowed, allow http, if it's not, parse should
- // fail.
+ // If http push eventing is allowed, allow http, if it's not, parse
+ // should fail.
#ifdef BMCWEB_INSECURE_ENABLE_HTTP_PUSH_STYLE_EVENTING
ASSERT_TRUE(
validateAndSplitUrl("http://foo.com/bar", urlProto, host, port, path));
diff --git a/http/utility.hpp b/http/utility.hpp
index 52a72b6..f7e456b 100644
--- a/http/utility.hpp
+++ b/http/utility.hpp
@@ -18,6 +18,7 @@
#include <tuple>
#include <type_traits>
#include <utility>
+#include <variant>
namespace crow
{
@@ -669,6 +670,69 @@
return details::urlFromPiecesDetail({args...});
}
+namespace details
+{
+
+// std::reference_wrapper<std::string> - extracts segment to variable
+// std::string_view - checks if segment is equal to variable
+using UrlSegment =
+ std::variant<std::reference_wrapper<std::string>, std::string_view>;
+
+class UrlSegmentMatcherVisitor
+{
+ public:
+ bool operator()(std::string& output)
+ {
+ output = std::string_view(segment.data(), segment.size());
+ return true;
+ }
+
+ bool operator()(std::string_view expected)
+ {
+ return std::string_view(segment.data(), segment.size()) == expected;
+ }
+
+ UrlSegmentMatcherVisitor(const boost::urls::string_value& segmentIn) :
+ segment(segmentIn)
+ {}
+
+ private:
+ const boost::urls::string_value& segment;
+};
+
+inline bool readUrlSegments(const boost::urls::url_view& urlView,
+ std::initializer_list<UrlSegment>&& segments)
+{
+ const boost::urls::segments_view& urlSegments = urlView.segments();
+
+ if (!urlSegments.is_absolute() || segments.size() != urlSegments.size())
+ {
+ return false;
+ }
+
+ boost::urls::segments_view::iterator it = urlSegments.begin();
+ boost::urls::segments_view::iterator end = urlSegments.end();
+
+ for (const auto& segment : segments)
+ {
+ if (!std::visit(UrlSegmentMatcherVisitor(*it), segment))
+ {
+ return false;
+ }
+ it++;
+ }
+ return true;
+}
+
+} // namespace details
+
+template <typename... Args>
+inline bool readUrlSegments(const boost::urls::url_view& urlView,
+ Args&&... args)
+{
+ return details::readUrlSegments(urlView, {std::forward<Args>(args)...});
+}
+
inline bool validateAndSplitUrl(std::string_view destUrl, std::string& urlProto,
std::string& host, std::string& port,
std::string& path)
diff --git a/redfish-core/include/utils/telemetry_utils.hpp b/redfish-core/include/utils/telemetry_utils.hpp
index 6930d0a..49e1e12 100644
--- a/redfish-core/include/utils/telemetry_utils.hpp
+++ b/redfish-core/include/utils/telemetry_utils.hpp
@@ -25,5 +25,63 @@
return {triggersPath / id};
}
+struct IncorrectMetricUri
+{
+ std::string uri;
+ size_t index;
+};
+
+inline std::optional<IncorrectMetricUri> getChassisSensorNode(
+ const std::vector<std::string>& uris,
+ boost::container::flat_set<std::pair<std::string, std::string>>& matched)
+{
+ size_t uriIdx = 0;
+ for (const std::string& uri : uris)
+ {
+ boost::urls::result<boost::urls::url_view> parsed =
+ boost::urls::parse_relative_ref(uri);
+
+ if (!parsed)
+ {
+ BMCWEB_LOG_ERROR << "Failed to get chassis and sensor Node "
+ "from "
+ << uri;
+ return std::make_optional<IncorrectMetricUri>({uri, uriIdx});
+ }
+
+ std::string chassis;
+ std::string node;
+
+ if (crow::utility::readUrlSegments(*parsed, "redfish", "v1", "Chassis",
+ std::ref(chassis), std::ref(node)))
+ {
+ matched.emplace(std::move(chassis), std::move(node));
+ uriIdx++;
+ continue;
+ }
+
+ // Those 2 segments cannot be validated here, as we don't know which
+ // sensors exist at the moment of parsing.
+ std::string ignoredSenorId;
+ std::string ignoredSensorUnit;
+
+ if (crow::utility::readUrlSegments(*parsed, "redfish", "v1", "Chassis",
+ std::ref(chassis), "Sensors",
+ std::ref(ignoredSenorId),
+ std::ref(ignoredSensorUnit)))
+ {
+ matched.emplace(std::move(chassis), "Sensors");
+ uriIdx++;
+ continue;
+ }
+
+ BMCWEB_LOG_ERROR << "Failed to get chassis and sensor Node "
+ "from "
+ << uri;
+ return std::make_optional<IncorrectMetricUri>({uri, uriIdx});
+ }
+ return std::nullopt;
+}
+
} // namespace telemetry
} // namespace redfish
diff --git a/redfish-core/lib/metric_report_definition.hpp b/redfish-core/lib/metric_report_definition.hpp
index d61afd9..b01c3bd 100644
--- a/redfish-core/lib/metric_report_definition.hpp
+++ b/redfish-core/lib/metric_report_definition.hpp
@@ -226,38 +226,24 @@
return true;
}
-inline bool getChassisSensorNode(
+inline bool getChassisSensorNodeFromMetrics(
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::vector<std::pair<std::string, std::vector<std::string>>>&
metrics,
boost::container::flat_set<std::pair<std::string, std::string>>& matched)
{
- for (const auto& [id, uris] : metrics)
+ for (const auto& metric : metrics)
{
- for (size_t i = 0; i < uris.size(); i++)
+ const std::vector<std::string>& uris = metric.second;
+
+ std::optional<IncorrectMetricUri> error =
+ getChassisSensorNode(uris, matched);
+ if (error)
{
- const std::string& uri = uris[i];
- std::string chassis;
- std::string node;
-
- if (!boost::starts_with(uri, "/redfish/v1/Chassis/") ||
- !dbus::utility::getNthStringFromPath(uri, 3, chassis) ||
- !dbus::utility::getNthStringFromPath(uri, 4, node))
- {
- BMCWEB_LOG_ERROR
- << "Failed to get chassis and sensor Node from " << uri;
- messages::propertyValueIncorrect(asyncResp->res, uri,
- "MetricProperties/" +
- std::to_string(i));
- return false;
- }
-
- if (boost::ends_with(node, "#"))
- {
- node.pop_back();
- }
-
- matched.emplace(std::move(chassis), std::move(node));
+ messages::propertyValueIncorrect(asyncResp->res, error->uri,
+ "MetricProperties/" +
+ std::to_string(error->index));
+ return false;
}
}
return true;
@@ -408,8 +394,8 @@
boost::container::flat_set<std::pair<std::string, std::string>>
chassisSensors;
- if (!telemetry::getChassisSensorNode(asyncResp, args.metrics,
- chassisSensors))
+ if (!telemetry::getChassisSensorNodeFromMetrics(
+ asyncResp, args.metrics, chassisSensors))
{
return;
}