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.
  - The RuleMessage instance is created in RuleWithActions::evaluate and
    then used to call the overloaded version of this method that is
    specialized by subclasses.
  - Once the call to the overloaded method returns, the std::shared_ptr
    is destroyed as it's not stored by any of the callers, so it can
    be replaced with a stack variable and avoid paying the cost of
    copying the std::shared_ptr (and its control block that is
    guaranteed to be thread-safe and thus is not a straightforward
    pointer copy)
- Introduced RuleMessage::reset because this is required by
  RuleWithActions::performLogging when it's not the 'last log', the rule
  has multimatch and it's to be logged.
  - The current version is creating allocating another instance of
    RuleMessage on the heap to copy the Rule & Transaction related state
    while all the other members in the RuleMessage are set to their
    default values.
  - The new version leverages the existent, unused and incomplete
    function 'clean' (renamed as 'reset') to do this on the current
    instance.
    - Notice that the current code preserves the value of m_saveMessage,
      so 'reset' provides an argument for the caller to control whether
      this member should be reinitialized.

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);
 

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; }
 };