Creates the RulesSetPhases clas

Makefile.am

@@ -125,6 +125,7 @@ TESTS+=test/test-cases/regression/config-body_limits.json
 TESTS+=test/test-cases/regression/config-calling_phases_by_name.json
 TESTS+=test/test-cases/regression/config-include-bad.json
 TESTS+=test/test-cases/regression/config-include.json
+TESTS+=test/test-cases/regression/config-phases.json
 TESTS+=test/test-cases/regression/config-remove_by_id.json
 TESTS+=test/test-cases/regression/config-remove_by_msg.json
 TESTS+=test/test-cases/regression/config-remove_by_tag.json

headers/modsecurity/rules_set.h

@@ -41,6 +41,87 @@ class Driver;
 }
 
 
+class RulesSetPhases {
+ public:
+
+    ~RulesSetPhases() {
+        /** Cleanup the rules */
+        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
+            std::vector<Rule *> rules = m_rules[i];
+            while (rules.empty() == false) {
+                Rule *rule = rules.back();
+                rules.pop_back();
+                if (rule->refCountDecreaseAndCheck()) {
+                    rule = NULL;
+                }
+            }
+        }
+    }
+
+    bool insert(Rule *rule) {
+        if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
+            return false;
+        }
+        m_rules[rule->m_phase].push_back(rule);
+
+        return true;
+    }
+
+    int append(RulesSetPhases *from, std::ostringstream *err) {
+        int amount_of_rules = 0;
+        std::vector<int64_t> v;
+
+        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
+            v.reserve(m_rules[i].size());
+            for (size_t z = 0; z < m_rules[i].size(); z++) {
+                Rule *rule_ckc = m_rules[i].at(z);
+                if (rule_ckc->m_secMarker == true) {
+                    continue;
+                }
+                v.push_back(rule_ckc->m_ruleId);
+            }
+        }
+        std::sort (v.begin(), v.end());
+
+        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
+            for (size_t j = 0; j < from->at(i).size(); j++) {
+                Rule *rule = from->at(i).at(j);
+                if (std::binary_search(v.begin(), v.end(), rule->m_ruleId)) {
+                    if (err != NULL) {
+                        *err << "Rule id: " << std::to_string(rule->m_ruleId) \
+                            << " is duplicated" << std::endl;
+                    }
+                    return -1;
+                }
+                amount_of_rules++;
+                rule->refCountIncrease();
+                m_rules[i].push_back(rule);
+            }
+        }
+
+        return amount_of_rules;
+    }
+
+    void dump() {
+        for (int i = 0; i <= modsecurity::Phases::NUMBER_OF_PHASES; i++) {
+            std::vector<Rule *> rules = m_rules[i];
+            std::cout << "Phase: " << std::to_string(i);
+            std::cout << " (" << std::to_string(rules.size());
+            std::cout << " rules)" << std::endl;
+            for (int j = 0; j < rules.size(); j++) {
+                std::cout << "    Rule ID: " << std::to_string(rules[j]->m_ruleId);
+                std::cout << "--" << rules[j] << std::endl;
+            }
+        }
+    }
+
+    std::vector<modsecurity::Rule *> operator[](int index) const { return m_rules[index]; }
+    std::vector<modsecurity::Rule *> at(int index) const { return m_rules[index]; }
+
+    std::vector<modsecurity::Rule *> m_rules[8];
+};
+
+
 /** @ingroup ModSecurity_CPP_API */
 class RulesSet : public RulesSetProperties {
  public:
@@ -80,6 +161,7 @@ class RulesSet : public RulesSetProperties {
 
     int64_t unicode_codepage;
 
+    RulesSetPhases m_rulesSetPhases;
  private:
 #ifndef NO_LOGS
     uint8_t m_secmarker_skipped;

headers/modsecurity/rules_set_properties.h

@@ -202,17 +202,7 @@ class RulesSetProperties {
 
     ~RulesSetProperties() {
         int i = 0;
-        /** Cleanup the rules */
+
-        for (i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-            std::vector<Rule *> rules = m_rules[i];
-            while (rules.empty() == false) {
-                Rule *rule = rules.back();
-                rules.pop_back();
-                if (rule->refCountDecreaseAndCheck()) {
-                    rule = NULL;
-                }
-            }
-        }
         for (i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
             std::vector<actions::Action *> *tmp = &m_defaultActions[i];
             while (tmp->empty() == false) {
@@ -354,12 +344,6 @@ class RulesSetProperties {
 
     static int mergeProperties(RulesSetProperties *from, RulesSetProperties *to,
         std::ostringstream *err) {
-        int amount_of_rules = 0;
-
-        amount_of_rules = appendRules(from->m_rules, to->m_rules, err);
-        if (amount_of_rules < 0) {
-            return amount_of_rules;
-        }
 
         merge_ruleengine_value(to->m_secRuleEngine, from->m_secRuleEngine,
                                PropertyNotSetRuleEngine);
@@ -471,56 +455,7 @@ class RulesSetProperties {
             }
         }
 
-        return amount_of_rules;
+        return 1;
-    }
-
-
-    static int appendRules(
-        std::vector<modsecurity::Rule *> *from,
-        std::vector<modsecurity::Rule *> *to,
-        std::ostringstream *err) {
-        int amount_of_rules = 0;
-        // TODO: std::vector could be replaced with something more efficient.
-        std::vector<int64_t> v;
-        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-            std::vector<modsecurity::Rule *> *rules_to = to+i;
-            v.reserve(rules_to->size());
-            for (size_t z = 0; z < rules_to->size(); z++) {
-                Rule *rule_ckc = rules_to->at(z);
-                if (rule_ckc->m_secMarker == true) {
-                    continue;
-                }
-                v.push_back(rule_ckc->m_ruleId);
-            }
-        }
-        std::sort (v.begin(), v.end());
-
-        for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-            std::vector<modsecurity::Rule *> *rules_from = from+i;
-            std::vector<modsecurity::Rule *> *rules_to = to+i;
-            for (size_t j = 0; j < rules_from->size(); j++) {
-                Rule *rule = rules_from->at(j);
-                if (std::binary_search(v.begin(), v.end(), rule->m_ruleId)) {
-                    if (err != NULL) {
-                        *err << "Rule id: " << std::to_string(rule->m_ruleId) \
-                            << " is duplicated" << std::endl;
-                    }
-                    return -1;
-                }
-                amount_of_rules++;
-                rule->refCountIncrease();
-                rules_to->push_back(rule);
-            }
-        }
-        return amount_of_rules;
-    }
-
-
-    std::vector<modsecurity::Rule *> *getRulesForPhase(int phase) {
-        if (phase >= modsecurity::Phases::NUMBER_OF_PHASES) {
-            return NULL;
-        }
-        return &m_rules[phase];
     }
 
 
@@ -551,7 +486,6 @@ class RulesSetProperties {
     ConfigString m_secArgumentSeparator;
     ConfigString m_secWebAppId;
     std::vector<actions::Action *> m_defaultActions[modsecurity::Phases::NUMBER_OF_PHASES];
-    std::vector<modsecurity::Rule *> m_rules[modsecurity::Phases::NUMBER_OF_PHASES];
     ConfigUnicodeMap m_unicodeMapTable;
 };
 

src/parser/driver.cc

@@ -45,7 +45,7 @@ int Driver::addSecMarker(std::string marker) {
     for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
         Rule *rule = new Rule(marker);
         rule->m_phase = i;
-        m_rules[i].push_back(rule);
+        m_rulesSetPhases.insert(rule);
     }
     return 0;
 }
@@ -58,14 +58,15 @@ int Driver::addSecAction(Rule *rule) {
         return false;
     }
 
-    m_rules[rule->m_phase].push_back(rule);
+
+    m_rulesSetPhases.insert(rule);
 
     return true;
 }
 
 
 int Driver::addSecRuleScript(RuleScript *rule) {
-    m_rules[rule->m_phase].push_back(rule);
+    m_rulesSetPhases.insert(rule);
     return true;
 }
 
@@ -118,7 +119,7 @@ int Driver::addSecRule(Rule *rule) {
         return false;
     }
     for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-        std::vector<Rule *> rules = m_rules[i];
+        std::vector<Rule *> rules = m_rulesSetPhases[i];
         for (int j = 0; j < rules.size(); j++) {
             if (rules[j]->m_ruleId == rule->m_ruleId) {
                 m_parserError << "Rule id: " << std::to_string(rule->m_ruleId) \
@@ -129,7 +130,7 @@ int Driver::addSecRule(Rule *rule) {
     }
 
     lastRule = rule;
-    m_rules[rule->m_phase].push_back(rule);
+    m_rulesSetPhases.insert(rule);
     return true;
 }
 

src/parser/driver.h

@@ -90,6 +90,8 @@ class Driver : public RulesSetProperties {
 
     std::string buffer;
     Rule *lastRule;
+
+    RulesSetPhases m_rulesSetPhases;
 };
 
 

src/rules_set.cc

@@ -110,7 +110,7 @@ int RulesSet::evaluate(int phase, Transaction *t) {
        return 0;
     }
 
-    std::vector<Rule *> rules = m_rules[phase];
+    std::vector<Rule *> rules = m_rulesSetPhases[phase];
 
     ms_dbg_a(t, 9, "This phase consists of " \
         + std::to_string(rules.size()) + " rule(s).");
@@ -222,7 +222,10 @@ int RulesSet::evaluate(int phase, Transaction *t) {
 
 int RulesSet::merge(Driver *from) {
     int amount_of_rules = 0;
-    amount_of_rules = mergeProperties(
+
+    amount_of_rules = m_rulesSetPhases.append(&from->m_rulesSetPhases,
+        &m_parserError);
+    mergeProperties(
         dynamic_cast<RulesSetProperties *>(from),
         dynamic_cast<RulesSetProperties *>(this),
         &m_parserError);
@@ -233,7 +236,10 @@ int RulesSet::merge(Driver *from) {
 
 int RulesSet::merge(RulesSet *from) {
     int amount_of_rules = 0;
-    amount_of_rules = mergeProperties(
+
+    amount_of_rules = m_rulesSetPhases.append(&from->m_rulesSetPhases,
+        &m_parserError);
+    mergeProperties(
         dynamic_cast<RulesSetProperties *>(from),
         dynamic_cast<RulesSetProperties *>(this),
         &m_parserError);
@@ -254,7 +260,7 @@ void RulesSet::debug(int level, const std::string &id,
 void RulesSet::dump() const {
     std::cout << "Rules: " << std::endl;
     for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) {
-        std::vector<Rule *> rules = m_rules[i];
+        std::vector<Rule *> rules = m_rulesSetPhases[i];
         std::cout << "Phase: " << std::to_string(i);
         std::cout << " (" << std::to_string(rules.size());
         std::cout << " rules)" << std::endl;

test/optimization/optimization.cc

@@ -67,8 +67,8 @@ int main(int argc, char **argv) {
     std::cout << std::endl;
 
     int nphases = modsecurity::Phases::NUMBER_OF_PHASES;
-    for (int j = 0; j < nphases; j++) {
+    for (int i = 0; i < nphases; i++) {
-        std::vector<Rule *> rules = modsecRules->m_rules[i];
+        std::vector<Rule *> rules = modsecRules->m_rulesSetPhases[i];
         if (rules.size() == 0) {
             continue;
         }

test/test-cases/regression/config-phases.json

@@ -0,0 +1,199 @@
+[
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Config :: Phases (1/n)",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*"
+      },
+      "uri":"/?key=value&key=other_value",
+      "method":"GET"
+    },
+    "response":{
+      "headers":{
+        "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+        "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+        "Content-Type":"text/html"
+      },
+      "body":[
+        "no need."
+      ]
+    },
+    "expected":{
+      "debug_log":"SecRules 0.*\n.*This phase consists of 1 rule\\(s\\)"
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:0,block,status:404\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Config :: Phases (2/n)",
+    "client":{
+      "ip":"200.249.12.31",
+      "port":123
+    },
+    "server":{
+      "ip":"200.249.12.31",
+      "port":80
+    },
+    "request":{
+      "headers":{
+        "Host":"localhost",
+        "User-Agent":"curl/7.38.0",
+        "Accept":"*/*"
+      },
+      "uri":"/?key=value&key=other_value",
+      "method":"GET"
+    },
+    "response":{
+      "headers":{
+        "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+        "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+        "Content-Type":"text/html"
+      },
+      "body":[
+        "no need."
+      ]
+    },
+    "expected":{
+      "debug_log":"SecRules 1.*\n.*This phase consists of 1 rule\\(s\\)"
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:1,block,status:404\""
+    ]
+},
+{
+  "enabled":1,
+  "version_min":300000,
+  "title":"Testing Config :: Phases (3/n)",
+  "client":{
+    "ip":"200.249.12.31",
+    "port":123
+  },
+  "server":{
+    "ip":"200.249.12.31",
+    "port":80
+  },
+  "request":{
+    "headers":{
+      "Host":"localhost",
+      "User-Agent":"curl/7.38.0",
+      "Accept":"*/*"
+    },
+    "uri":"/?key=value&key=other_value",
+    "method":"GET"
+  },
+  "response":{
+    "headers":{
+      "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+      "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+      "Content-Type":"text/html"
+    },
+    "body":[
+      "no need."
+    ]
+  },
+  "expected":{
+    "debug_log":"SecRules 3.*\n.*This phase consists of 1 rule\\(s\\)"
+  },
+  "rules":[
+    "SecRuleEngine On",
+    "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:3,block,status:404\""
+  ]
+},
+{
+  "enabled":1,
+  "version_min":300000,
+  "title":"Testing Config :: Phases (4/n)",
+  "client":{
+    "ip":"200.249.12.31",
+    "port":123
+  },
+  "server":{
+    "ip":"200.249.12.31",
+    "port":80
+  },
+  "request":{
+    "headers":{
+      "Host":"localhost",
+      "User-Agent":"curl/7.38.0",
+      "Accept":"*/*"
+    },
+    "uri":"/?key=value&key=other_value",
+    "method":"GET"
+  },
+  "response":{
+    "headers":{
+      "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+      "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+      "Content-Type":"text/html"
+    },
+    "body":[
+      "no need."
+    ]
+  },
+  "expected":{
+    "debug_log":"SecRules 4.*\n.*This phase consists of 1 rule\\(s\\)"
+  },
+  "rules":[
+    "SecRuleEngine On",
+    "SecResponseBodyAccess On",
+    "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:4,block,status:404\""
+  ]
+},
+{
+  "enabled":1,
+  "version_min":300000,
+  "title":"Testing Config :: Phases (5/n)",
+  "client":{
+    "ip":"200.249.12.31",
+    "port":123
+  },
+  "server":{
+    "ip":"200.249.12.31",
+    "port":80
+  },
+  "request":{
+    "headers":{
+      "Host":"localhost",
+      "User-Agent":"curl/7.38.0",
+      "Accept":"*/*"
+    },
+    "uri":"/?key=value&key=other_value",
+    "method":"GET"
+  },
+  "response":{
+    "headers":{
+      "Date":"Mon, 13 Jul 2015 20:02:41 GMT",
+      "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
+      "Content-Type":"text/html"
+    },
+    "body":[
+      "no need."
+    ]
+  },
+  "expected":{
+    "debug_log":"SecRules 5.*\n.*This phase consists of 1 rule\\(s\\)"
+  },
+  "rules":[
+    "SecRuleEngine On",
+    "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:5,block,status:404\""
+  ]
+}
+]
+