Merge pull request #3253 from eduar-hte/rule-message

Simplified handling of RuleMessage by removing usage of std::shared_ptr

build/win32/CMakeLists.txt

@@ -171,13 +171,13 @@ endfunction()
 
 # unit tests
 file(GLOB unitTestSources ${BASE_DIR}/test/unit/*.cc)
-add_executable(unit_tests ${unitTestSources})
+add_executable(unit_tests ${unitTestSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
 setTestTargetProperties(unit_tests)
 target_compile_options(unit_tests PRIVATE /wd4805)
 
 # regression tests
 file(GLOB regressionTestsSources ${BASE_DIR}/test/regression/*.cc)
-add_executable(regression_tests ${regressionTestsSources})
+add_executable(regression_tests ${regressionTestsSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
 setTestTargetProperties(regression_tests)
 
 macro(add_regression_test_capability compile_definition flag)

examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h

@@ -160,13 +160,13 @@ class ReadingLogsViaRuleMessage {
         std::cout << std::endl;
         if (ruleMessage->m_isDisruptive) {
             std::cout << " * Disruptive action: ";
-            std::cout << modsecurity::RuleMessage::log(ruleMessage);
+            std::cout << modsecurity::RuleMessage::log(*ruleMessage);
             std::cout << std::endl;
             std::cout << " ** %d is meant to be informed by the webserver.";
             std::cout << std::endl;
         } else {
             std::cout << " * Match, but no disruptive action: ";
-            std::cout << modsecurity::RuleMessage::log(ruleMessage);
+            std::cout << modsecurity::RuleMessage::log(*ruleMessage);
             std::cout << std::endl;
         }
     }

examples/using_bodies_in_chunks/simple_request.cc

@@ -81,13 +81,13 @@ static void logCb(void *data, const void *ruleMessagev) {
     std::cout << std::endl;
     if (ruleMessage->m_isDisruptive) {
         std::cout << " * Disruptive action: ";
-        std::cout << modsecurity::RuleMessage::log(ruleMessage);
+        std::cout << modsecurity::RuleMessage::log(*ruleMessage);
         std::cout << std::endl;
         std::cout << " ** %d is meant to be informed by the webserver.";
         std::cout << std::endl;
     } else {
         std::cout << " * Match, but no disruptive action: ";
-        std::cout << modsecurity::RuleMessage::log(ruleMessage);
+        std::cout << modsecurity::RuleMessage::log(*ruleMessage);
         std::cout << std::endl;
     }
 }

headers/modsecurity/actions/action.h

@@ -89,7 +89,7 @@ class Action {
 
     virtual bool evaluate(RuleWithActions *rule, Transaction *transaction);
     virtual bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> ruleMessage) {
+        RuleMessage &ruleMessage) {
         return evaluate(rule, transaction);
     }
     virtual bool init(std::string *error) { return true; }

headers/modsecurity/modsecurity.h

@@ -292,7 +292,7 @@ class ModSecurity {
      */
     void setServerLogCb(ModSecLogCb cb, int properties);
 
-    void serverLog(void *data, std::shared_ptr<RuleMessage> rm);
+    void serverLog(void *data, const RuleMessage &rm);
 
     const std::string& getConnectorInformation() const;
 

headers/modsecurity/rule.h

@@ -78,8 +78,7 @@ class Rule {
 
     virtual bool evaluate(Transaction *transaction) = 0;
 
-    virtual bool evaluate(Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) = 0;
+    virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) = 0;
 
     const std::string& getFileName() const {
         return m_fileName;

headers/modsecurity/rule_marker.h

@@ -42,8 +42,7 @@ class RuleMarker : public Rule {
 
     RuleMarker &operator=(const RuleMarker &r) = delete;
 
-    virtual bool evaluate(Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override {
+    virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override {
         return evaluate(transaction);
     }
 

headers/modsecurity/rule_message.h

@@ -13,14 +13,6 @@
  *
  */
 
-#ifdef __cplusplus
-#include <stack>
-#include <vector>
-#include <string>
-#include <list>
-#include <cstring>
-#endif
-
 #ifndef HEADERS_MODSECURITY_RULE_MESSAGE_H_
 #define HEADERS_MODSECURITY_RULE_MESSAGE_H_
 
@@ -31,8 +23,10 @@
 
 #ifdef __cplusplus
 
-namespace modsecurity {
+#include <string>
+#include <list>
 
+namespace modsecurity {
 
 
 class RuleMessage {
@@ -45,43 +39,51 @@ class RuleMessage {
     RuleMessage(const RuleWithActions &rule, const Transaction &trans) :
         m_rule(rule),
         m_transaction(trans)
-    { }
+    {
+        reset(true);
+    }
 
     RuleMessage(const RuleMessage &ruleMessage) = default;
     RuleMessage &operator=(const RuleMessage &ruleMessage) = delete;
 
-    void clean() {
-        m_data = "";
-        m_match = "";
+    void reset(const bool resetSaveMessage)
+    {
+        m_data.clear();
         m_isDisruptive = false;
-        m_reference = "";
+        m_match.clear();
+        m_message.clear();
+        m_noAuditLog = false;
+        m_reference.clear();
+        if (resetSaveMessage == true)
+            m_saveMessage = true;
         m_severity = 0;
+        m_tags.clear();
     }
 
-    std::string log() {
-        return log(this, 0);
+    std::string log() const {
+        return log(*this, 0);
     }
-    std::string log(int props) {
-        return log(this, props);
+    std::string log(int props) const {
+        return log(*this, props);
     }
-    std::string log(int props, int responseCode) {
-        return log(this, props, responseCode);
+    std::string log(int props, int responseCode) const {
+        return log(*this, props, responseCode);
     }
-    std::string errorLog() {
-        return log(this,
-		ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
+    std::string errorLog() const {
+        return log(*this,
+                   ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
     }
 
-    static std::string log(const RuleMessage *rm, int props, int code);
-    static std::string log(const RuleMessage *rm, int props) {
+    static std::string log(const RuleMessage &rm, int props, int code);
+    static std::string log(const RuleMessage &rm, int props) {
         return log(rm, props, -1);
     }
-    static std::string log(const RuleMessage *rm) {
+    static std::string log(const RuleMessage &rm) {
         return log(rm, 0);
     }
 
-    static std::string _details(const RuleMessage *rm);
-    static std::string _errorLogTail(const RuleMessage *rm);
+    static std::string _details(const RuleMessage &rm);
+    static std::string _errorLogTail(const RuleMessage &rm);
 
     int getPhase() const { return m_rule.getPhase() - 1; }
 

headers/modsecurity/rule_unconditional.h

@@ -36,7 +36,7 @@ class RuleUnconditional : public RuleWithActions {
  public:
     using RuleWithActions::RuleWithActions;
 
-    virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
+    virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
 };
 
 

headers/modsecurity/rule_with_actions.h

@@ -51,21 +51,21 @@ class RuleWithActions : public Rule {
 
     virtual bool evaluate(Transaction *transaction) override;
 
-    virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
+    virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
 
     void executeActionsIndependentOfChainedRuleResult(
         Transaction *trasn,
         bool *containsDisruptive,
-        std::shared_ptr<RuleMessage> ruleMessage);
+        RuleMessage &ruleMessage);
 
     void executeActionsAfterFullMatch(
         Transaction *trasn,
         bool containsDisruptive,
-        std::shared_ptr<RuleMessage> ruleMessage);
+        RuleMessage &ruleMessage);
 
     void executeAction(Transaction *trans,
         bool containsBlock,
-        std::shared_ptr<RuleMessage> ruleMessage,
+        RuleMessage &ruleMessage,
         actions::Action *a,
         bool context);
 
@@ -74,7 +74,7 @@ class RuleWithActions : public Rule {
         const Transaction *trasn, const std::string &value, TransformationResults &ret);
 
     void performLogging(Transaction *trans,
-        std::shared_ptr<RuleMessage> ruleMessage,
+        RuleMessage &ruleMessage,
         bool lastLog = true,
         bool chainedParentNull = false) const;
 

headers/modsecurity/rule_with_operator.h

@@ -47,16 +47,15 @@ class RuleWithOperator : public RuleWithActions {
 
     ~RuleWithOperator() override;
 
-    bool evaluate(Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
 
     void getVariablesExceptions(Transaction &t,
         variables::Variables *exclusion, variables::Variables *addition);
     inline void getFinalVars(variables::Variables *vars,
         variables::Variables *eclusion, Transaction *trans);
 
     bool executeOperatorAt(Transaction *trasn, const std::string &key,
-        const std::string &value, std::shared_ptr<RuleMessage> rm);
+        const std::string &value, RuleMessage &ruleMessage);
 
     static void updateMatchedVars(Transaction *trasn, const std::string &key,
         const std::string &value);

headers/modsecurity/transaction.h

@@ -407,7 +407,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
 #ifndef NO_LOGS
     void debug(int, const std::string &) const; // cppcheck-suppress functionStatic
 #endif
-    void serverLog(std::shared_ptr<RuleMessage> rm);
+    void serverLog(const RuleMessage &rm);
 
     int getRuleEngineState() const;
 

src/actions/audit_log.cc

@@ -27,11 +27,10 @@ namespace modsecurity {
 namespace actions {
 
 
-bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
-    rm->m_noAuditLog = false;
+bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
+    ruleMessage.m_noAuditLog = false;
     ms_dbg_a(transaction, 9, "Saving transaction to logs");
-    rm->m_saveMessage = true;
+    ruleMessage.m_saveMessage = true;
 
     return true;
 }

src/actions/audit_log.h

@@ -35,8 +35,7 @@ class AuditLog : public Action {
     explicit AuditLog(const std::string &action) 
         : Action(action) { }
 
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
 };
 
 

src/actions/block.cc

@@ -29,15 +29,14 @@ namespace modsecurity {
 namespace actions {
 
 
-bool Block::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+bool Block::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
     ms_dbg_a(transaction, 8, "Marking request as disruptive.");
 
     for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
         if (a->isDisruptive() == false) {
             continue;
         }
-        a->evaluate(rule, transaction, rm);
+        a->evaluate(rule, transaction, ruleMessage);
     }
 
     return true;

src/actions/block.h

@@ -35,8 +35,7 @@ class Block : public Action {
  public:
     explicit Block(const std::string &action) : Action(action) { }
 
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
 };
 
 

src/actions/data/status.cc

@@ -39,7 +39,7 @@ bool Status::init(std::string *error) {
 
 
 bool Status::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+    RuleMessage &ruleMessage) {
     transaction->m_it.status = m_status;
     return true;
 }

src/actions/data/status.h

@@ -37,8 +37,7 @@ class Status : public Action {
         : Action(action), m_status(0) { }
 
     bool init(std::string *error) override;
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
 
     int m_status;
 };

src/actions/disruptive/deny.cc

@@ -29,7 +29,7 @@ namespace disruptive {
 
 
 bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+    RuleMessage &ruleMessage) {
     ms_dbg_a(transaction, 8, "Running action deny");
 
     if (transaction->m_it.status == 200) {
@@ -38,9 +38,9 @@ bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
 
     transaction->m_it.disruptive = true;
     intervention::freeLog(&transaction->m_it);
-    rm->m_isDisruptive = true;
+    ruleMessage.m_isDisruptive = true;
     transaction->m_it.log = strdup(
-        rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
+        ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
 
     return true;
 }

src/actions/disruptive/deny.h

@@ -33,8 +33,7 @@ class Deny : public Action {
  public:
     explicit Deny(const std::string &action) : Action(action) { }
 
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
     bool isDisruptive() override { return true; }
 };
 

src/actions/disruptive/drop.cc

@@ -33,7 +33,7 @@ namespace disruptive {
 
 
 bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+    RuleMessage &ruleMessage) {
     ms_dbg_a(transaction, 8, "Running action drop " \
         "[executing deny instead of drop.]");
 
@@ -43,9 +43,9 @@ bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
 
     transaction->m_it.disruptive = true;
     intervention::freeLog(&transaction->m_it);
-    rm->m_isDisruptive = true;
+    ruleMessage.m_isDisruptive = true;
     transaction->m_it.log = strdup(
-        rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
+        ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
 
     return true;
 }

src/actions/disruptive/drop.h

@@ -32,8 +32,7 @@ class Drop : public Action {
  public:
     explicit Drop(const std::string &action) : Action(action) { }
 
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
     bool isDisruptive() override { return true; }
 };
 

src/actions/disruptive/pass.cc

@@ -30,7 +30,7 @@ namespace disruptive {
 
 
 bool Pass::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+    RuleMessage &ruleMessage) {
     intervention::free(&transaction->m_it);
     intervention::reset(&transaction->m_it);