Moves default actions to be part of the rules

CHANGES

@@ -1,6 +1,16 @@
 v3.x.y - YYYY-MMM-DD (to be released)
 ------------------------------------- 
 
+  - Using std::shared_ptr instead of generates its own references counters
+    for Rules and related.
+    [@zimmerle]
+  - Better handle shared_pointers on messages aiming for better performance.
+    [@zimmerle]
+  - Better handle memory usage on transformations aiming for better
+    performance.
+    [@zimmerle]
+  - Coding refactoring on the Rule class. The Rule class is now refactored
+    into RuleWithOperator, RuleWithActions, and RuleUnconditional.
   - Fix: FILES variable does not use multipart part name for key
     [Issue #2377 - @martinhsv]
   - Regex key selection should not be case-sensitive

headers/modsecurity/rules.h

@@ -84,6 +84,8 @@ class Rules {
     std::shared_ptr<Rule> operator[](int index) const { return m_rules[index]; }
     std::shared_ptr<Rule> at(int index) const { return m_rules[index]; }
 
+    std::vector<std::shared_ptr<actions::Action> > m_defaultActions;
+
     std::vector<std::shared_ptr<Rule> > m_rules;
 };
 

headers/modsecurity/rules_set_properties.h

@@ -201,16 +201,6 @@ class RulesSetProperties {
     RulesSetProperties &operator =(const RulesSetProperties &r) = delete;
 
     ~RulesSetProperties() {
-        int i = 0;
-
-        for (i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-            std::vector<std::shared_ptr<actions::Action> > *tmp = \
-                &m_defaultActions[i];
-            while (tmp->empty() == false) {
-                tmp->pop_back();
-            }
-        }
-
         delete m_debugLog;
         delete m_auditLog;
     }
@@ -410,16 +400,6 @@ class RulesSetProperties {
             to->m_responseBodyTypeToBeInspected.m_set = true;
         }
 
-        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-            std::vector<std::shared_ptr<actions::Action> > *actions_from = \
-                &from->m_defaultActions[i];
-            std::vector<std::shared_ptr<actions::Action> > *actions_to = \
-                &to->m_defaultActions[i];
-            for (size_t j = 0; j < actions_from->size(); j++) {
-                actions_to->push_back(actions_from->at(j));
-            }
-        }
-
         if (to->m_auditLog) {
             std::string error;
             to->m_auditLog->merge(from->m_auditLog, &error);
@@ -481,8 +461,6 @@ class RulesSetProperties {
     ConfigString m_uploadTmpDirectory;
     ConfigString m_secArgumentSeparator;
     ConfigString m_secWebAppId;
-    std::vector<std::shared_ptr<actions::Action> > \
-        m_defaultActions[modsecurity::Phases::NUMBER_OF_PHASES];
     ConfigUnicodeMap m_unicodeMapTable;
 };
 

src/actions/block.cc

@@ -33,7 +33,7 @@ bool Block::evaluate(RuleWithActions *rule, Transaction *transaction,
     std::shared_ptr<RuleMessage> rm) {
     ms_dbg_a(transaction, 8, "Marking request as disruptive.");
 
-    for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
+    for (auto &a : transaction->m_rules->m_rulesSetPhases[rule->getPhase()]->m_defaultActions) {
         if (a->isDisruptive() == false) {
             continue;
         }

src/parser/seclang-parser.cc

@@ -2437,7 +2437,7 @@ namespace yy {
             YYERROR;
         }
 
-        if (!driver.m_defaultActions[definedPhase].empty()) {
+        if (!driver.m_rulesSetPhases[definedPhase]->m_defaultActions.empty()) {
             std::stringstream ss;
             ss << "SecDefaultActions can only be placed once per phase and configuration context. Phase ";
             ss << secRuleDefinedPhase;
@@ -2447,7 +2447,7 @@ namespace yy {
         }
 
         for (actions::Action *a : checkedActions) {
-            driver.m_defaultActions[definedPhase].push_back(
+            driver.m_rulesSetPhases[definedPhase]->m_defaultActions.push_back(
                 std::unique_ptr<actions::Action>(a));
         }
 

src/parser/seclang-parser.yy

@@ -1209,7 +1209,7 @@ expression:
             YYERROR;
         }
 
-        if (!driver.m_defaultActions[definedPhase].empty()) {
+        if (!driver.m_rulesSetPhases[definedPhase]->m_defaultActions.empty()) {
             std::stringstream ss;
             ss << "SecDefaultActions can only be placed once per phase and configuration context. Phase ";
             ss << secRuleDefinedPhase;
@@ -1219,7 +1219,7 @@ expression:
         }
 
         for (actions::Action *a : checkedActions) {
-            driver.m_defaultActions[definedPhase].push_back(
+            driver.m_rulesSetPhases[definedPhase]->m_defaultActions.push_back(
                 std::unique_ptr<actions::Action>(a));
         }
 

src/rule_with_actions.cc

@@ -222,7 +222,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans,
     bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage) {
     bool disruptiveAlreadyExecuted = false;
 
-    for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
+    for (auto &a : trans->m_rules->m_rulesSetPhases[getPhase()]->m_defaultActions) {
         if (a.get()->action_kind != actions::Action::RunTimeOnlyIfMatchKind) {
             continue;
         }
@@ -356,7 +356,7 @@ void RuleWithActions::executeTransformations(
     // Notice that first we make sure that won't be a t:none
     // on the target rule.
     if (none == 0) {
-        for (auto &a : trans->m_rules->m_defaultActions[getPhase()]) {
+        for (auto &a : trans->m_rules->m_rulesSetPhases[getPhase()]->m_defaultActions) {
             if (a->action_kind \
                 != actions::Action::RunTimeBeforeMatchAttemptKind) {
                 continue;

src/rules_set_phases.cc

@@ -61,6 +61,13 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) {
             return res;
         }
         amount_of_rules = amount_of_rules + res;
+
+        std::vector<std::shared_ptr<actions::Action> > *actions_from = &from->at(phase)->m_defaultActions;
+        std::vector<std::shared_ptr<actions::Action> > *actions_to = &at(phase)->m_defaultActions;
+
+        for (size_t j = 0; j < actions_from->size(); j++) {
+            actions_to->push_back(actions_from->at(j));
+        }
     }
 
     return amount_of_rules;