Avoid passing RuleMessage by std::shared_ptr and use a reference inst…

…ead. - Avoids copying std::shared_ptr when lifetime of the RuleMessage is controlled by the caller.
eduar-hte · Sep 2, 2024 · 0fb37f8 · 0fb37f8
1 parent 91dee47
commit 0fb37f8
Show file tree

Hide file tree

Showing 86 changed files with 202 additions and 230 deletions.
diff --git a/examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h b/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;
         }
     }

diff --git a/examples/using_bodies_in_chunks/simple_request.cc b/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;
     }
 }

diff --git a/headers/modsecurity/actions/action.h b/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; }

diff --git a/headers/modsecurity/modsecurity.h b/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;
 

diff --git a/headers/modsecurity/rule.h b/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;

diff --git a/headers/modsecurity/rule_marker.h b/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);
     }
 

diff --git a/headers/modsecurity/rule_message.h b/headers/modsecurity/rule_message.h
@@ -59,30 +59,30 @@ class RuleMessage {
     RuleMessage(const RuleMessage &ruleMessage) = default;
     RuleMessage &operator=(const RuleMessage &ruleMessage) = delete;
 
-    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; }
 

diff --git a/headers/modsecurity/rule_unconditional.h b/headers/modsecurity/rule_unconditional.h
@@ -41,7 +41,7 @@ class RuleUnconditional : public RuleWithActions {
         int lineNumber)
         : RuleWithActions(actions, transformations, fileName, lineNumber) { }
 
-    virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
+    virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
 };
 
 

diff --git a/headers/modsecurity/rule_with_actions.h b/headers/modsecurity/rule_with_actions.h
@@ -47,21 +47,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);
 
@@ -70,9 +70,9 @@ class RuleWithActions : public Rule {
         const Transaction *trasn, const std::string &value, TransformationResults &ret);
 
     void performLogging(Transaction *trans,
-        std::shared_ptr<RuleMessage> ruleMessage,
+        const RuleMessage &ruleMessage,
         bool lastLog = true,
-        bool chainedParentNull = false);
+        bool chainedParentNull = false) const;
 
     std::vector<actions::Action *> getActionsByName(const std::string& name,
         Transaction *t);

diff --git a/headers/modsecurity/rule_with_operator.h b/headers/modsecurity/rule_with_operator.h
@@ -47,16 +47,15 @@ class RuleWithOperator : public RuleWithActions {
 
     virtual ~RuleWithOperator();
 
-    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);

diff --git a/headers/modsecurity/transaction.h b/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;
 

diff --git a/src/actions/audit_log.cc b/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;
 }

diff --git a/src/actions/audit_log.h b/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;
 };
 
 

diff --git a/src/actions/block.cc b/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;

diff --git a/src/actions/block.h b/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;
 };
 
 

diff --git a/src/actions/data/status.cc b/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;
 }

diff --git a/src/actions/data/status.h b/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;
 };

diff --git a/src/actions/disruptive/deny.cc b/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;
 }

diff --git a/src/actions/disruptive/deny.h b/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; }
 };
 

diff --git a/src/actions/disruptive/drop.cc b/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;
 }

diff --git a/src/actions/disruptive/drop.h b/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; }
 };
 

diff --git a/src/actions/disruptive/pass.cc b/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);
 

diff --git a/src/actions/disruptive/pass.h b/src/actions/disruptive/pass.h
@@ -31,8 +31,7 @@ class Pass : public Action {
  public:
     explicit Pass(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; }
 };
 

diff --git a/src/actions/disruptive/redirect.cc b/src/actions/disruptive/redirect.cc
@@ -35,7 +35,7 @@ bool Redirect::init(std::string *error) {
 
 
 bool Redirect::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+    RuleMessage &ruleMessage) {
     std::string m_urlExpanded(m_string->evaluate(transaction));
     /* if it was changed before, lets keep it. */
     if (transaction->m_it.status == 200
@@ -47,9 +47,9 @@ bool Redirect::evaluate(RuleWithActions *rule, Transaction *transaction,
     transaction->m_it.url = strdup(m_urlExpanded.c_str());
     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;
 }

diff --git a/src/actions/disruptive/redirect.h b/src/actions/disruptive/redirect.h
@@ -46,8 +46,7 @@ class Redirect : public Action {
             m_status(0),
             m_string(std::move(z)) { }
 
-    bool evaluate(RuleWithActions *rule, Transaction *transaction,
-        std::shared_ptr<RuleMessage> rm) override;
+    bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
     bool init(std::string *error) override;
     bool isDisruptive() override { return true; }
 

diff --git a/src/actions/log.cc b/src/actions/log.cc
@@ -28,10 +28,9 @@ namespace modsecurity {
 namespace actions {
 
 
-bool Log::evaluate(RuleWithActions *rule, Transaction *transaction,
-    std::shared_ptr<RuleMessage> rm) {
+bool Log::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
     ms_dbg_a(transaction, 9, "Saving transaction to logs");
-    rm->m_saveMessage = true;
+    ruleMessage.m_saveMessage = true;
     return true;
 }
 

diff --git a/src/actions/log.h b/src/actions/log.h
@@ -33,8 +33,7 @@ class Log : public Action {
     explicit Log(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;
 };
 
 }  // namespace actions