Changed the $filter string conversion to use Regex and be more fault tolerant.

PiperOrigin-RevId: 615581952

Author: ecclesia-robot

ecclesia/lib/redfish/BUILD

@@ -31,6 +31,7 @@ cc_library(
         "@com_google_absl//absl/time",
         "@com_google_absl//absl/types:optional",
         "@com_google_absl//absl/types:span",
+        "@com_googlesource_code_re2//:re2",
         "@com_json//:json",
     ],
 )

ecclesia/lib/redfish/interface.cc

@@ -35,6 +35,11 @@
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
+#include "re2/re2.h"
+
+// Pattern for expression [lhs][operator][rhs]
+constexpr LazyRE2 kRelationalExpressionRegex = {
+    "^(?P<left>[^\\s<=>!]+)(?:(<=|>=|!=|>|<|=)(?P<right>[^<=>!]+))$"};
 
 constexpr std::array<const char *, 6> kPredicateOperators = {
     "<=", ">=", "!=", ">", "<", "=",
@@ -64,14 +69,19 @@ struct EncodedPredicate {
 absl::StatusOr<RelationalExpression> EncodeRelationalExpression(
     std::string_view expression) {
   RelationalExpression relational_expression;
-  for (const std::string predicate_operator : kPredicateOperators) {
-    auto pos = expression.find(predicate_operator);
-    if (pos == std::string::npos) continue;
-    RelationalExpression rel_expr;
-    rel_expr.lhs = expression.substr(0, pos);
-    rel_expr.rel_operator = predicate_operator;
-    rel_expr.rhs = expression.substr(pos + predicate_operator.size());
-    return rel_expr;
+
+  // Regex match the expression. The operator must be a valid relational
+  // operator and the right hand side must not have spaces or characters
+  // included in relation operators.
+  std::string lhs;
+  std::string op;
+  std::string rhs;
+  if (RE2::FullMatch(expression, *kRelationalExpressionRegex, &lhs, &op,
+                     &rhs)) {
+    relational_expression.lhs = lhs;
+    relational_expression.rel_operator = op;
+    relational_expression.rhs = rhs;
+    return relational_expression;
   }
   return absl::InvalidArgumentError("Invalid expression");
 }
@@ -83,7 +93,7 @@ absl::StatusOr<RelationalExpression> EncodeRelationalExpression(
 //   expressions = [[lhs: "Prop1", rel_operator: "<=", rhs: "42"],
 //                  [lhs: "Prop1", rel_operator: ">", rhs: "84"]
 //                 ]
-EncodedPredicate EncodePredicate(absl::string_view predicate) {
+absl::StatusOr<EncodedPredicate> EncodePredicate(absl::string_view predicate) {
   // The high level logic of this function is to break down the predicate into
   // logical operators (or/and) and the relational expressions (prop>value).
   // After breaking them up go through the relational expressions and break them
@@ -120,6 +130,9 @@ EncodedPredicate EncodePredicate(absl::string_view predicate) {
     auto encoded_expression = EncodeRelationalExpression(expression);
     if (encoded_expression.ok()) {
       encoded_predicate.expressions.push_back(encoded_expression.value());
+    } else {
+      // Invalid expression, return an error
+      return absl::InvalidArgumentError(encoded_expression.status().message());
     }
   }
   return encoded_predicate;
@@ -206,15 +219,29 @@ std::string GenerateFilterString(const EncodedPredicate &predicate_object) {
 
 void RedfishQueryParamFilter::BuildFromRedpathPredicate(
     absl::string_view predicate) {
-  filter_string_ = GenerateFilterString(EncodePredicate(predicate));
+  auto encoded_predicate = EncodePredicate(predicate);
+  if (!encoded_predicate.ok()) {
+    // Invalid predicate, set filter_string_ to empty, indicating an invalid
+    // expression, and return.
+    filter_string_ = "";
+    return;
+  }
+  filter_string_ = GenerateFilterString(*encoded_predicate);
 }
 
 void RedfishQueryParamFilter::BuildFromRedpathPredicateList(
     const std::vector<std::string> &predicates) {
   std::vector<std::string> filter_strings;
   filter_strings.reserve(predicates.size());
   for (absl::string_view predicate : predicates) {
-    filter_strings.push_back(GenerateFilterString(EncodePredicate(predicate)));
+    auto encoded_predicate = EncodePredicate(predicate);
+    if (!encoded_predicate.ok()) {
+      // Invalid predicate, set filter_string_ to empty, indicating an invalid
+      // expression, and return.
+      filter_string_ = "";
+      return;
+    }
+    filter_strings.push_back(GenerateFilterString(*encoded_predicate));
   }
   filter_string_ = absl::StrJoin(filter_strings, "%20or%20");
 }

ecclesia/lib/redfish/interface_test.cc

@@ -201,6 +201,40 @@ TEST(RedfishVariant, RedfishQueryParamFilter) {
   EXPECT_EQ(filter.ToString(), absl::StrCat("$filter=", filter_string));
 }
 
+TEST(RedfishVariant, RedfishQueryParamFilterInvalid) {
+  auto filter = RedfishQueryParamFilter();
+  // Add a valid expression to $filter to make sure the string is overwritten by
+  // the builder.
+  filter.BuildFromRedpathPredicate("Prop1=42");
+  // Invalid operator (wrong equality)
+  std::string predicate1 = "Prop1==42";
+  filter.BuildFromRedpathPredicate(predicate1);
+  EXPECT_EQ(filter.ToString(), "");
+  // Invalid operator
+  std::string predicate2 = "Prop1>>42";
+  filter.BuildFromRedpathPredicate(predicate2);
+  EXPECT_EQ(filter.ToString(), "");
+  // Spaces on left
+  std::string predicate3 = "Bad Property>42";
+  filter.BuildFromRedpathPredicate(predicate3);
+  EXPECT_EQ(filter.ToString(), "");
+  // Add a valid expression to $filter to make sure the string is overwritten by
+  // the list builder.
+  filter.BuildFromRedpathPredicate("Prop1=42");
+  // Special characters in operands
+  std::string predicate4 = "Prop<erty1=42";
+  std::string predicate5 = "Property2=4>2";
+  filter.BuildFromRedpathPredicateList({predicate4, predicate5});
+  EXPECT_EQ(filter.ToString(), "");
+  // One side of a logical exp is bad. Try both sides.
+  std::string predicate6 = "Property2=42";
+  std::string predicate7 = "Prop<erty1=42";
+  filter.BuildFromRedpathPredicateList({predicate6, predicate7});
+  EXPECT_EQ(filter.ToString(), "");
+  filter.BuildFromRedpathPredicateList({predicate7, predicate6});
+  EXPECT_EQ(filter.ToString(), "");
+}
+
 TEST(RedfishVariant, RedfishQueryParamFilterEmpty) {
   auto filter = RedfishQueryParamFilter();
   EXPECT_EQ(filter.ToString(), "");