Yet another refactoring in Rule

headers/modsecurity/rule.h

@@ -56,11 +56,19 @@ using TransformationResult = std::pair<std::shared_ptr<std::string>,
     std::shared_ptr<std::string>>;
 using TransformationResults = std::list<TransformationResult>;
 
+using Transformation = actions::transformations::Transformation;
+using Transformations = std::vector<Transformation *>;
+
+using Tags = std::vector<actions::Tag *>;
+using SetVars = std::vector<actions::SetVar *>;
+using MatchActions = std::vector<actions::Action *>;
+
 class Rule {
  public:
-    Rule(operators::Operator *_op,
+    Rule(operators::Operator *op,
-            variables::Variables *_variables,
+            variables::Variables *variables,
-            std::vector<actions::Action *> *_actions,
+            std::vector<actions::Action *> *actions,
+            Transformations *transformations,
             std::unique_ptr<std::string> fileName,
             int lineNumber);
     explicit Rule(const std::string &marker);
@@ -108,37 +116,67 @@ class Rule {
         int *nth) const;
 
 
-    actions::Action *m_theDisruptiveAction;
+
-    actions::LogData *m_logData;
+    inline bool isUnconditional() const { return m_operator == NULL; }
-    actions::Msg *m_msg;
+
-    actions::Severity *m_severity;
+    virtual bool isMarker() { return m_isSecMarker; }
-    bool m_chained;
+
-    bool m_containsCaptureAction;
+    inline bool isChained() const { return m_isChained == true; }
-    bool m_containsMultiMatchAction;
+    inline bool hasCaptureAction() const { return m_containsCaptureAction == true; }
-    bool m_containsStaticBlockAction;
+    inline void setChained(bool b) { m_isChained = b; }
-    bool m_secMarker;
+    inline bool hasDisruptiveAction() const { return m_disruptiveAction != NULL; }
+
+
+    inline bool hasLogData() const { return m_logData != NULL; }
+    std::string logData(Transaction *t);
+    inline bool hasMsg() const { return m_msg != NULL; }
+    std::string msg(Transaction *t);
+    inline bool hasSeverity() const { return m_severity != NULL; }
+    int severity() const;
+    int getPhase() const { return m_phase; }
+    void setPhase(int phase) { m_phase = phase; }
+
+    std::string getOperatorName() const;
+
     int64_t m_ruleId;
-    int m_accuracy;
-    int m_lineNumber;
-    int m_maturity;
-    int m_phase;
-    modsecurity::variables::Variables *m_variables;
-    operators::Operator *m_op;
     std::unique_ptr<Rule> m_chainedRuleChild;
     Rule *m_chainedRuleParent;
+
     std::shared_ptr<std::string> m_fileName;
+
     std::string m_marker;
     std::string m_rev;
     std::string m_ver;
-    std::vector<actions::Action *> m_actionsRuntimePos;
+    int m_accuracy;
-    std::vector<actions::Action *> m_actionsRuntimePre;
+    int m_maturity;
-    std::vector<actions::SetVar *> m_actionsSetVar;
+    int m_lineNumber;
-    std::vector<actions::Tag *> m_actionsTag;
 
  private:
-    bool m_unconditional;
+    modsecurity::variables::Variables *m_variables;
-};
+    operators::Operator *m_operator;
 
+    /* actions */
+    actions::Action *m_disruptiveAction;
+    actions::LogData *m_logData;
+    actions::Msg *m_msg;
+    actions::Severity *m_severity;
+    MatchActions m_actionsRuntimePos;
+    SetVars m_actionsSetVar;
+    Tags m_actionsTag;
+
+    /* actions > transformations */
+    Transformations m_transformations;
+
+    bool m_containsCaptureAction:1;
+    bool m_containsMultiMatchAction:1;
+    bool m_containsStaticBlockAction:1;
+    bool m_isChained:1;
+    bool m_isSecMarker:1;
+    bool m_unconditional:1;
+
+    int m_phase;
+
+};
 
 }  // namespace modsecurity
 #endif

headers/modsecurity/rule_message.h

