Uses shared_ptr on variable names

Felipe Zimmerle · Felipe Zimmerle · commit 003a8e8e5f33 · 2017-08-27T22:06:20.000-03:00
diff --git a/headers/modsecurity/collection/variable.h b/headers/modsecurity/collection/variable.h
@@ -37,33 +37,32 @@ namespace collection {
 class Variable {
  public:
     explicit Variable(const std::string *key) :
-        m_key(key),
-        m_value(),
-        m_dynamic_value(false),
-        m_dynamic_key(false),
-        m_dynamic(true) { }
+        m_dynamic(true) {
+            m_key.reset(new std::string(*key));
+        }
     Variable(const std::string *key, const std::string *value) :
-        m_key(key),
-        m_value(value),
-        m_dynamic_value(false),
-        m_dynamic_key(false),
-        m_dynamic(true) { }
+        m_dynamic(true) {
+            m_key.reset(new std::string(*key));
+            m_value.reset(new std::string(*value));
 
-    ~Variable() {
-        if (m_dynamic_value) {
-            delete m_value;
         }
-        if (m_dynamic_key) {
-            delete m_key;
+    explicit Variable(const std::shared_ptr<std::string> key) :
+        m_dynamic(true) {
+            m_key = key;
+        }
+    Variable(std::shared_ptr<std::string> key, std::shared_ptr<std::string> value) :
+        m_dynamic(true) {
+            m_key = key;
+            m_value = value;
+
         }
+    ~Variable() {
     }
 
-    const std::string *m_key;
-    const std::string *m_value;
-    bool m_dynamic_value;
-    bool m_dynamic_key;
-    bool m_dynamic;
+    std::shared_ptr<std::string> m_key;
+    std::shared_ptr<std::string> m_value;
     std::list<std::unique_ptr<VariableOrigin>> m_orign;
+    bool m_dynamic;
 };
 
 }  // namespace collection
diff --git a/headers/modsecurity/rules_exceptions.h b/headers/modsecurity/rules_exceptions.h
@@ -60,7 +60,7 @@ class RulesExceptions {
         std::unique_ptr<std::vector<std::unique_ptr<Variables::Variable> > > var,
         std::string *error);
 
-    std::unordered_multimap<std::string, std::unique_ptr<Variables::Variable>> m_variable_update_target_by_tag;
+    std::unordered_multimap<std::shared_ptr<std::string>, std::unique_ptr<Variables::Variable>> m_variable_update_target_by_tag;
     std::unordered_multimap<double, std::unique_ptr<Variables::Variable>> m_variable_update_target_by_id;
     std::list<std::string> m_remove_rule_by_msg;
 
diff --git a/src/anchored_set_variable.cc b/src/anchored_set_variable.cc
@@ -43,8 +43,6 @@ AnchoredSetVariable::~AnchoredSetVariable() {
 void AnchoredSetVariable::unset() {
     for (const auto& x : *this) {
         collection::Variable *var = x.second;
-        delete var->m_key;
-        var->m_key = NULL;
         delete var;
     }
     clear();
@@ -61,7 +59,6 @@ void AnchoredSetVariable::set(const std::string &key,
     origin->m_offset = offset;
     origin->m_length = len;
 
-    var->m_dynamic_value = true;
     var->m_dynamic = false;
     var->m_orign.push_back(std::move(origin));
     emplace(key, var);
@@ -78,7 +75,6 @@ void AnchoredSetVariable::set(const std::string &key,
     origin->m_offset = offset;
     origin->m_length = value.size();
 
-    var->m_dynamic_value = true;
     var->m_dynamic = false;
     var->m_orign.push_back(std::move(origin));
     emplace(key, var);
diff --git a/src/anchored_variable.cc b/src/anchored_variable.cc
@@ -38,15 +38,17 @@ AnchoredVariable::AnchoredVariable(Transaction *t,
         m_name.append(name);
         m_var = new collection::Variable(&m_name);
         m_var->m_dynamic = false;
-        m_var->m_value = &m_value;
+        m_var->m_value.reset(&m_value);
 }
 
 
 AnchoredVariable::~AnchoredVariable() {
+    /*
     if (m_var) {
         delete (m_var);
         m_var = NULL;
     }
+    */
 }
 
 
diff --git a/src/rule.cc b/src/rule.cc
@@ -431,16 +431,16 @@ std::list<std::pair<std::shared_ptr<std::string>,
 
 std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
     Transaction *trans) {
-    std::list<const std::string*> exclusions;
-    std::list<const std::string*> exclusions_update_by_tag_remove;
-    std::list<const std::string*> exclusions_update_by_id_remove;
+    std::list<std::shared_ptr<std::string>> exclusions;
+    std::list<std::shared_ptr<std::string>> exclusions_update_by_tag_remove;
+    std::list<std::shared_ptr<std::string>> exclusions_update_by_id_remove;
     std::vector<Variables::Variable *> variables;
     std::vector<std::unique_ptr<collection::Variable>> finalVars;
 
     std::copy (m_variables->begin(), m_variables->end(), std::back_inserter(variables));
 
     for (auto &a : trans->m_rules->m_exceptions.m_variable_update_target_by_tag) {
-        if (containsTag(a.first, trans) == false) {
+        if (containsTag(*a.first.get(), trans) == false) {
             continue;
         }
         if (a.second->m_isExclusion) {
@@ -449,7 +449,7 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
             for (auto &y : z) {
                 exclusions_update_by_tag_remove.push_back(y->m_key);
             }
-            exclusions_update_by_tag_remove.push_back(&a.second->m_name);
+            exclusions_update_by_tag_remove.push_back(std::make_shared<std::string>(a.second->m_name));
 
         } else {
             Variable *b = a.second.get();
@@ -467,7 +467,7 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
             for (auto &y : z) {
                 exclusions_update_by_id_remove.push_back(y->m_key);
             }
-            exclusions_update_by_id_remove.push_back(&a.second->m_name);
+            exclusions_update_by_id_remove.push_back(std::make_shared<std::string>(a.second->m_name));
         } else {
             Variable *b = a.second.get();
             variables.push_back(b);
@@ -482,7 +482,7 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
             for (auto &y : z) {
                 exclusions.push_back(y->m_key);
             }
-            exclusions.push_back(&variable->m_name);
+//            exclusions.push_back(std::make_shared<std::string>(&variable->m_name));
         }
     }
 
@@ -497,9 +497,9 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
 
         variable->evaluateInternal(trans, this, &e);
         for (const collection::Variable *v : e) {
-            const std::string *key = v->m_key;
+            const std::shared_ptr<std::string> key = v->m_key;
             if (std::find_if(exclusions.begin(), exclusions.end(),
-                [key](const std::string *m) -> bool { return *key == *m; })
+                [key](std::shared_ptr<std::string> m) -> bool { return *key == *m.get(); })
                 != exclusions.end()) {
 #ifndef NO_LOGS
                 trans->debug(9, "Variable: " + *key +
@@ -513,7 +513,7 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
             }
             if (std::find_if(exclusions_update_by_tag_remove.begin(),
                 exclusions_update_by_tag_remove.end(),
-                [key](const std::string *m) -> bool { return *key == *m; })
+                [key](std::shared_ptr<std::string> m) -> bool { return *key == *m.get(); })
                 != exclusions_update_by_tag_remove.end()) {
 #ifndef NO_LOGS
                 trans->debug(9, "Variable: " + *key +
@@ -529,7 +529,7 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
 
             if (std::find_if(exclusions_update_by_id_remove.begin(),
                 exclusions_update_by_id_remove.end(),
-                [key](const std::string *m) -> bool { return *key == *m; })
+                [key](std::shared_ptr<std::string> m) -> bool { return *key == *m.get(); })
                 != exclusions_update_by_id_remove.end()) {
 #ifndef NO_LOGS
                 trans->debug(9, "Variable: " + *key +
@@ -621,8 +621,6 @@ std::vector<std::unique_ptr<collection::Variable>> Rule::getFinalVars(
             std::unique_ptr<collection::Variable> var(new collection::Variable(
                 new std::string(*v->m_key),
                 new std::string(*v->m_value)));
-            var->m_dynamic_value = true;
-            var->m_dynamic_key = true;
             for (auto &i : v->m_orign) {
                 std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
                 origin->m_offset = i->m_offset;
diff --git a/src/rules_exceptions.cc b/src/rules_exceptions.cc
@@ -44,7 +44,7 @@ bool RulesExceptions::loadUpdateTargetByTag(const std::string &tag,
     std::string *error) {
 
     for (auto &i : *var) {
-        m_variable_update_target_by_tag.emplace(std::pair<std::string, std::unique_ptr<Variables::Variable>>(tag, std::move(i)));
+        m_variable_update_target_by_tag.emplace(std::pair<std::shared_ptr<std::string>, std::unique_ptr<Variables::Variable>>(std::make_shared<std::string>(tag), std::move(i)));
     }
 
     return true;
@@ -164,7 +164,7 @@ bool RulesExceptions::merge(RulesExceptions& from) {
     }
 
     for (auto &p : from.m_variable_update_target_by_tag) {
-        m_variable_update_target_by_tag.emplace(std::pair<std::string, std::unique_ptr<Variables::Variable>>(p.first, std::move(p.second)));
+        m_variable_update_target_by_tag.emplace(std::pair<std::shared_ptr<std::string>, std::unique_ptr<Variables::Variable>>(p.first, std::move(p.second)));
     }
 
     for (auto &p : from.m_variable_update_target_by_id) {
diff --git a/src/variables/remote_user.cc b/src/variables/remote_user.cc
@@ -64,7 +64,7 @@ void RemoteUser::evaluate(Transaction *transaction,
     transaction->m_variableRemoteUser.assign(std::string(base64, 0, pos));
 
     var = new collection::Variable(l->at(0)->m_key,
-        &transaction->m_variableRemoteUser);
+        std::make_shared<std::string>(transaction->m_variableRemoteUser));
 
     for (auto &i : l->at(0)->m_orign) {
         std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
diff --git a/src/variables/variable.h b/src/variables/variable.h
@@ -143,7 +143,6 @@ class VariableModificatorCount : public Variable {
         res = new std::string(std::to_string(count));
 
         val = new collection::Variable(&m_name, res);
-        val->m_dynamic_value = true;
         val->m_dynamic = true;
 
         l->push_back(val);
diff --git a/src/variables/xml.cc b/src/variables/xml.cc
@@ -127,7 +127,6 @@ void XML::evaluate(Transaction *t,
         if (content != NULL) {
             collection::Variable *var = new collection::Variable(&m_name,
                 new std::string(content));
-            var->m_dynamic_value = true;
             l->push_back(var);
             xmlFree(content);
          }
diff --git a/src/variables/xml.h b/src/variables/xml.h
@@ -41,7 +41,6 @@ class XML_NoDictElement : public Variable {
         m_plain("[XML document tree]"),
         m_var(&m_name, &m_plain) {
             m_var.m_dynamic = false;
-            m_var.m_dynamic_value = false;
         }
 
     void evaluate(Transaction *transaction,

Original file line number	Diff line number	Diff line change
`@@ -38,15 +38,17 @@ AnchoredVariable::AnchoredVariable(Transaction *t,`
`38`	`38`	`m_name.append(name);`
`39`	`39`	`m_var = new collection::Variable(&m_name);`
`40`	`40`	`m_var->m_dynamic = false;`
`41`		`- m_var->m_value = &m_value;`
	`41`	`+ m_var->m_value.reset(&m_value);`
`42`	`42`	`}`
`43`	`43`
`44`	`44`
`45`	`45`	`AnchoredVariable::~AnchoredVariable() {`
	`46`	`+ /*`
`46`	`47`	`if (m_var) {`
`47`	`48`	`delete (m_var);`
`48`	`49`	`m_var = NULL;`
`49`	`50`	`}`
	`51`	`+ */`
`50`	`52`	`}`
`51`	`53`
`52`	`54`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ bool RulesExceptions::loadUpdateTargetByTag(const std::string &tag,`
`44`	`44`	`std::string *error) {`
`45`	`45`
`46`	`46`	`for (auto &i : *var) {`
`47`		`- m_variable_update_target_by_tag.emplace(std::pair<std::string, std::unique_ptr<Variables::Variable>>(tag, std::move(i)));`
	`47`	`+ m_variable_update_target_by_tag.emplace(std::pair<std::shared_ptr<std::string>, std::unique_ptr<Variables::Variable>>(std::make_shared<std::string>(tag), std::move(i)));`
`48`	`48`	`}`
`49`	`49`
`50`	`50`	`return true;`
`@@ -164,7 +164,7 @@ bool RulesExceptions::merge(RulesExceptions& from) {`
`164`	`164`	`}`
`165`	`165`
`166`	`166`	`for (auto &p : from.m_variable_update_target_by_tag) {`
`167`		`- m_variable_update_target_by_tag.emplace(std::pair<std::string, std::unique_ptr<Variables::Variable>>(p.first, std::move(p.second)));`
	`167`	`+ m_variable_update_target_by_tag.emplace(std::pair<std::shared_ptr<std::string>, std::unique_ptr<Variables::Variable>>(p.first, std::move(p.second)));`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`for (auto &p : from.m_variable_update_target_by_id) {`
Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,6 @@ void XML::evaluate(Transaction *t,`
`127`	`127`	`if (content != NULL) {`
`128`	`128`	`collection::Variable *var = new collection::Variable(&m_name,`
`129`	`129`	`new std::string(content));`
`130`		`- var->m_dynamic_value = true;`
`131`	`130`	`l->push_back(var);`
`132`	`131`	`xmlFree(content);`
`133`	`132`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,6 @@ class XML_NoDictElement : public Variable {`
`41`	`41`	`m_plain("[XML document tree]"),`
`42`	`42`	`m_var(&m_name, &m_plain) {`
`43`	`43`	`m_var.m_dynamic = false;`
`44`		`- m_var.m_dynamic_value = false;`
`45`	`44`	`}`
`46`	`45`
`47`	`46`	`void evaluate(Transaction *transaction,`