Cleanup on Action class

headers/modsecurity/actions/action.h

@@ -39,35 +39,27 @@ namespace actions {
 
 class Action {
  public:
-
     explicit Action(const std::string& _action)
-        : m_isNone(false),
-        temporaryAction(false),
-        action_kind(2),
+        : m_actionKind(2),
         m_name(nullptr),
         m_parser_payload("") {
             set_name_and_payload(_action);
         }
+
     Action(const std::string& _action, int kind)
-        : m_isNone(false),
-        temporaryAction(false),
-        action_kind(kind),
+        : m_actionKind(kind),
         m_name(nullptr),
         m_parser_payload("") {
             set_name_and_payload(_action);
         }
 
     Action(const Action &a)
-        : m_isNone(a.m_isNone),
-        temporaryAction(a.temporaryAction),
-        action_kind(a.action_kind),
+        : m_actionKind(a.m_actionKind),
         m_name(a.m_name),
         m_parser_payload(a.m_parser_payload) { }
 
     Action &operator=(const Action& a) {
-        m_isNone = a.m_isNone;
-        temporaryAction = a.temporaryAction;
-        action_kind = a.action_kind;
+        m_actionKind = a.m_actionKind;
         m_name = a.m_name;
         m_parser_payload = a.m_parser_payload;
         return *this;
@@ -75,52 +67,24 @@ class Action {
 
     virtual ~Action() { }
 
+    virtual bool init(std::string *error) { return true; }
+
     virtual std::string execute(const std::string &exp,
         Transaction *transaction);
-    virtual bool execute(RuleWithActions *rule, Transaction *transaction);
-
+    virtual bool execute(RuleWithActions *rule,
+        Transaction *transaction);
     /**
      * This method is meant to be used by transformations — a particular
      * type of action.
      *
      */
     virtual void execute(Transaction *t,
-        ModSecStackString &in,
-        ModSecStackString &out) {
+        ModSecString &in,
+        ModSecString &out) {
     };
 
-    virtual bool init(std::string *error) { return true; }
     virtual bool isDisruptive() { return false; }
 
-
-    void set_name_and_payload(const std::string& data) {
-        size_t pos = data.find(":");
-        std::string t = "t:";
-
-        if (data.compare(0, t.length(), t) == 0) {
-            pos = data.find(":", 2);
-        }
-
-        if (pos == std::string::npos) {
-            m_name = std::shared_ptr<std::string>(new std::string(data));
-            return;
-        }
-
-        m_name = std::shared_ptr<std::string>(new std::string(data, 0, pos));
-        m_parser_payload = std::string(data, pos + 1, data.length());
-
-        if (m_parser_payload.at(0) == '\'' && m_parser_payload.size() > 2) {
-            m_parser_payload.erase(0, 1);
-            m_parser_payload.pop_back();
-        }
-    }
-
-    bool m_isNone;
-    bool temporaryAction;
-    int action_kind;
-    std::shared_ptr<std::string> m_name;
-    std::string m_parser_payload;
-
     /**
      *
      * Define the action kind regarding to the execution time.
@@ -152,7 +116,35 @@ class Action {
      */
      RunTimeOnlyIfMatchKind,
     };
- };
+
+    int m_actionKind;
+    std::shared_ptr<std::string> m_name;
+    std::string m_parser_payload;
+
+ private:
+
+    void set_name_and_payload(const std::string& data) {
+        size_t pos = data.find(":");
+        std::string t = "t:";
+
+        if (data.compare(0, t.length(), t) == 0) {
+            pos = data.find(":", 2);
+        }
+
+        if (pos == std::string::npos) {
+            m_name = std::shared_ptr<std::string>(new std::string(data));
+            return;
+        }
+
+        m_name = std::shared_ptr<std::string>(new std::string(data, 0, pos));
+        m_parser_payload = std::string(data, pos + 1, data.length());
+
+        if (m_parser_payload.at(0) == '\'' && m_parser_payload.size() > 2) {
+            m_parser_payload.erase(0, 1);
+            m_parser_payload.pop_back();
+        }
+    }
+};
 
 
 }  // namespace actions

headers/modsecurity/modsecurity.h