@@ -51,7 +51,7 @@ class RuleMessage {
         m_maturity(rule->m_maturity),
         m_message(""),
         m_noAuditLog(false),
-        m_phase(rule->m_phase - 1),
+        m_phase(rule->getPhase() - 1),
         m_reference(""),
         m_rev(rule->m_rev),
         m_rule(rule),

headers/modsecurity/rules_set_phases.h

@@ -39,17 +39,18 @@ class Rule;
 /** @ingroup ModSecurity_CPP_API */
 class RulesSetPhases {
  public:
-    ~RulesSetPhases();
 
     bool insert(std::shared_ptr<Rule> rule);
 
     int append(RulesSetPhases *from, std::ostringstream *err);
     void dump() const;
 
-    Rules *operator[](int index) { return &m_rules[index]; }
+    Rules *operator[](int index) { return &m_rulesAtPhase[index]; }
-    Rules *at(int index) { return &m_rules[index]; }
+    Rules *at(int index) { return &m_rulesAtPhase[index]; }
+
+ private:
+    Rules m_rulesAtPhase[8];
 
-    Rules m_rules[8];
 };
 
 

src/actions/block.cc

@@ -33,7 +33,7 @@ bool Block::evaluate(Rule *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->m_phase]) {
+    for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
         if (a->isDisruptive() == false) {
             continue;
         }

src/actions/chain.cc

@@ -26,7 +26,7 @@ namespace actions {
 
 
 bool Chain::evaluate(Rule *rule, Transaction *transaction) {
-    rule->m_chained = true;
+    rule->setChained(true);
     return true;
 }
 

src/actions/phase.cc

@@ -73,7 +73,7 @@ bool Phase::init(std::string *error) {
 
 
 bool Phase::evaluate(Rule *rule, Transaction *transaction) {
-    rule->m_phase = m_phase;
+    rule->setPhase(m_phase);
     return true;
 }
 

src/operators/detect_sqli.cc

@@ -38,7 +38,7 @@ bool DetectSQLi::evaluate(Transaction *t, Rule *rule,
             ms_dbg_a(t, 4, "detected SQLi using libinjection with " \
                 "fingerprint '" + std::string(fingerprint) + "' at: '" +
                 input + "'");
-            if (rule && t && rule->m_containsCaptureAction) {
+            if (rule && t && rule->hasCaptureAction()) {
                 t->m_collections.m_tx_collection->storeOrUpdateFirst(
                     "0", std::string(fingerprint));
                 ms_dbg_a(t, 7, "Added DetectSQLi match TX.0: " + \

src/operators/detect_xss.cc

@@ -34,7 +34,7 @@ bool DetectXSS::evaluate(Transaction *t, Rule *rule,
     if (t) {
         if (is_xss) {
             ms_dbg_a(t, 5, "detected XSS using libinjection.");
-            if (rule && t && rule->m_containsCaptureAction) {
+            if (rule && t && rule->hasCaptureAction()) {
                 t->m_collections.m_tx_collection->storeOrUpdateFirst(
                     "0", std::string(input));
                 ms_dbg_a(t, 7, "Added DetectXSS match TX.0: " + \

src/operators/pm.cc

@@ -97,16 +97,16 @@ bool Pm::evaluate(Transaction *transaction, Rule *rule,
 #endif
 
     if (rc >= 0 && transaction) {
-        std::string match_(match);
+        std::string match_(match?match:"");
         logOffset(ruleMessage, rc - match_.size() + 1, match_.size());
         transaction->m_matched.push_back(match_);
-    }
 
-    if (rule && rule->m_containsCaptureAction && transaction && rc >= 0) {
+        if (rule && rule->hasCaptureAction()) {
-        transaction->m_collections.m_tx_collection->storeOrUpdateFirst("0",
+            transaction->m_collections.m_tx_collection->storeOrUpdateFirst("0",
-            std::string(match));
+                match_);
-        ms_dbg_a(transaction, 7, "Added pm match TX.0: " + \
+            ms_dbg_a(transaction, 7, "Added pm match TX.0: " + \
-            std::string(match));
+                match_);
+        }
     }
 
     return rc >= 0;

src/operators/rbl.cc

@@ -226,7 +226,7 @@ bool Rbl::evaluate(Transaction *t, Rule *rule,
     furtherInfo(sin, ipStr, t, m_provider);
 
     freeaddrinfo(info);
-    if (rule && t && rule->m_containsCaptureAction) {
+    if (rule && t && rule->hasCaptureAction()) {
         t->m_collections.m_tx_collection->storeOrUpdateFirst(
         "0", std::string(ipStr));
         ms_dbg_a(t, 7, "Added RXL match TX.0: " + \

src/operators/rx.cc

@@ -53,7 +53,7 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule,
     }
 
     matches = re->searchAll(input);
-    if (rule && rule->m_containsCaptureAction && transaction) {
+    if (rule && rule->hasCaptureAction() && transaction) {
         int i = 0;
         matches.reverse();
         for (const SMatch& a : matches) {

src/operators/verify_cc.cc

@@ -141,7 +141,7 @@ bool VerifyCC::evaluate(Transaction *t, Rule *rule,
             int is_cc = luhnVerify(match.c_str(), match.size());
             if (is_cc) {
                 if (t) {
-                    if (rule && t && rule->m_containsCaptureAction) {
+                    if (rule && t && rule->hasCaptureAction()) {
                         t->m_collections.m_tx_collection->storeOrUpdateFirst(
                             "0", std::string(match));
                         ms_dbg_a(t, 7, "Added VerifyCC match TX.0: " + \

src/operators/verify_cpf.cc

@@ -124,7 +124,7 @@ bool VerifyCPF::evaluate(Transaction *t, Rule *rule,
             is_cpf = verify(m.str().c_str(), m.str().size());
             if (is_cpf) {
                 logOffset(ruleMessage, m.offset(), m.str().size());
-                if (rule && t && rule->m_containsCaptureAction) {
+                if (rule && t && rule->hasCaptureAction()) {
                     t->m_collections.m_tx_collection->storeOrUpdateFirst(
                         "0", m.str());
                     ms_dbg_a(t, 7, "Added VerifyCPF match TX.0: " + \

src/operators/verify_ssn.cc

@@ -126,7 +126,7 @@ bool VerifySSN::evaluate(Transaction *t, Rule *rule,
             is_ssn = verify(j.str().c_str(), j.str().size());
             if (is_ssn) {
                 logOffset(ruleMessage, j.offset(), j.str().size());
-                if (rule && t && rule->m_containsCaptureAction) {
+                if (rule && t && rule->hasCaptureAction()) {
                     t->m_collections.m_tx_collection->storeOrUpdateFirst(
                         "0", j.str());
                     ms_dbg_a(t, 7, "Added VerifySSN match TX.0: " + \

src/operators/verify_svnr.cc

@@ -94,7 +94,7 @@ bool VerifySVNR::evaluate(Transaction *t, Rule *rule,
             is_svnr = verify(j.str().c_str(), j.str().size());
             if (is_svnr) {
                 logOffset(ruleMessage, j.offset(), j.str().size());
-                if (rule && t && rule->m_containsCaptureAction) {
+                if (rule && t && rule->hasCaptureAction()) {
                     t->m_collections.m_tx_collection->storeOrUpdateFirst(
                         "0", j.str());
                     ms_dbg_a(t, 7, "Added VerifySVNR match TX.0: " + \

src/parser/driver.cc

@@ -44,7 +44,7 @@ Driver::~Driver() {
 int Driver::addSecMarker(std::string marker) {
     for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
         std::unique_ptr<Rule> rule(new Rule(marker));
-        rule->m_phase = i;
+        rule->setPhase(i);
         m_rulesSetPhases.insert(std::move(rule));
     }
     return 0;
@@ -52,8 +52,8 @@ int Driver::addSecMarker(std::string marker) {
 
 
 int Driver::addSecAction(std::unique_ptr<Rule> rule) {
-    if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
+    if (rule->getPhase() >= modsecurity::Phases::NUMBER_OF_PHASES) {
-        m_parserError << "Unknown phase: " << std::to_string(rule->m_phase);
+        m_parserError << "Unknown phase: " << std::to_string(rule->getPhase());
         m_parserError << std::endl;
         return false;
     }
@@ -72,16 +72,16 @@ int Driver::addSecRuleScript(std::unique_ptr<RuleScript> rule) {
 
 
 int Driver::addSecRule(std::unique_ptr<Rule> r) {
-    if (r->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
+    if (r->getPhase() >= modsecurity::Phases::NUMBER_OF_PHASES) {
-        m_parserError << "Unknown phase: " << std::to_string(r->m_phase);
+        m_parserError << "Unknown phase: " << std::to_string(r->getPhase());
         m_parserError << std::endl;
         return false;
     }
 
     /* is it a chained rule? */
-    if (m_lastRule != nullptr && m_lastRule->m_chained) {
+    if (m_lastRule != nullptr && m_lastRule->isChained()) {
-        r->m_phase = m_lastRule->m_phase;
+        r->setPhase(m_lastRule->getPhase());
-        if (r->m_theDisruptiveAction) {
+        if (r->hasDisruptiveAction()) {
             m_parserError << "Disruptive actions can only be specified by";
             m_parserError << " chain starter rules.";
             return false;
@@ -148,7 +148,7 @@ int Driver::parse(const std::string &f, const std::string &ref) {
      *
      */
     /*
-    if (m_lastRule != nullptr && m_lastRule->m_chained) {
+    if (m_lastRule != nullptr && m_lastRule->isChained()) {
         m_parserError << "Last rule is marked as chained but there " \
             "isn't a subsequent rule." << std::endl;
         return false;

src/parser/seclang-parser.hh

@@ -99,12 +99,12 @@ class Driver;
 #include "src/actions/skip_after.h"
 #include "src/actions/skip.h"
 #include "src/actions/tag.h"
-#include "src/actions/transformations/none.h"
-#include "src/actions/transformations/transformation.h"
-#include "src/actions/transformations/url_decode_uni.h"
 #include "src/actions/ver.h"
 #include "src/actions/xmlns.h"
 
+#include "src/actions/transformations/none.h"
+#include "src/actions/transformations/transformation.h"
+#include "src/actions/transformations/url_decode_uni.h"
 #include "src/actions/transformations/hex_encode.h"
 #include "src/actions/transformations/parity_even_7bit.h"
 #include "src/actions/transformations/utf8_to_unicode.h"

src/parser/seclang-parser.yy

@@ -61,12 +61,12 @@ class Driver;
 #include "src/actions/skip_after.h"
 #include "src/actions/skip.h"
 #include "src/actions/tag.h"
-#include "src/actions/transformations/none.h"
-#include "src/actions/transformations/transformation.h"
-#include "src/actions/transformations/url_decode_uni.h"
 #include "src/actions/ver.h"
 #include "src/actions/xmlns.h"
 
+#include "src/actions/transformations/none.h"
+#include "src/actions/transformations/transformation.h"
+#include "src/actions/transformations/url_decode_uni.h"
 #include "src/actions/transformations/hex_encode.h"
 #include "src/actions/transformations/parity_even_7bit.h"
 #include "src/actions/transformations/utf8_to_unicode.h"
@@ -1067,8 +1067,13 @@ expression:
     | DIRECTIVE variables op actions
       {
         std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
+        std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
         for (auto &i : *$4.get()) {
-            a->push_back(i.release());
+            if (dynamic_cast<actions::transformations::Transformation *>(i.get())) {
+              t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release()));
+            } else {
+              a->push_back(i.release());
+            }
         }
         variables::Variables *v = new variables::Variables();
         for (auto &i : *$2.get()) {
@@ -1080,6 +1085,7 @@ expression:
             /* op */ op,
             /* variables */ v,
             /* actions */ a,
+            /* transformations */ t,
             /* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
             /* line number */ @1.end.line
             ));
@@ -1099,6 +1105,7 @@ expression:
             /* op */ $3.release(),
             /* variables */ v,
             /* actions */ NULL,
+            /* transformations */ NULL,
             /* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
             /* line number */ @1.end.line
             ));
@@ -1109,13 +1116,19 @@ expression:
     | CONFIG_DIR_SEC_ACTION actions
       {
         std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
+        std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
         for (auto &i : *$2.get()) {
-            a->push_back(i.release());
+            if (dynamic_cast<actions::transformations::Transformation *>(i.get())) {
+              t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release()));
+            } else {
+              a->push_back(i.release());
+            }
         }
         std::unique_ptr<Rule> rule(new Rule(
             /* op */ NULL,
             /* variables */ NULL,
             /* actions */ a,
+            /* transformations */ t,
             /* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
             /* line number */ @1.end.line
             ));
@@ -1125,12 +1138,18 @@ expression:
       {
         std::string err;
         std::vector<actions::Action *> *a = new std::vector<actions::Action *>();
+        std::vector<actions::transformations::Transformation *> *t = new std::vector<actions::transformations::Transformation *>();
         for (auto &i : *$2.get()) {
-            a->push_back(i.release());
+            if (dynamic_cast<actions::transformations::Transformation *>(i.get())) {
+              t->push_back(dynamic_cast<actions::transformations::Transformation *>(i.release()));
+            } else {
+              a->push_back(i.release());
+            }
         }
         std::unique_ptr<RuleScript> r(new RuleScript(
             /* path to script */ $1,
             /* actions */ a,
+            /* transformations */ t,
             /* file name */ std::unique_ptr<std::string>(new std::string(*@1.end.filename)),
             /* line number */ @1.end.line
             ));
@@ -1164,7 +1183,7 @@ expression:
                 delete phase;
             } else if (a->action_kind == actions::Action::RunTimeOnlyIfMatchKind ||
                 a->action_kind == actions::Action::RunTimeBeforeMatchAttemptKind) {
-                actions::transformations::None *none = dynamic_cast<actions::transformations::None *>(a);
+                                actions::transformations::None *none = dynamic_cast<actions::transformations::None *>(a);
                 if (none != NULL) {
                     driver.error(@0, "The transformation none is not suitable to be part of the SecDefaultActions");
                     YYERROR;

src/rule.cc

@@ -52,67 +52,68 @@ using actions::transformations::None;
 using actions::transformations::Transformation;
 
 Rule::Rule(const std::string &marker)
-    : m_theDisruptiveAction(nullptr),
+    : m_ruleId(0),
-    m_logData(nullptr),
-    m_msg(nullptr),
-    m_severity(nullptr),
-    m_chained(false),
-    m_containsCaptureAction(false),
-    m_containsMultiMatchAction(false),
-    m_containsStaticBlockAction(false),
-    m_secMarker(true),
-    m_ruleId(0),
-    m_accuracy(0),
-    m_lineNumber(0),
-    m_maturity(0),
-    m_phase(-1),
-    m_variables(NULL),
-    m_op(NULL),
     m_chainedRuleChild(nullptr),
     m_chainedRuleParent(NULL),
     /* m_fileName(""), */
     m_marker(marker),
     m_rev(""),
     m_ver(""),
-    m_actionsRuntimePos(),
+    m_accuracy(0),
-    m_actionsRuntimePre(),
+    m_maturity(0),
-    m_actionsSetVar(),
+    m_lineNumber(0),
-    m_actionsTag(),
+    m_variables(NULL),
-    m_unconditional(false) { }
+    m_operator(NULL),
-
+    m_disruptiveAction(nullptr),
-Rule::Rule(Operator *_op,
-    variables::Variables *_variables,
-    std::vector<Action *> *actions,
-    std::unique_ptr<std::string> fileName,
-    int lineNumber)
-    : m_theDisruptiveAction(nullptr),
     m_logData(nullptr),
     m_msg(nullptr),
     m_severity(nullptr),
-    m_chained(false),
+    m_actionsRuntimePos(),
+    m_actionsSetVar(),
+    m_actionsTag(),
+    m_transformations(),
     m_containsCaptureAction(false),
     m_containsMultiMatchAction(false),
     m_containsStaticBlockAction(false),
-    m_secMarker(false),
+    m_isChained(false),
-    m_ruleId(0),
+    m_isSecMarker(true),
-    m_accuracy(0),
+    m_unconditional(false),
-    m_lineNumber(lineNumber),
+    m_phase(-1) { }
-    m_maturity(0),
+
-    m_phase(-1),
+Rule::Rule(Operator *op,
-    m_variables(_variables),
+    variables::Variables *variables,
-    m_op(_op),
+    std::vector<Action *> *actions,
+    Transformations *transformations,
+    std::unique_ptr<std::string> fileName,
+    int lineNumber)
+    : m_ruleId(0),
     m_chainedRuleChild(nullptr),
     m_chainedRuleParent(NULL),
     m_fileName(std::move(fileName)),
     m_marker(""),
     m_rev(""),
     m_ver(""),
+    m_accuracy(0),
+    m_maturity(0),
+    m_lineNumber(lineNumber),
+    m_variables(variables),
+    m_operator(op),
+    m_disruptiveAction(nullptr),
+    m_logData(nullptr),
+    m_msg(nullptr),
+    m_severity(nullptr),
     m_actionsRuntimePos(),
-    m_actionsRuntimePre(),
     m_actionsSetVar(),
     m_actionsTag(),
-    m_unconditional(false) {
+    m_transformations(transformations != NULL ? *transformations : Transformations()),
-    /* */
+    m_containsCaptureAction(false),
+    m_containsMultiMatchAction(false),
+    m_containsStaticBlockAction(false),
+    m_isChained(false),
+    m_isSecMarker(false),
+    m_unconditional(false),
+    m_phase(-1) {
+
     organizeActions(actions);
 
     /**
@@ -123,15 +124,13 @@ Rule::Rule(Operator *_op,
         m_phase = modsecurity::Phases::RequestHeadersPhase;
     }
 
-    m_unconditional = (m_op == NULL);
-
     delete actions;
 }
 
 
 Rule::~Rule() {
-    if (m_op != NULL) {
+    if (m_operator != NULL) {
-        delete m_op;
+        delete m_operator;
     }
 
     cleanUpActions();
@@ -156,8 +155,6 @@ void Rule::organizeActions(std::vector<Action *> *actions) {
         if (a->action_kind == Action::ConfigurationKind) {
             a->evaluate(this, NULL);
             delete a;
-        } else if (a->action_kind == Action::RunTimeBeforeMatchAttemptKind) {
-            m_actionsRuntimePre.push_back(a);
         } else if (a->action_kind == Action::RunTimeOnlyIfMatchKind) {
             if (dynamic_cast<actions::Capture *>(a)) {
                 m_containsCaptureAction = true;
@@ -180,11 +177,11 @@ void Rule::organizeActions(std::vector<Action *> *actions) {
                 m_actionsRuntimePos.push_back(a);
                 m_containsStaticBlockAction = true;
             } else if (a->isDisruptive() == true) {
-                if (m_theDisruptiveAction != nullptr) {
+                if (m_disruptiveAction != nullptr) {
-                    delete m_theDisruptiveAction;
+                    delete m_disruptiveAction;
-                    m_theDisruptiveAction = nullptr;
+                    m_disruptiveAction = nullptr;
                 }
-                m_theDisruptiveAction = a;
+                m_disruptiveAction = a;
             } else {
                 m_actionsRuntimePos.push_back(a);
             }
@@ -210,9 +207,9 @@ void Rule::cleanUpActions() {
         delete m_msg;
         m_msg = nullptr;
     }
-    while (m_actionsRuntimePre.empty() == false) {
+    while (m_transformations.empty() == false) {
-        auto *a = m_actionsRuntimePre.back();
+        auto *a = m_transformations.back();
-        m_actionsRuntimePre.pop_back();
+        m_transformations.pop_back();
         delete a;
     }
     while (m_actionsRuntimePos.empty() == false) {
@@ -230,9 +227,9 @@ void Rule::cleanUpActions() {
         m_actionsTag.pop_back();
         delete a;
     }
-    if (m_theDisruptiveAction != nullptr) {
+    if (m_disruptiveAction != nullptr) {
-        delete m_theDisruptiveAction;
+        delete m_disruptiveAction;
-        m_theDisruptiveAction = nullptr;
+        m_disruptiveAction = nullptr;
     }
 }
 
@@ -310,7 +307,7 @@ bool Rule::executeOperatorAt(Transaction *trans, const std::string &key,
         utils::string::toHexIfNeeded(value)) \
         + "\" (Variable: " + key + ")");
 
-    ret = this->m_op->evaluateInternal(trans, this, value, ruleMessage);
+    ret = this->m_operator->evaluateInternal(trans, this, value, ruleMessage);
     if (ret == false) {
         return false;
     }
@@ -374,7 +371,7 @@ void Rule::executeTransformations(
             std::shared_ptr<std::string>(new std::string(path))));
     }
 
-    for (Action *a : this->m_actionsRuntimePre) {
+    for (Action *a : m_transformations) {
         if (a->m_isNone) {
             none++;
         }
@@ -397,7 +394,7 @@ void Rule::executeTransformations(
         }
     }
 
-    for (Action *a : this->m_actionsRuntimePre) {
+    for (Transformation *a : m_transformations) {
         if (none == 0) {
             Transformation *t = dynamic_cast<Transformation *>(a);
             executeTransformation(t, &value, trans, &ret, &path,
@@ -408,12 +405,14 @@ void Rule::executeTransformations(
         }
     }
 
+    // FIXME: It can't be something different from transformation. Sort this
+    //        on rules compile time.
     for (auto &b :
         trans->m_rules->m_exceptions.m_action_pre_update_target_by_id) {
         if (m_ruleId != b.first) {
             continue;
         }
-        actions::Action *a = dynamic_cast<actions::Action*>(b.second.get());
+        Transformation *a = dynamic_cast<Transformation*>(b.second.get());
         if (a->m_isNone) {
             none++;
         }
@@ -424,7 +423,7 @@ void Rule::executeTransformations(
         if (m_ruleId != b.first) {
             continue;
         }
-        actions::Action *a = dynamic_cast<actions::Action*>(b.second.get());
+        Transformation *a = dynamic_cast<Transformation*>(b.second.get());
         if (none == 0) {
             Transformation *t = dynamic_cast<Transformation *>(a);
             executeTransformation(t, &value, trans, &ret, &path,
@@ -598,9 +597,9 @@ void Rule::executeActionsAfterFullMatch(Transaction *trans,
             executeAction(trans, containsBlock, ruleMessage, a, false);
         }
     }
-    if (!disruptiveAlreadyExecuted && m_theDisruptiveAction != nullptr) {
+    if (!disruptiveAlreadyExecuted && m_disruptiveAction != nullptr) {
         executeAction(trans, containsBlock, ruleMessage,
-            m_theDisruptiveAction, false);
+            m_disruptiveAction, false);
     }
 }
 
@@ -623,11 +622,11 @@ bool Rule::evaluate(Transaction *trans,
 
     trans->m_matched.clear();
 
-    if (m_secMarker == true) {
+    if (isMarker() == true) {
         return true;
     }
 
-    if (m_unconditional == true) {
+    if (isUnconditional() == true) {
         ms_dbg_a(trans, 4, "(Rule: " + std::to_string(m_ruleId) \
             + ") Executing unconditional rule...");
         executeActionsIndependentOfChainedRuleResult(trans,
@@ -652,24 +651,24 @@ bool Rule::evaluate(Transaction *trans,
         return true;
     }
 
-    if (m_op->m_string) {
+    if (m_operator->m_string) {
-        eparam = m_op->m_string->evaluate(trans);
+        eparam = m_operator->m_string->evaluate(trans);
 
-        if (m_op->m_string->containsMacro()) {
+        if (m_operator->m_string->containsMacro()) {
             eparam = "\"" + eparam + "\" Was: \"" \
-                + m_op->m_string->evaluate(NULL) + "\"";
+                + m_operator->m_string->evaluate(NULL) + "\"";
         } else {
             eparam = "\"" + eparam + "\"";
         }
     ms_dbg_a(trans, 4, "(Rule: " + std::to_string(m_ruleId) \
-        + ") Executing operator \"" + this->m_op->m_op \
+        + ") Executing operator \"" + getOperatorName() \
         + "\" with param " \
         + eparam \
         + " against " \
         + variables + ".");
     } else {
         ms_dbg_a(trans, 4, "(Rule: " + std::to_string(m_ruleId) \
-            + ") Executing operator \"" + this->m_op->m_op \
+            + ") Executing operator \"" + getOperatorName() \
             + " against " \
             + variables + ".");
     }
@@ -720,7 +719,7 @@ bool Rule::evaluate(Transaction *trans,
                 ret = executeOperatorAt(trans, key, valueAfterTrans, ruleMessage);
 
                 if (ret == true) {
-                    ruleMessage->m_match = m_op->resolveMatchMessage(trans,
+                    ruleMessage->m_match = m_operator->resolveMatchMessage(trans,
                         key, value);
                     for (auto &i : v->getOrigin()) {
                         ruleMessage->m_reference.append(i->toText());
@@ -763,7 +762,7 @@ bool Rule::evaluate(Transaction *trans,
     }
     ms_dbg_a(trans, 4, "Rule returned 1.");
 
-    if (this->m_chained == false) {
+    if (this->isChained() == false) {
         goto end_exec;
     }
 
@@ -811,7 +810,7 @@ std::vector<actions::Action *> Rule::getActionsByName(const std::string& name,
             ret.push_back(z);
         }
     }
-    for (auto &z : m_actionsRuntimePre) {
+    for (auto &z : m_transformations) {
         if (*z->m_name.get() == name) {
             ret.push_back(z);
         }
@@ -854,5 +853,9 @@ bool Rule::containsMsg(const std::string& name, Transaction *t) {
     return m_msg && m_msg->data(t) == name;
 }
 
+std::string Rule::getOperatorName() const { return m_operator->m_op; }
+std::string Rule::logData(Transaction *t) { return m_logData->data(t); }
+std::string Rule::msg(Transaction *t) { return m_msg->data(t); }
+int Rule::severity() const { return m_severity->m_severity; }
 
 }  // namespace modsecurity

src/rule_message.cc

@@ -79,7 +79,7 @@ std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
             msg.append(std::to_string(code));
         }
         msg.append(" (phase ");
-        msg.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
+        msg.append(std::to_string(rm->m_rule->getPhase() - 1) + "). ");
     } else {
         msg.append("ModSecurity: Warning. ");
     }

src/rule_script.h

@@ -46,9 +46,10 @@ class RuleScript : public Rule {
  public:
     RuleScript(const std::string &name,
         std::vector<Action *> *actions,
+        Transformations *t,
         std::unique_ptr<std::string> fileName,
         int lineNumber)
-            : Rule(NULL, NULL, actions, std::move(fileName), lineNumber),
+            : Rule(NULL, NULL, actions, t, std::move(fileName), lineNumber),
         m_name(name) { }
 
     bool init(std::string *err);

src/rules_set.cc

@@ -142,7 +142,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
 #endif
             ms_dbg_a(t, 9, "Rule: " + rule->m_marker);
 
-            if (rule->m_secMarker && rule->m_marker == t->m_marker) {
+            if (rule->isMarker() && rule->m_marker == t->m_marker) {
                 ms_dbg_a(t, 4, "Out of a SecMarker after skip " \
                     + std::to_string(m_secmarker_skipped) + " rules.");
                 t->m_marker.clear();

src/rules_set_phases.cc

@@ -28,14 +28,13 @@
 
 namespace modsecurity {
 
-RulesSetPhases::~RulesSetPhases() {
-}
 
 bool RulesSetPhases::insert(std::shared_ptr<Rule> rule) {
-    if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
+    if (rule->getPhase() >= modsecurity::Phases::NUMBER_OF_PHASES) {
         return false;
     }
-    m_rules[rule->m_phase].insert(rule);
+    m_rulesAtPhase[rule->getPhase()].insert(rule);
+
     return true;
 }
 
@@ -45,10 +44,10 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) {
     std::vector<int64_t> v;
 
     for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-        v.reserve(m_rules[i].size());
+        v.reserve(m_rulesAtPhase[i].size());
-        for (size_t z = 0; z < m_rules[i].size(); z++) {
+        for (size_t z = 0; z < m_rulesAtPhase[i].size(); z++) {
-            Rule *rule_ckc = m_rules[i].at(z).get();
+            Rule *rule_ckc = m_rulesAtPhase[i].at(z).get();
-            if (rule_ckc->m_secMarker == true) {
+            if (rule_ckc->isMarker() == true) {
                 continue;
             }
             v.push_back(rule_ckc->m_ruleId);
@@ -57,9 +56,11 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) {
     std::sort (v.begin(), v.end());
 
     for (int phase = 0; phase < modsecurity::Phases::NUMBER_OF_PHASES; phase++) {
-        if (m_rules[phase].append(from->at(phase), v, err) < 0) {
+        int res = m_rulesAtPhase[phase].append(from->at(phase), v, err);
-            return -1;
+        if (res < 0) {
+            return res;
         }
+        amount_of_rules = amount_of_rules + res;
     }
 
     return amount_of_rules;
@@ -68,9 +69,9 @@ int RulesSetPhases::append(RulesSetPhases *from, std::ostringstream *err) {
 void RulesSetPhases::dump() const {
     for (int i = 0; i <= modsecurity::Phases::NUMBER_OF_PHASES; i++) {
         std::cout << "Phase: " << std::to_string(i);
-        std::cout << " (" << std::to_string(m_rules[i].size());
+        std::cout << " (" << std::to_string(m_rulesAtPhase[i].size());
         std::cout << " rules)" << std::endl;
-        m_rules[i].dump();
+        m_rulesAtPhase[i].dump();
     }
 }
 

src/variables/rule.h

@@ -93,13 +93,13 @@ class Rule_DictElement : public VariableDictElement { \
         std::vector<const VariableValue *> *l) {
         Rule *r = rule;
 
-        while (r && !r->m_severity) {
+        while (r && !r->hasSeverity()) {
             r = r->m_chainedRuleParent;
         }
 
-        if (r && r->m_severity) {
+        if (r && r->hasSeverity()) {
             std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
-            std::string *a = new std::string(std::to_string(r->m_severity->m_severity));
+            std::string *a = new std::string(std::to_string(r->severity()));
             VariableValue *var = new VariableValue(&m_rule, &m_rule_severity,
                 a
             );
@@ -117,13 +117,13 @@ class Rule_DictElement : public VariableDictElement { \
         std::vector<const VariableValue *> *l) {
         Rule *r = rule;
 
-        while (r && !r->m_logData) {
+        while (r && !r->hasLogData()) {
             r = r->m_chainedRuleParent;
         }
 
-        if (r && r->m_logData) {
+        if (r && r->hasLogData()) {
             std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
-            std::string *a = new std::string(r->m_logData->data(t));
+            std::string *a = new std::string(r->logData(t));
             VariableValue *var = new VariableValue(&m_rule, &m_rule_logdata,
                 a
             );
@@ -140,13 +140,13 @@ class Rule_DictElement : public VariableDictElement { \
         std::vector<const VariableValue *> *l) {
         Rule *r = rule;
 
-        while (r && !r->m_msg) {
+        while (r && !r->hasMsg()) {
             r = r->m_chainedRuleParent;
         }
 
-        if (r && r->m_msg) {
+        if (r && r->hasMsg()) {
             std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
-            std::string *a = new std::string(r->m_msg->data(t));
+            std::string *a = new std::string(r->msg(t));
             VariableValue *var = new VariableValue(&m_rule, &m_rule_msg,
                 a
             );

test/optimization/optimization.cc

@@ -86,8 +86,8 @@ int main(int argc, char **argv) {
             if (z == NULL) {
                 continue;
             }
-            if (z->m_op != NULL) {
+            if (z->isUnconditional() == false) {
-                std::string op = z->m_op->m_op;
+                std::string op = z->getOperatorName();
                 if (operators.count(op) > 0) {
                     operators[op] = 1 + operators[op];
                 } else {
@@ -95,6 +95,10 @@ int main(int argc, char **argv) {
                 }
                 key = op;
             }
+
+            #if 0
+            FIXME: This test may not be useful anymore. Disabling it for now.
+
             if (z->m_variables != NULL) {
                 std::string var = std::string("") + z->m_variables;
                 if (variables.count(var) > 0) {
@@ -111,6 +115,7 @@ int main(int argc, char **argv) {
                     op2var[key] = 1;
                 }
             }
+            #endif
         }
 
         if (operators.empty() && variables.empty() && op2var.empty()) {