Add filter parameter support
$filter is a parameter documented in the Redfish specification, section
7.3.4. It defines a mechanism for filtering arbitrary collections of
parameters based on a set of arbitrary language expressions.
From the specification, it supports the following language operators:
() Precedence grouping operator.
(Status/State eq 'Enabled' and Status/Health eq 'OK')
or SystemType eq 'Physical'
and Logical and operator.
ProcessorSummary/Count eq 2 and MemorySummary/TotalSystemMemoryGiB gt 64
eq Equal comparison operator.
ProcessorSummary/Count eq 2
ge Greater than or equal to comparison operator.
ProcessorSummary/Count ge 2
gt Great than comparison operator.
ProcessorSummary/Count gt 2
le Less than or equal to comparison operator
MemorySummary/TotalSystemMemoryGiB le 64
lt Less than comparison operator.
MemorySummary/TotalSystemMemoryGiB lt 64
ne Not equal comparison operator.
SystemType ne 'Physical'
not Logical negation operator.
not (ProcessorSummary/Count eq 2)
or Logical or operator.
ProcessorSummary/Count eq 2 or ProcessorSummary/Count eq 4
Support for these operators have been added in previous commits. This
commit enables them behind the insecure-enable-redfish-query meson
option. This is an arbitrary language, so the likelihood there's some
improper implementation in the patch is high. This gives folks the
ability to test it.
Tested:
Lots of unit tests included in this patch.
Functionally tested the basic operators:
```
GET /redfish/v1/Managers/bmc/LogServices/Journal/Entries?\$filter=EntryType+eq+'Oem'
GET /redfish/v1/Managers/bmc/LogServices/Journal/Entries?\$filter=EntryType+ne+'Oem'
```
Function as expected, producing multiple results or no results
respectively.
GET /redfish/v1 reports "FilterQuery": true
Redfish service validator passes.
Change-Id: Id568acc5dcfce868af12da5ee16c4f0caae8060a
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/meson.build b/meson.build
index 6ca861d..63fed9e 100644
--- a/meson.build
+++ b/meson.build
@@ -72,6 +72,7 @@
'-Wformat=2',
'-Wno-c++98-compat-pedantic',
'-Wno-c++98-compat',
+ '-Wno-c++20-extensions',
'-Wno-documentation-unknown-command',
'-Wno-documentation',
'-Wno-exit-time-destructors',
@@ -332,6 +333,7 @@
'redfish-core/src/error_messages.cpp',
# Begin large files, should be at the beginning
+ 'redfish-core/src/filter_expr_executor.cpp',
'redfish-core/src/filter_expr_printer.cpp',
'redfish-core/src/redfish.cpp',
'redfish-core/src/registries.cpp',
@@ -393,6 +395,7 @@
'test/include/ssl_key_handler_test.cpp',
'test/include/str_utility_test.cpp',
'test/redfish-core/include/privileges_test.cpp',
+ 'test/redfish-core/include/filter_expr_executor_test.cpp',
'test/redfish-core/include/filter_expr_parser_test.cpp',
'test/redfish-core/include/redfish_aggregator_test.cpp',
'test/redfish-core/include/registries_test.cpp',
diff --git a/redfish-core/include/filter_expr_executor.hpp b/redfish-core/include/filter_expr_executor.hpp
new file mode 100644
index 0000000..a5aef04
--- /dev/null
+++ b/redfish-core/include/filter_expr_executor.hpp
@@ -0,0 +1,13 @@
+#pragma once
+
+#include "filter_expr_parser_ast.hpp"
+
+#include <nlohmann/json.hpp>
+
+namespace redfish
+{
+
+bool applyFilter(nlohmann::json& body,
+ const filter_ast::LogicalAnd& filterParam);
+
+}
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 3b04e21..e5f6195 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -4,6 +4,8 @@
#include "app.hpp"
#include "async_resp.hpp"
#include "error_messages.hpp"
+#include "filter_expr_executor.hpp"
+#include "filter_expr_printer.hpp"
#include "http_request.hpp"
#include "http_response.hpp"
#include "json_formatters.hpp"
@@ -169,6 +171,7 @@
{
// Only
bool isOnly = false;
+
// Expand
uint8_t expandLevel = 0;
ExpandType expandType = ExpandType::None;
@@ -180,6 +183,9 @@
static constexpr size_t maxTop = 1000; // Max entries a response contain
std::optional<size_t> top = std::nullopt;
+ // Filter
+ std::optional<filter_ast::LogicalAnd> filter = std::nullopt;
+
// Select
// Unclear how to make this use structured initialization without this.
// Might be a tidy bug? Ignore for now
@@ -374,6 +380,13 @@
return true;
}
+// Parses and validates the $filter parameter.
+inline bool getFilterParam(std::string_view value, Query& query)
+{
+ query.filter = parseFilter(value);
+ return query.filter.has_value();
+}
+
inline std::optional<Query> parseParameters(boost::urls::params_view urlParams,
crow::Response& res)
{
@@ -437,6 +450,14 @@
return std::nullopt;
}
}
+ else if (it.key == "$filter" && BMCWEB_INSECURE_ENABLE_REDFISH_QUERY)
+ {
+ if (!getFilterParam(it.value, ret))
+ {
+ messages::queryParameterValueFormatError(res, it.value, it.key);
+ return std::nullopt;
+ }
+ }
else
{
// Intentionally ignore other errors Redfish spec, 7.3.1
@@ -1047,6 +1068,11 @@
return;
}
+ if (query.filter)
+ {
+ applyFilter(intermediateResponse.jsonValue, *query.filter);
+ }
+
// According to Redfish Spec Section 7.3.1, $select is the last parameter to
// to process
if (!query.selectTrie.root.empty())
diff --git a/redfish-core/lib/service_root.hpp b/redfish-core/lib/service_root.hpp
index fcaee98..57b967c 100644
--- a/redfish-core/lib/service_root.hpp
+++ b/redfish-core/lib/service_root.hpp
@@ -109,7 +109,7 @@
BMCWEB_INSECURE_ENABLE_REDFISH_QUERY;
protocolFeatures["ExpandQuery"]["NoLinks"] =
BMCWEB_INSECURE_ENABLE_REDFISH_QUERY;
- protocolFeatures["FilterQuery"] = false;
+ protocolFeatures["FilterQuery"] = BMCWEB_INSECURE_ENABLE_REDFISH_QUERY;
protocolFeatures["OnlyMemberQuery"] = true;
protocolFeatures["SelectQuery"] = true;
protocolFeatures["DeepOperations"]["DeepPOST"] = false;
diff --git a/redfish-core/src/filter_expr_executor.cpp b/redfish-core/src/filter_expr_executor.cpp
new file mode 100644
index 0000000..291b1e4
--- /dev/null
+++ b/redfish-core/src/filter_expr_executor.cpp
@@ -0,0 +1,310 @@
+#include "filter_expr_executor.hpp"
+
+#include "filter_expr_parser_ast.hpp"
+#include "logging.hpp"
+
+namespace redfish
+{
+
+namespace
+{
+
+// Class that can convert an arbitrary AST type into a structured value
+// Pulling from the json pointer when required
+struct ValueVisitor
+{
+ using result_type =
+ std::variant<std::monostate, double, int64_t, std::string>;
+ nlohmann::json& body;
+ result_type operator()(double n);
+ result_type operator()(int64_t x);
+ result_type operator()(const filter_ast::UnquotedString& x);
+ result_type operator()(const filter_ast::QuotedString& x);
+};
+
+ValueVisitor::result_type ValueVisitor::operator()(double n)
+{
+ return {n};
+}
+
+ValueVisitor::result_type ValueVisitor::operator()(int64_t x)
+{
+ return {x};
+}
+
+ValueVisitor::result_type
+ ValueVisitor::operator()(const filter_ast::QuotedString& x)
+{
+ return {x};
+}
+
+ValueVisitor::result_type
+ ValueVisitor::operator()(const filter_ast::UnquotedString& x)
+{
+ // Future, handle paths with / in them
+ nlohmann::json::const_iterator entry = body.find(x);
+ if (entry == body.end())
+ {
+ BMCWEB_LOG_ERROR("Key {} doesn't exist in output, cannot filter",
+ static_cast<std::string>(x));
+ BMCWEB_LOG_DEBUG("Output {}", body.dump());
+ return {};
+ }
+ const double* dValue = entry->get_ptr<const double*>();
+ if (dValue != nullptr)
+ {
+ return {*dValue};
+ }
+ const int64_t* iValue = entry->get_ptr<const int64_t*>();
+ if (iValue != nullptr)
+ {
+ return {*iValue};
+ }
+ const std::string* strValue = entry->get_ptr<const std::string*>();
+ if (strValue != nullptr)
+ {
+ return {*strValue};
+ }
+
+ BMCWEB_LOG_ERROR(
+ "Type for key {} was {} which does not have a comparison operator",
+ static_cast<std::string>(x), static_cast<int>(entry->type()));
+ return {};
+}
+
+struct ApplyFilter
+{
+ nlohmann::json& body;
+ const filter_ast::LogicalAnd& filter;
+ using result_type = bool;
+ bool operator()(const filter_ast::LogicalNot& x);
+ bool operator()(const filter_ast::LogicalOr& x);
+ bool operator()(const filter_ast::LogicalAnd& x);
+ bool operator()(const filter_ast::Comparison& x);
+ bool operator()(const filter_ast::BooleanOp& x);
+
+ public:
+ bool matches();
+};
+
+bool ApplyFilter::operator()(const filter_ast::LogicalNot& x)
+{
+ bool subValue = (*this)(x.operand);
+ if (x.isLogicalNot)
+ {
+ return !subValue;
+ }
+ return subValue;
+}
+
+// Helper function to reduce the number of permutations of a single comparison
+// For all possible types.
+bool doDoubleComparison(double left, filter_ast::ComparisonOpEnum comparator,
+ double right)
+{
+ if (!std::isfinite(left) || !std::isfinite(right))
+ {
+ BMCWEB_LOG_ERROR("Refusing to do comparision of non finite numbers");
+ return false;
+ }
+ switch (comparator)
+ {
+ case filter_ast::ComparisonOpEnum::Equals:
+ // Note, floating point comparisons are hard. Compare equality
+ // based on epsilon
+ return std::fabs(left - right) <=
+ std::numeric_limits<double>::epsilon();
+ case filter_ast::ComparisonOpEnum::NotEquals:
+ return std::fabs(left - right) >
+ std::numeric_limits<double>::epsilon();
+ case filter_ast::ComparisonOpEnum::GreaterThan:
+ return left > right;
+ case filter_ast::ComparisonOpEnum::GreaterThanOrEqual:
+ return left >= right;
+ case filter_ast::ComparisonOpEnum::LessThan:
+ return left < right;
+ case filter_ast::ComparisonOpEnum::LessThanOrEqual:
+ return left <= right;
+ default:
+ BMCWEB_LOG_ERROR("Got x.token that should never happen {}",
+ static_cast<int>(comparator));
+ return true;
+ }
+}
+
+bool doIntComparison(int64_t left, filter_ast::ComparisonOpEnum comparator,
+ int64_t right)
+{
+ switch (comparator)
+ {
+ case filter_ast::ComparisonOpEnum::Equals:
+ return left == right;
+ case filter_ast::ComparisonOpEnum::NotEquals:
+ return left != right;
+ case filter_ast::ComparisonOpEnum::GreaterThan:
+ return left > right;
+ case filter_ast::ComparisonOpEnum::GreaterThanOrEqual:
+ return left >= right;
+ case filter_ast::ComparisonOpEnum::LessThan:
+ return left < right;
+ case filter_ast::ComparisonOpEnum::LessThanOrEqual:
+ return left <= right;
+ default:
+ BMCWEB_LOG_ERROR("Got comparator that should never happen {}",
+ static_cast<int>(comparator));
+ return true;
+ }
+}
+
+bool doStringComparison(std::string_view left,
+ filter_ast::ComparisonOpEnum comparator,
+ std::string_view right)
+{
+ switch (comparator)
+ {
+ case filter_ast::ComparisonOpEnum::Equals:
+ return left == right;
+ case filter_ast::ComparisonOpEnum::NotEquals:
+ return left != right;
+ default:
+ BMCWEB_LOG_ERROR(
+ "Got comparator that should never happen. Attempt to do numeric comparison on string {}",
+ static_cast<int>(comparator));
+ return true;
+ }
+}
+
+bool ApplyFilter::operator()(const filter_ast::Comparison& x)
+{
+ ValueVisitor numeric(body);
+ std::variant<std::monostate, double, int64_t, std::string> left =
+ boost::apply_visitor(numeric, x.left);
+ std::variant<std::monostate, double, int64_t, std::string> right =
+ boost::apply_visitor(numeric, x.right);
+
+ // Numeric comparisons
+ const double* lDoubleValue = std::get_if<double>(&left);
+ const double* rDoubleValue = std::get_if<double>(&right);
+ const int64_t* lIntValue = std::get_if<int64_t>(&left);
+ const int64_t* rIntValue = std::get_if<int64_t>(&right);
+
+ if (lDoubleValue != nullptr)
+ {
+ if (rDoubleValue != nullptr)
+ {
+ // Both sides are doubles, do the comparison as doubles
+ return doDoubleComparison(*lDoubleValue, x.token, *rDoubleValue);
+ }
+ if (rIntValue != nullptr)
+ {
+ // If right arg is int, promote right arg to double
+ return doDoubleComparison(*lDoubleValue, x.token,
+ static_cast<double>(*rIntValue));
+ }
+ }
+ if (lIntValue != nullptr)
+ {
+ if (rIntValue != nullptr)
+ {
+ // Both sides are ints, do the comparison as ints
+ return doIntComparison(*lIntValue, x.token, *rIntValue);
+ }
+
+ if (rDoubleValue != nullptr)
+ {
+ // Left arg is int, promote left arg to double
+ return doDoubleComparison(static_cast<double>(*lIntValue), x.token,
+ *rDoubleValue);
+ }
+ }
+
+ // String comparisons
+ const std::string* lStrValue = std::get_if<std::string>(&left);
+ const std::string* rStrValue = std::get_if<std::string>(&right);
+ if (lStrValue != nullptr && rStrValue != nullptr)
+ {
+ return doStringComparison(*lStrValue, x.token, *rStrValue);
+ }
+
+ BMCWEB_LOG_ERROR(
+ "Fell through. Should never happen. Attempt to compare type {} to type {}",
+ static_cast<int>(left.index()), static_cast<int>(right.index()));
+ return true;
+}
+
+bool ApplyFilter::operator()(const filter_ast::BooleanOp& x)
+{
+ return boost::apply_visitor(*this, x);
+}
+
+bool ApplyFilter::operator()(const filter_ast::LogicalOr& x)
+{
+ bool value = (*this)(x.first);
+ for (const filter_ast::LogicalNot& bOp : x.rest)
+ {
+ value = value || (*this)(bOp);
+ }
+ return value;
+}
+
+bool ApplyFilter::operator()(const filter_ast::LogicalAnd& x)
+{
+ bool value = (*this)(x.first);
+ for (const filter_ast::LogicalOr& bOp : x.rest)
+ {
+ value = value && (*this)(bOp);
+ }
+ return value;
+}
+
+bool ApplyFilter::matches()
+{
+ return (*this)(filter);
+}
+
+} // namespace
+
+// Applies a filter expression to a member array
+bool applyFilter(nlohmann::json& body,
+ const filter_ast::LogicalAnd& filterParam)
+{
+ using nlohmann::json;
+
+ json::object_t* obj = body.get_ptr<json::object_t*>();
+ if (obj == nullptr)
+ {
+ BMCWEB_LOG_ERROR("Requested filter wasn't an object????");
+ return false;
+ }
+ json::object_t::iterator members = obj->find("Members");
+ if (members == obj->end())
+ {
+ BMCWEB_LOG_ERROR("Collection didn't have members?");
+ return false;
+ }
+ json::array_t* memberArr = members->second.get_ptr<json::array_t*>();
+ if (memberArr == nullptr)
+ {
+ BMCWEB_LOG_ERROR("Members wasn't an object????");
+ return false;
+ }
+
+ json::array_t::iterator it = memberArr->begin();
+ size_t index = 0;
+ while (it != memberArr->end())
+ {
+ ApplyFilter filterApplier(*it, filterParam);
+ if (!filterApplier.matches())
+ {
+ BMCWEB_LOG_DEBUG("Removing item at index {}", index);
+ it = memberArr->erase(it);
+ index++;
+ continue;
+ }
+ it++;
+ index++;
+ }
+
+ return true;
+}
+} // namespace redfish
diff --git a/test/redfish-core/include/filter_expr_executor_test.cpp b/test/redfish-core/include/filter_expr_executor_test.cpp
new file mode 100644
index 0000000..adc5748
--- /dev/null
+++ b/test/redfish-core/include/filter_expr_executor_test.cpp
@@ -0,0 +1,168 @@
+#include "filter_expr_executor.hpp"
+#include "filter_expr_parser_ast.hpp"
+#include "filter_expr_printer.hpp"
+
+#include <optional>
+#include <string_view>
+
+#include "gmock/gmock.h"
+
+namespace redfish
+{
+
+static void filterTrue(std::string_view filterExpr, nlohmann::json json)
+{
+ std::optional<filter_ast::LogicalAnd> ast = parseFilter(filterExpr);
+ EXPECT_TRUE(ast);
+ if (!ast)
+ {
+ return;
+ }
+ EXPECT_EQ(json["Members"].size(), 1);
+ EXPECT_TRUE(applyFilter(json, *ast));
+ EXPECT_EQ(json["Members"].size(), 1);
+}
+
+static void filterFalse(std::string_view filterExpr, nlohmann::json json)
+{
+ std::optional<filter_ast::LogicalAnd> ast = parseFilter(filterExpr);
+ EXPECT_TRUE(ast);
+ if (!ast)
+ {
+ return;
+ }
+ EXPECT_EQ(json["Members"].size(), 1);
+ EXPECT_TRUE(applyFilter(json, *ast));
+ EXPECT_EQ(json["Members"].size(), 0);
+}
+
+TEST(FilterParser, Integers)
+{
+ const nlohmann::json members = R"({"Members": [{"Count": 2}]})"_json;
+ // Forward true conditions
+ filterTrue("Count eq 2", members);
+ filterTrue("Count ne 3", members);
+ filterTrue("Count gt 1", members);
+ filterTrue("Count ge 2", members);
+ filterTrue("Count lt 3", members);
+ filterTrue("Count le 2", members);
+
+ // Reverse true conditions
+ filterTrue("2 eq Count", members);
+ filterTrue("3 ne Count", members);
+ filterTrue("3 gt Count", members);
+ filterTrue("2 ge Count", members);
+ filterTrue("1 lt Count", members);
+ filterTrue("2 le Count", members);
+
+ // Forward false conditions
+ filterFalse("Count eq 3", members);
+ filterFalse("Count ne 2", members);
+ filterFalse("Count gt 2", members);
+ filterFalse("Count ge 3", members);
+ filterFalse("Count lt 2", members);
+ filterFalse("Count le 1", members);
+
+ // Reverse false conditions
+ filterFalse("3 eq Count", members);
+ filterFalse("2 ne Count", members);
+ filterFalse("2 gt Count", members);
+ filterFalse("1 ge Count", members);
+ filterFalse("2 lt Count", members);
+ filterFalse("3 le Count", members);
+}
+
+TEST(FilterParser, FloatingPointToInteger)
+{
+ const nlohmann::json members = R"({"Members": [{"Count": 2.0}]})"_json;
+ // Forward true conditions
+ filterTrue("Count eq 2", members);
+ filterTrue("Count ne 3", members);
+ filterTrue("Count gt 1", members);
+ filterTrue("Count ge 2", members);
+ filterTrue("Count lt 3", members);
+ filterTrue("Count le 2", members);
+
+ // Reverse true conditions
+ filterTrue("2 eq Count", members);
+ filterTrue("3 ne Count", members);
+ filterTrue("3 gt Count", members);
+ filterTrue("2 ge Count", members);
+ filterTrue("1 lt Count", members);
+ filterTrue("2 le Count", members);
+
+ // Forward false conditions
+ filterFalse("Count eq 3", members);
+ filterFalse("Count ne 2", members);
+ filterFalse("Count gt 2", members);
+ filterFalse("Count ge 3", members);
+ filterFalse("Count lt 2", members);
+ filterFalse("Count le 1", members);
+
+ // Reverse false conditions
+ filterFalse("3 eq Count", members);
+ filterFalse("2 ne Count", members);
+ filterFalse("2 gt Count", members);
+ filterFalse("1 ge Count", members);
+ filterFalse("2 lt Count", members);
+ filterFalse("3 le Count", members);
+}
+
+TEST(FilterParser, FloatingPointToFloatingPoint)
+{
+ const nlohmann::json members = R"({"Members": [{"Count": 2.0}]})"_json;
+ // Forward true conditions
+ filterTrue("Count eq 2.0", members);
+ filterTrue("Count ne 3.0", members);
+ filterTrue("Count gt 1.0", members);
+ filterTrue("Count ge 2.0", members);
+ filterTrue("Count lt 3.0", members);
+ filterTrue("Count le 2.0", members);
+
+ // Reverse true conditions
+ filterTrue("2.0 eq Count", members);
+ filterTrue("3.0 ne Count", members);
+ filterTrue("3.0 gt Count", members);
+ filterTrue("2.0 ge Count", members);
+ filterTrue("1.0 lt Count", members);
+ filterTrue("2.0 le Count", members);
+
+ // Forward false conditions
+ filterFalse("Count eq 3.0", members);
+ filterFalse("Count ne 2.0", members);
+ filterFalse("Count gt 2.0", members);
+ filterFalse("Count ge 3.0", members);
+ filterFalse("Count lt 2.0", members);
+ filterFalse("Count le 1.0", members);
+
+ // Reverse false conditions
+ filterFalse("3.0 eq Count", members);
+ filterFalse("2.0 ne Count", members);
+ filterFalse("2.0 gt Count", members);
+ filterFalse("1.0 ge Count", members);
+ filterFalse("2.0 lt Count", members);
+ filterFalse("3.0 le Count", members);
+}
+
+TEST(FilterParser, String)
+{
+ const nlohmann::json members =
+ R"({"Members": [{"SerialNumber": "Foo"}]})"_json;
+ // Forward true conditions
+ filterTrue("SerialNumber eq 'Foo'", members);
+ filterTrue("SerialNumber ne 'NotFoo'", members);
+
+ // Reverse true conditions
+ filterTrue("'Foo' eq SerialNumber", members);
+ filterTrue("'NotFoo' ne SerialNumber", members);
+
+ // Forward false conditions
+ filterFalse("SerialNumber eq 'NotFoo'", members);
+ filterFalse("SerialNumber ne 'Foo'", members);
+
+ // Reverse false conditions
+ filterFalse("'NotFoo' eq SerialNumber", members);
+ filterFalse("'Foo' ne SerialNumber", members);
+}
+
+} // namespace redfish
diff --git a/test/redfish-core/lib/service_root_test.cpp b/test/redfish-core/lib/service_root_test.cpp
index ab30e70..57583dd 100644
--- a/test/redfish-core/lib/service_root_test.cpp
+++ b/test/redfish-core/lib/service_root_test.cpp
@@ -101,7 +101,8 @@
{
EXPECT_EQ(json["ProtocolFeaturesSupported"]["ExpandQuery"].size(), 4);
}
- EXPECT_FALSE(json["ProtocolFeaturesSupported"]["FilterQuery"]);
+ EXPECT_EQ(json["ProtocolFeaturesSupported"]["FilterQuery"],
+ BMCWEB_INSECURE_ENABLE_REDFISH_QUERY);
EXPECT_TRUE(json["ProtocolFeaturesSupported"]["OnlyMemberQuery"]);
EXPECT_TRUE(json["ProtocolFeaturesSupported"]["SelectQuery"]);
EXPECT_FALSE(