Refactoring in Rule: Meaningful structures name

Author: zimmerle

headers/modsecurity/rule.h

@@ -44,11 +44,18 @@ class Msg;
 class Rev;
 class SetVar;
 class Tag;
+namespace transformations {
+class Transformation;
+}
 }
 namespace operators {
 class Operator;
 }
 
+using TransformationResult = std::pair<std::shared_ptr<std::string>,
+    std::shared_ptr<std::string>>;
+using TransformationResults = std::list<TransformationResult>;
+
 class Rule {
  public:
     Rule(operators::Operator *_op,
@@ -68,13 +75,6 @@ class Rule {
     bool containsBlock, std::shared_ptr<RuleMessage> ruleMessage,
         actions::Action *a, bool context);
 
-    inline void executeTransformation(actions::Action *a,
-        std::shared_ptr<std::string> *value,
-        Transaction *trans,
-        std::list<std::pair<std::shared_ptr<std::string>,
-        std::shared_ptr<std::string>>> *ret,
-        std::string *path,
-        int *nth) const;
 
     void getVariablesExceptions(Transaction *t,
         variables::Variables *exclusion, variables::Variables *addition);
@@ -83,10 +83,6 @@ class Rule {
     void executeActionsAfterFullMatch(Transaction *trasn,
         bool containsDisruptive, std::shared_ptr<RuleMessage> ruleMessage);
 
-    std::list<std::pair<std::shared_ptr<std::string>,
-        std::shared_ptr<std::string>>> executeDefaultTransformations(
-        Transaction *trasn, const std::string &value);
-
     bool executeOperatorAt(Transaction *trasn, const std::string &key,
         std::string value, std::shared_ptr<RuleMessage> rm);
     void executeActionsIndependentOfChainedRuleResult(Transaction *trasn,
@@ -100,15 +96,17 @@ class Rule {
     bool containsTag(const std::string& name, Transaction *t);
     bool containsMsg(const std::string& name, Transaction *t);
 
+
     void executeTransformations(
-        actions::Action *a,
-        std::shared_ptr<std::string> newValue,
-        std::shared_ptr<std::string> value,
+        Transaction *trasn, const std::string &value, TransformationResults &ret);
+    inline void executeTransformation(
+        actions::transformations::Transformation *a,
+        std::shared_ptr<std::string> *value,
         Transaction *trans,
-        std::list<std::pair<std::shared_ptr<std::string>,
-        std::shared_ptr<std::string>>> *ret,
-        std::shared_ptr<std::string> transStr,
-        int nth);
+        TransformationResults *ret,
+        std::string *path,
+        int *nth) const;
+
 
     actions::Action *m_theDisruptiveAction;
     actions::LogData *m_logData;

src/parser/seclang-parser.cc

@@ -2425,7 +2425,7 @@ namespace yy {
                 }
                 checkedActions.push_back(a);
             } else {
-                driver.error(yystack_[2].location, "The action '" + a->m_name + "' is not suitable to be part of the SecDefaultActions");
+                driver.error(yystack_[2].location, "The action '" + *a->m_name.get() + "' is not suitable to be part of the SecDefaultActions");
                 YYERROR;
             }
         }

src/rule.cc

@@ -49,7 +49,7 @@ using operators::Operator;
 using actions::Action;
 using variables::Variable;
 using actions::transformations::None;
-
+using actions::transformations::Transformation;
 
 Rule::Rule(const std::string &marker)
     : m_theDisruptiveAction(nullptr),
@@ -326,11 +326,11 @@ bool Rule::executeOperatorAt(Transaction *trans, const std::string &key,
 }
 
 
-inline void Rule::executeTransformation(actions::Action *a,
+inline void Rule::executeTransformation(
+    actions::transformations::Transformation *a,
     std::shared_ptr<std::string> *value,
     Transaction *trans,
-    std::list<std::pair<std::shared_ptr<std::string>,
-        std::shared_ptr<std::string>>> *ret,
+    TransformationResults *ret,
     std::string *path,
     int *nth) const {
 
@@ -359,15 +359,11 @@ inline void Rule::executeTransformation(actions::Action *a,
 }
 
 
-std::list<std::pair<std::shared_ptr<std::string>,
-    std::shared_ptr<std::string>>>
-    Rule::executeDefaultTransformations(
-    Transaction *trans, const std::string &in) {
+void Rule::executeTransformations(
+    Transaction *trans, const std::string &in, TransformationResults &ret) {
     int none = 0;
     int transformations = 0;
     std::string path("");
-    std::list<std::pair<std::shared_ptr<std::string>,
-        std::shared_ptr<std::string>>> ret;
     std::shared_ptr<std::string> value =
         std::shared_ptr<std::string>(new std::string(in));
 
@@ -394,14 +390,17 @@ std::list<std::pair<std::shared_ptr<std::string>,
                 continue;
             }
 
-            executeTransformation(a.get(), &value, trans, &ret, &path,
+            // FIXME: here the object needs to be a transformation already.
+            Transformation *t = dynamic_cast<Transformation *>(a.get());
+            executeTransformation(t, &value, trans, &ret, &path,
                 &transformations);
         }
     }
 
     for (Action *a : this->m_actionsRuntimePre) {
         if (none == 0) {
-            executeTransformation(a, &value, trans, &ret, &path,
+            Transformation *t = dynamic_cast<Transformation *>(a);
+            executeTransformation(t, &value, trans, &ret, &path,
                 &transformations);
         }
         if (a->m_isNone) {
@@ -427,7 +426,8 @@ std::list<std::pair<std::shared_ptr<std::string>,
         }
         actions::Action *a = dynamic_cast<actions::Action*>(b.second.get());
         if (none == 0) {
-            executeTransformation(a, &value, trans, &ret, &path,
+            Transformation *t = dynamic_cast<Transformation *>(a);
+            executeTransformation(t, &value, trans, &ret, &path,
                 &transformations);
         }
         if (a->m_isNone) {
@@ -446,8 +446,6 @@ std::list<std::pair<std::shared_ptr<std::string>,
             std::shared_ptr<std::string>(new std::string(*value)),
             std::shared_ptr<std::string>(new std::string(path))));
     }
-
-    return ret;
 }
 
 
@@ -711,10 +709,9 @@ bool Rule::evaluate(Transaction *trans,
                 continue;
             }
 
-            std::list<std::pair<std::shared_ptr<std::string>,
-                std::shared_ptr<std::string>>> values;
+            TransformationResults values;
 
-            values = executeDefaultTransformations(trans, value);
+            executeTransformations(trans, value, values);
 
             for (const auto &valueTemp : values) {
                 bool ret;