From 96efe83174dc89d44593c9f37b445f583687fc4b Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Thu, 11 Apr 2019 10:18:03 -0300 Subject: [PATCH] Improves rules dump for better testing --- CHANGES | 2 ++ headers/modsecurity/rule.h | 21 ++++++++++++++++----- headers/modsecurity/rules.h | 7 ++++++- src/rule_marker.h | 14 ++++++++++++++ src/rule_with_actions.h | 5 +++++ src/rule_with_operator.h | 11 +++++++++++ src/rules.cc | 8 +++----- tools/rules-check/rules-check.cc | 1 + 8 files changed, 58 insertions(+), 11 deletions(-) diff --git a/CHANGES b/CHANGES index 1366fab8..32f289c9 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v3.x.y - YYYY-MMM-DD (to be released) ------------------------------------- + - More structured rules dump. Better supporting debugging. + [@zimmerle] - Added the basics for supporting better error/warning handling while loading configurations. [@zimmerle] diff --git a/headers/modsecurity/rule.h b/headers/modsecurity/rule.h index a81bdd16..f4d5eb47 100644 --- a/headers/modsecurity/rule.h +++ b/headers/modsecurity/rule.h @@ -46,15 +46,14 @@ class Rule { Rule(std::unique_ptr fileName, int lineNumber) : m_fileName(std::move(fileName)), m_lineNumber(lineNumber), - m_phase(modsecurity::Phases::RequestHeadersPhase) { - } + m_phase(modsecurity::Phases::RequestHeadersPhase) + { } Rule(const Rule &r) : m_fileName(r.m_fileName), m_lineNumber(r.m_lineNumber), - m_phase(r.m_phase) { - - } + m_phase(r.m_phase) + { } Rule &operator=(const Rule& other) { m_fileName = other.m_fileName; @@ -80,6 +79,18 @@ class Rule { return *m_fileName + ":" + std::to_string(m_lineNumber); } + virtual void dump(std::stringstream &out) { + out << getOriginInTextFormat() << std::endl; + } + + protected: + std::string getOriginInTextFormat() const { + std::stringstream ss; + ss << "# File name: " << *getFileName() << std::endl; + ss << "# Line number: " << getLineNumber(); + return ss.str(); + } + private: std::shared_ptr m_fileName; int m_lineNumber; diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index eeebd29d..3721f8bb 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -61,7 +61,12 @@ class Rules { std::vector > m_defaultActions; std::vector > m_defaultTransformations; - void dump(); + virtual void dump() { + std::stringstream ss; + dump(ss); + std::cout << ss.str(); + }; + virtual void dump(std::stringstream &out); inline iterator begin() noexcept { return m_rules.begin(); } inline const_iterator cbegin() const noexcept { return m_rules.cbegin(); } diff --git a/src/rule_marker.h b/src/rule_marker.h index 10986f6f..2f311999 100644 --- a/src/rule_marker.h +++ b/src/rule_marker.h @@ -44,6 +44,15 @@ class RuleMarker : public Rule { : Rule(std::move(fileName), lineNumber), m_name(std::make_shared(name)) { } + RuleMarker(RuleMarker &&r) : + Rule(r), + m_name(std::move(r.m_name)) + { }; + + RuleMarker(const RuleMarker &r) : + Rule(r), + m_name(std::move(r.m_name)) + { }; virtual bool evaluate(Transaction *transaction) override { if (transaction->isInsideAMarker()) { @@ -62,6 +71,11 @@ class RuleMarker : public Rule { return m_name; } + virtual void dump(std::stringstream &out) override { + Rule::dump(out); + out << "SecMarker \"" << *getName() << "\"" << std::endl; + } + private: std::shared_ptr m_name; }; diff --git a/src/rule_with_actions.h b/src/rule_with_actions.h index 8ec19971..d9489670 100644 --- a/src/rule_with_actions.h +++ b/src/rule_with_actions.h @@ -454,6 +454,11 @@ class RuleWithActions : public Rule { return dst; } + + virtual void dump(std::stringstream &out) override { + out << "RuleWithActions" << std::endl; + } + private: RuleId m_ruleId; diff --git a/src/rule_with_operator.h b/src/rule_with_operator.h index b7cf3065..8f00414a 100644 --- a/src/rule_with_operator.h +++ b/src/rule_with_operator.h @@ -30,6 +30,8 @@ #include "modsecurity/variable_value.h" #include "modsecurity/rule.h" #include "src/rule_with_actions.h" +#include "src/variables/variable.h" +#include "src/operators/operator.h" #ifdef __cplusplus @@ -80,6 +82,15 @@ class RuleWithOperator : public RuleWithActions { return std::to_string(getId()); } + virtual void dump(std::stringstream &out) override { + Rule::dump(out); + out << "# RuleWithOperator" << std::endl; + out << "SecRule "; + out << m_variables->getVariableNames() << " "; + out << "\"" << "@" << m_operator->m_op << " " << m_operator->m_param << "\""; + out << std::endl; + } + private: std::shared_ptr m_variables; std::shared_ptr m_operator; diff --git a/src/rules.cc b/src/rules.cc index 0665231a..ff91e8b4 100644 --- a/src/rules.cc +++ b/src/rules.cc @@ -57,13 +57,11 @@ std::shared_ptr Rules::at(int index) const { } -void Rules::dump() { - for (int j = 0; j < m_rules.size(); j++) { - std::cout << " Rule ID: " << m_rules.at(j)->getReference(); - std::cout << "--" << m_rules.at(j) << std::endl; +void Rules::dump(std::stringstream &out) { + for (auto &r : m_rules) { + r->dump(out); } } - } // namespace modsecurity diff --git a/tools/rules-check/rules-check.cc b/tools/rules-check/rules-check.cc index 91d78d84..fb3108d1 100644 --- a/tools/rules-check/rules-check.cc +++ b/tools/rules-check/rules-check.cc @@ -91,6 +91,7 @@ int main(int argc, char **argv) { if (err.empty() == false) { std::cerr << " " << err << std::endl; } + rules->dump(); next: args++; }