@@ -89,6 +89,12 @@
 typedef struct ModSecurity_t modsecurity;
 #else
 namespace modsecurity {
+    /**
+     *  Further that will be changed to be a stack-based string,
+     * for the benefit of performance.
+     */
+    using ModSecString = std::string;
+
     /**
      *
      * The Phases enumerator consists in mapping the different stages of a

headers/modsecurity/rule_with_actions.h

@@ -64,18 +64,17 @@ using MatchActionsPtr = std::vector<actions::Action *>;
 using XmlNSs = std::vector<std::shared_ptr<actions::XmlNS> >;
 using XmlNSsPtr = std::vector<actions::XmlNS *>;
 
-using ModSecStackString = std::basic_string<char, std::char_traits<char>, std::allocator<char> >;
 
 class TransformationResult {
  public:
     TransformationResult(
-        ModSecStackString *after,
+        ModSecString *after,
         std::string *transformation)
         : m_after(*after),
         m_transformation(transformation) { };
 
     explicit TransformationResult(
-        ModSecStackString *after)
+        ModSecString *after)
         : m_after(*after),
         m_transformation(nullptr) { };
 
@@ -84,7 +83,7 @@ class TransformationResult {
         m_transformation(t2.m_transformation) { };
 
 
-    ModSecStackString *getAfter() {
+    ModSecString *getAfter() {
         return &m_after;
     }
 
@@ -95,7 +94,7 @@ class TransformationResult {
 
 
  private:
-    ModSecStackString m_after;
+    ModSecString m_after;
     std::string *m_transformation;
 };
 
@@ -224,7 +223,7 @@ class RuleWithActions : public Rule {
 
     static void executeTransformation(
         Transaction *transaction,
-        ModSecStackString in,
+        ModSecString in,
         TransformationsResults *ret,
         Transformation *transformation);
 

headers/modsecurity/rules.h

@@ -40,52 +40,20 @@ namespace modsecurity {
 
 class Rules {
  public:
-    void dump() const {
-        for (int j = 0; j < m_rules.size(); j++) {
-            std::cout << "    Rule ID: " << m_rules.at(j)->getReference();
-            std::cout << "--" << m_rules.at(j) << std::endl;
-        }
-    }
+    void dump() const;
 
-    int append(Rules *from, const std::vector<int64_t> &ids, std::ostringstream *err) {
-         size_t j = 0;
-         for (; j < from->size(); j++) {
-            RuleWithOperator *rule = dynamic_cast<RuleWithOperator *>(from->at(j).get());
-            if (rule && std::binary_search(ids.begin(), ids.end(), rule->getId())) {
-                 if (err != NULL) {
-                     *err << "Rule id: " << std::to_string(rule->getId()) \
-                        << " is duplicated" << std::endl;
-                 }
-                 return -1;
-             }
-         }
-         m_rules.insert(m_rules.end(), from->m_rules.begin(), from->m_rules.end());
-         return j;
-    }
+    int append(Rules *from, const std::vector<int64_t> &ids, std::ostringstream *err);
 
-    bool insert(const std::shared_ptr<Rule> &rule) {
-        return insert(rule, nullptr, nullptr);
-    }
+    bool insert(const std::shared_ptr<Rule> &rule);
 
-    bool insert(std::shared_ptr<Rule> rule, const std::vector<int64_t> *ids, std::ostringstream *err) {
-        RuleWithOperator *r = dynamic_cast<RuleWithOperator *>(rule.get());
-        if (r && ids != nullptr && std::binary_search(ids->begin(), ids->end(), r->getId())) {
-            if (err != nullptr) {
-                *err << "Rule id: " << std::to_string(r->getId()) \
-                    << " is duplicated" << std::endl;
-            }
-            return false;
-        }
-        m_rules.push_back(rule);
-        return true;
-    }
+    bool insert(std::shared_ptr<Rule> rule, const std::vector<int64_t> *ids, std::ostringstream *err);
+
+    size_t size() const;
+    std::shared_ptr<Rule> operator[](int index) const;
+    std::shared_ptr<Rule> at(int index) const;
 
     void fixDefaultActions();
 
-    size_t size() const { return m_rules.size(); }
-    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<actions::transformations::Transformation> > m_defaultTransformations;
 

headers/modsecurity/transaction.h

@@ -49,7 +49,6 @@ typedef struct Rules_t RulesSet;
 #include "modsecurity/collection/collection.h"
 #include "modsecurity/variable_origin.h"
 
-
 #ifndef NO_LOGS
 #define ms_dbg(b, c) \
   do { \