Simplify condition parsing and unify negated matcher names

src/condition/scalar_condition.cpp

@@ -175,11 +175,11 @@ eval_result scalar_condition::eval(condition_cache &cache, const object_store &s
 
  if (match.has_value()) {
  cache.match = std::move(match);
- return {true, ephemeral};
+ return {.outcome = true, .ephemeral = ephemeral};
  }
  }
 
- return {false, false};
+ return {.outcome = false, .ephemeral = false};
 }
 
 eval_result scalar_negated_condition::eval(condition_cache &cache, const object_store &store,
@@ -224,11 +224,11 @@ eval_result scalar_negated_condition::eval(condition_cache &cache, const object_
  if (!match) {
  cache.match = {{{{"input"sv, object_to_string(*object), target_.name,
  {target_.key_path.begin(), target_.key_path.end()}}},
- {}, matcher_name_, matcher->to_string(), ephemeral}};
- return {true, ephemeral};
+ {}, matcher->negated_name(), matcher->to_string(), ephemeral}};
+ return {.outcome = true, .ephemeral = ephemeral};
  }
 
- return {false, false};
+ return {.outcome = false, .ephemeral = false};
 }
 
 } // namespace ddwaf

src/condition/scalar_condition.hpp

@@ -53,10 +53,8 @@ class scalar_condition : public base_condition {
 class scalar_negated_condition : public base_condition {
 public:
  scalar_negated_condition(std::unique_ptr<matcher::base> &&matcher, std::string data_id,
- std::vector<condition_parameter> args, std::string matcher_name,
- const object_limits &limits = {})
- : matcher_(std::move(matcher)), data_id_(std::move(data_id)),
- matcher_name_(std::move(matcher_name)), limits_(limits)
+ std::vector<condition_parameter> args, const object_limits &limits = {})
+ : matcher_(std::move(matcher)), data_id_(std::move(data_id)), limits_(limits)
  {
  if (args.size() > 1) {
  throw std::invalid_argument("matcher initialised with more than one argument");
@@ -93,7 +91,6 @@ class scalar_negated_condition : public base_condition {
  std::unique_ptr<matcher::base> matcher_;
  std::string data_id_;
  condition_target target_;
- std::string matcher_name_;
  object_limits limits_;
 };
 

src/matcher/base.hpp

@@ -28,6 +28,7 @@ class base {
  // for example, through a constexpr class static string_view initialised
  // with a literal.
  [[nodiscard]] virtual std::string_view name() const = 0;
+ [[nodiscard]] virtual std::string_view negated_name() const = 0;
  // Returns a string representing this particular instance of the operator, for example,
  // an operator matching regexes could provide the regex as its string representation.
  [[nodiscard]] virtual std::string_view to_string() const = 0;
@@ -48,6 +49,7 @@ template <typename T> class base_impl : public base {
  base_impl &operator=(base_impl &&) noexcept = default;
 
  [[nodiscard]] std::string_view name() const override { return T::matcher_name; }
+ [[nodiscard]] std::string_view negated_name() const override { return T::negated_matcher_name; }
 
  [[nodiscard]] std::string_view to_string() const override
  {

src/matcher/equals.hpp

@@ -17,6 +17,7 @@ namespace ddwaf::matcher {
 template <typename T = void> class equals : public base_impl<equals<T>> {
 public:
  static constexpr std::string_view matcher_name = "equals";
+ static constexpr std::string_view negated_matcher_name = "!equals";
 
  explicit equals(T expected)
  requires(!std::is_floating_point_v<T>)
@@ -72,6 +73,7 @@ template <typename T = void> class equals : public base_impl<equals<T>> {
 template <> class equals<double> : public base_impl<equals<double>> {
 public:
  static constexpr std::string_view matcher_name = "equals";
+ static constexpr std::string_view negated_matcher_name = "!equals";
 
  // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
  equals(double expected, double delta) : expected_(expected), delta_(delta) {}
@@ -102,6 +104,7 @@ template <> class equals<double> : public base_impl<equals<double>> {
 template <> class equals<void> : public base_impl<equals<void>> {
 public:
  static constexpr std::string_view matcher_name = "equals";
+ static constexpr std::string_view negated_matcher_name = "!equals";
 
  // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
  ~equals() override = default;

src/matcher/exact_match.hpp

@@ -19,6 +19,7 @@ class exact_match : public base_impl<exact_match> {
  using data_type = std::vector<std::pair<std::string_view, uint64_t>>;
 
  static constexpr std::string_view matcher_name = "exact_match";
+ static constexpr std::string_view negated_matcher_name = "!exact_match";
 
  exact_match() = default;
  explicit exact_match(std::vector<std::string> &&data);

src/matcher/greater_than.hpp

@@ -18,6 +18,7 @@ namespace ddwaf::matcher {
 template <typename T = void> class greater_than : public base_impl<greater_than<T>> {
 public:
  static constexpr std::string_view matcher_name = "greater_than";
+ static constexpr std::string_view negated_matcher_name = "!greater_than";
 
  explicit greater_than(T minimum)
  requires std::is_same_v<T, uint64_t> || std::is_same_v<T, int64_t> ||
@@ -56,6 +57,7 @@ template <typename T = void> class greater_than : public base_impl<greater_than<
 template <> class greater_than<void> : public base_impl<greater_than<void>> {
 public:
  static constexpr std::string_view matcher_name = "greater_than";
+ static constexpr std::string_view negated_matcher_name = "!greater_than";
 
  ~greater_than() override = default;
 

src/matcher/ip_match.hpp

@@ -20,6 +20,7 @@ class ip_match : public base_impl<ip_match> {
  using data_type = std::vector<std::pair<std::string_view, uint64_t>>;
 
  static constexpr std::string_view matcher_name = "ip_match";
+ static constexpr std::string_view negated_matcher_name = "!ip_match";
 
  ip_match() = default;
  explicit ip_match(const std::vector<std::string_view> &ip_list);

src/matcher/is_sqli.hpp

@@ -15,6 +15,7 @@ namespace ddwaf::matcher {
 class is_sqli : public base_impl<is_sqli> {
 public:
  static constexpr std::string_view matcher_name = "is_sqli";
+ static constexpr std::string_view negated_matcher_name = "!is_sqli";
 
  is_sqli() = default;
  ~is_sqli() override = default;

src/matcher/is_xss.hpp

@@ -15,6 +15,7 @@ namespace ddwaf::matcher {
 class is_xss : public base_impl<is_xss> {
 public:
  static constexpr std::string_view matcher_name = "is_xss";
+ static constexpr std::string_view negated_matcher_name = "!is_xss";
 
  is_xss() = default;
  ~is_xss() override = default;

src/matcher/lower_than.hpp

@@ -18,6 +18,7 @@ namespace ddwaf::matcher {
 template <typename T = void> class lower_than : public base_impl<lower_than<T>> {
 public:
  static constexpr std::string_view matcher_name = "lower_than";
+ static constexpr std::string_view negated_matcher_name = "!lower_than";
 
  explicit lower_than(T maximum)
  requires std::is_same_v<T, uint64_t> || std::is_same_v<T, int64_t> ||
@@ -57,6 +58,7 @@ template <typename T = void> class lower_than : public base_impl<lower_than<T>>
 template <> class lower_than<void> : public base_impl<lower_than<void>> {
 public:
  static constexpr std::string_view matcher_name = "lower_than";
+ static constexpr std::string_view negated_matcher_name = "!lower_than";
 
  ~lower_than() override = default;
 

src/matcher/phrase_match.hpp

@@ -17,6 +17,7 @@ namespace ddwaf::matcher {
 class phrase_match : public base_impl<phrase_match> {
 public:
  static constexpr std::string_view matcher_name = "phrase_match";
+ static constexpr std::string_view negated_matcher_name = "!phrase_match";
 
  phrase_match(std::vector<const char *> pattern, std::vector<uint32_t> lengths,
  bool enforce_word_boundary = false);

src/matcher/regex_match.hpp

@@ -17,6 +17,7 @@ namespace ddwaf::matcher {
 class regex_match : public base_impl<regex_match> {
 public:
  static constexpr std::string_view matcher_name = "match_regex";
+ static constexpr std::string_view negated_matcher_name = "!match_regex";
 
  regex_match(const std::string &regex_str, std::size_t minLength, bool case_sensitive);
  ~regex_match() override = default;

src/parser/expression_parser.cpp

@@ -22,7 +22,11 @@
 #include "log.hpp"
 #include "matcher/equals.hpp"
 #include "matcher/exact_match.hpp"
+#include "matcher/greater_than.hpp"
 #include "matcher/ip_match.hpp"
+#include "matcher/is_sqli.hpp"
+#include "matcher/is_xss.hpp"
+#include "matcher/lower_than.hpp"
 #include "matcher/phrase_match.hpp"
 #include "matcher/regex_match.hpp"
 #include "parameter.hpp"
@@ -106,6 +110,20 @@ std::vector<condition_parameter> parse_arguments(const parameter::map &params, d
  return definitions;
 }
 
+template <typename T, typename... Matchers>
+auto build_condition(auto operator_name, auto &params, auto &data_ids_to_type, auto source,
+ auto &transformers, auto &addresses, auto &limits)
+{
+ auto [data_id, matcher] = parse_matcher<Matchers...>(operator_name, params);
+
+ if (!matcher && !data_id.empty()) {
+ data_ids_to_type.emplace(data_id, operator_name);
+ }
+
+ auto arguments = parse_arguments<T>(params, source, transformers, addresses, limits);
+ return std::make_unique<T>(std::move(matcher), data_id, std::move(arguments), limits);
+}
+
 } // namespace
 
 std::shared_ptr<expression> parse_expression(const parameter::vector &conditions_array,
@@ -146,37 +164,19 @@ std::shared_ptr<expression> parse_expression(const parameter::vector &conditions
  params, source, transformers, addresses, limits);
  conditions.emplace_back(
  std::make_unique<exists_negated_condition>(std::move(arguments), limits));
+ } else if (operator_name.starts_with('!')) {
+ conditions.emplace_back(
+ build_condition<scalar_negated_condition, matcher::ip_match, matcher::exact_match,
+ matcher::regex_match, matcher::phrase_match, matcher::equals<>>(
+ operator_name.substr(1), params, data_ids_to_type, source, transformers,
+ addresses, limits));
  } else {
- auto raw_operator_name = operator_name;
- auto negated = operator_name.starts_with('!');
- if (!negated) {
- auto [data_id, matcher] = parse_any_matcher(operator_name, params);
-
- if (!matcher && !data_id.empty()) {
- data_ids_to_type.emplace(data_id, operator_name);
- }
-
- auto arguments = parse_arguments<scalar_condition>(
- params, source, transformers, addresses, limits);
- conditions.emplace_back(std::make_unique<scalar_condition>(
- std::move(matcher), data_id, std::move(arguments), limits));
-
- } else {
- operator_name = operator_name.substr(1);
- auto [data_id, matcher] =
- parse_matcher<matcher::ip_match, matcher::exact_match, matcher::regex_match,
- matcher::phrase_match, matcher::equals<>>(operator_name, params);
-
- if (!matcher && !data_id.empty()) {
- data_ids_to_type.emplace(data_id, operator_name);
- }
-
- auto arguments = parse_arguments<scalar_negated_condition>(
- params, source, transformers, addresses, limits);
- conditions.emplace_back(
- std::make_unique<scalar_negated_condition>(std::move(matcher), data_id,
- std::move(arguments), std::string{raw_operator_name}, limits));
- }
+ conditions.emplace_back(
+ build_condition<scalar_condition, matcher::equals<>, matcher::exact_match,
+ matcher::greater_than<>, matcher::ip_match, matcher::is_sqli, matcher::is_xss,
+ matcher::lower_than<>, matcher::phrase_match, matcher::regex_match>(
+ operator_name, params, data_ids_to_type, source, transformers, addresses,
+ limits));
  }
  }
 

tests/test_utils.hpp

@@ -77,7 +77,7 @@ class expression_builder {
  } else {
  conditions_.emplace_back(std::make_unique<scalar_negated_condition>(
  std::make_unique<T>(std::forward<Args>(args)...), std::string{},
- std::move(arguments_), "!" + std::string{T::matcher_name}));
+ std::move(arguments_)));
  }
  }