From 47be22e62a4b2ab06dae1075c2187f47b462b2f1 Mon Sep 17 00:00:00 2001 From: WGH Date: Tue, 28 Jul 2020 18:46:03 +0300 Subject: [PATCH] Use std::shared_ptr for variable resolution AnchoredSetVariable::resolve is called for every rule (see RuleWithOperator::evaluate). The previous implementation allocated a new copy of every variable, which quickly added up. In my tests, AnchoredSetVariable::resolve function consumed 7.8% of run time. AnchoredSetVariable (which is a multimap) values are never changed, only added. This means it's safe to store them in std::shared_ptr, and make resolve return shared_ptr pointing to the same object. Other resolve implementation could also use this optimization by not allocating new objects, however, they are not hot spots, so this optimization was not implemented there. In my benchmark, this raises performance from 117 requests per second to 131 RPS, and overhead is lowered from 7.8% to 2.4%. As a bonus, replacing plain pointer with smart pointers make code cleaner, since using smart pointers makes manual deletes no longer necessary. Additionally, VariableOrigin is now stored in plain std::vector, since it's wasteful to store structure containing just two integer values using std::list>. --- headers/modsecurity/anchored_set_variable.h | 13 ++- headers/modsecurity/anchored_variable.h | 6 +- headers/modsecurity/collection/collection.h | 18 ++-- headers/modsecurity/variable_origin.h | 2 +- headers/modsecurity/variable_value.h | 20 ++--- src/actions/set_var.cc | 5 +- src/anchored_set_variable.cc | 75 +++++++--------- src/anchored_variable.cc | 75 ++++++---------- .../backend/in_memory-per_process.cc | 16 ++-- .../backend/in_memory-per_process.h | 6 +- src/collection/backend/lmdb.cc | 15 ++-- src/collection/backend/lmdb.h | 6 +- src/engine/lua.cc | 6 +- src/rule_with_operator.cc | 18 ++-- src/run_time_string.cc | 5 +- src/transaction.cc | 27 +++--- src/variables/duration.cc | 5 +- src/variables/duration.h | 2 +- src/variables/env.cc | 5 +- src/variables/env.h | 2 +- src/variables/global.h | 8 +- src/variables/highest_severity.cc | 5 +- src/variables/highest_severity.h | 2 +- src/variables/ip.h | 8 +- src/variables/modsec_build.cc | 4 +- src/variables/modsec_build.h | 2 +- src/variables/remote_user.cc | 35 +++----- src/variables/remote_user.h | 2 +- src/variables/resource.h | 8 +- src/variables/rule.h | 90 ++++++++----------- src/variables/session.h | 8 +- src/variables/time.cc | 5 +- src/variables/time.h | 2 +- src/variables/time_day.cc | 5 +- src/variables/time_day.h | 2 +- src/variables/time_epoch.cc | 5 +- src/variables/time_epoch.h | 2 +- src/variables/time_hour.cc | 5 +- src/variables/time_hour.h | 2 +- src/variables/time_min.cc | 5 +- src/variables/time_min.h | 2 +- src/variables/time_mon.cc | 5 +- src/variables/time_mon.h | 2 +- src/variables/time_sec.cc | 5 +- src/variables/time_sec.h | 2 +- src/variables/time_wday.cc | 5 +- src/variables/time_wday.h | 2 +- src/variables/time_year.cc | 5 +- src/variables/time_year.h | 2 +- src/variables/tx.h | 8 +- src/variables/user.h | 8 +- src/variables/variable.h | 35 +++----- src/variables/web_app_id.h | 5 +- src/variables/xml.cc | 11 +-- src/variables/xml.h | 10 +-- 55 files changed, 264 insertions(+), 375 deletions(-) diff --git a/headers/modsecurity/anchored_set_variable.h b/headers/modsecurity/anchored_set_variable.h index 02b5351b..790cf4d8 100644 --- a/headers/modsecurity/anchored_set_variable.h +++ b/headers/modsecurity/anchored_set_variable.h @@ -72,10 +72,9 @@ struct MyHash{ class AnchoredSetVariable : public std::unordered_multimap { + std::shared_ptr, MyHash, MyEqual> { public: AnchoredSetVariable(Transaction *t, const std::string &name); - ~AnchoredSetVariable(); void unset(); @@ -93,18 +92,18 @@ class AnchoredSetVariable : public std::unordered_multimap *l); - void resolve(std::vector *l, + void resolve(std::vector> *l); + void resolve(std::vector> *l, variables::KeyExclusions &ke); void resolve(const std::string &key, - std::vector *l); + std::vector> *l); void resolveRegularExpression(Utils::Regex *r, - std::vector *l); + std::vector> *l); void resolveRegularExpression(Utils::Regex *r, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke); std::unique_ptr resolveFirst(const std::string &key); diff --git a/headers/modsecurity/anchored_variable.h b/headers/modsecurity/anchored_variable.h index 1e8b5841..21cbb7bd 100644 --- a/headers/modsecurity/anchored_variable.h +++ b/headers/modsecurity/anchored_variable.h @@ -59,8 +59,6 @@ class AnchoredVariable { m_var(a.m_var) { } */ - ~AnchoredVariable(); - void unset(); void set(const std::string &a, size_t offset); void set(const bpstd::string_view &a, size_t offset); @@ -71,7 +69,7 @@ class AnchoredVariable { void append(const std::string &a, size_t offset, bool spaceSeparator, int size); - void evaluate(std::vector *l); + void evaluate(std::vector> *l); std::string * evaluate(); std::unique_ptr resolveFirst(); @@ -81,7 +79,7 @@ class AnchoredVariable { std::string m_value; private: - VariableValue *m_var; + VariableValue m_var; }; } // namespace modsecurity diff --git a/headers/modsecurity/collection/collection.h b/headers/modsecurity/collection/collection.h index db80e8e7..fbe4ec3d 100644 --- a/headers/modsecurity/collection/collection.h +++ b/headers/modsecurity/collection/collection.h @@ -60,12 +60,12 @@ class Collection { const std::string& var) = 0; virtual void resolveSingleMatch(const std::string& var, - std::vector *l) = 0; + std::vector> *l) = 0; virtual void resolveMultiMatches(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) = 0; virtual void resolveRegularExpression(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) = 0; @@ -146,7 +146,7 @@ class Collection { /* resolveSingleMatch */ virtual void resolveSingleMatch(const std::string& var, - std::string compartment, std::vector *l) { + std::string compartment, std::vector> *l) { std::string nkey = compartment + "::" + var; resolveSingleMatch(nkey, l); } @@ -154,7 +154,7 @@ class Collection { virtual void resolveSingleMatch(const std::string& var, std::string compartment, std::string compartment2, - std::vector *l) { + std::vector> *l) { std::string nkey = compartment + "::" + compartment2 + "::" + var; resolveSingleMatch(nkey, l); } @@ -162,7 +162,7 @@ class Collection { /* resolveMultiMatches */ virtual void resolveMultiMatches(const std::string& var, - std::string compartment, std::vector *l, + std::string compartment, std::vector> *l, variables::KeyExclusions &ke) { std::string nkey = compartment + "::" + var; resolveMultiMatches(nkey, l, ke); @@ -171,7 +171,7 @@ class Collection { virtual void resolveMultiMatches(const std::string& var, std::string compartment, std::string compartment2, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) { std::string nkey = compartment + "::" + compartment2 + "::" + var; resolveMultiMatches(nkey, l, ke); @@ -180,7 +180,7 @@ class Collection { /* resolveRegularExpression */ virtual void resolveRegularExpression(const std::string& var, - std::string compartment, std::vector *l, + std::string compartment, std::vector> *l, variables::KeyExclusions &ke) { std::string nkey = compartment + "::" + var; resolveRegularExpression(nkey, l, ke); @@ -189,7 +189,7 @@ class Collection { virtual void resolveRegularExpression(const std::string& var, std::string compartment, std::string compartment2, - std::vector *l, variables::KeyExclusions &ke) { + std::vector> *l, variables::KeyExclusions &ke) { std::string nkey = compartment + "::" + compartment2 + "::" + var; resolveRegularExpression(nkey, l, ke); } diff --git a/headers/modsecurity/variable_origin.h b/headers/modsecurity/variable_origin.h index 88079a7f..dce5e4ff 100644 --- a/headers/modsecurity/variable_origin.h +++ b/headers/modsecurity/variable_origin.h @@ -37,7 +37,7 @@ class VariableOrigin { : m_length(0), m_offset(0) { } - std::string toText() { + std::string toText() const { std::string offset = std::to_string(m_offset); std::string len = std::to_string(m_length); return "v" + offset + "," + len; diff --git a/headers/modsecurity/variable_value.h b/headers/modsecurity/variable_value.h index e632f1e6..a7d145f5 100644 --- a/headers/modsecurity/variable_value.h +++ b/headers/modsecurity/variable_value.h @@ -37,7 +37,7 @@ namespace modsecurity { class Collection; class VariableValue { public: - using Origins = std::list>; + using Origins = std::vector; explicit VariableValue(const std::string *key, const std::string *value = nullptr) @@ -63,10 +63,10 @@ class VariableValue { m_value(o->m_value) { for (auto &i : o->m_orign) { - std::unique_ptr origin(new VariableOrigin()); - origin->m_offset = i->m_offset; - origin->m_length = i->m_length; - m_orign.push_back(std::move(origin)); + VariableOrigin *origin(new VariableOrigin()); + origin->m_offset = i.m_offset; + origin->m_length = i.m_length; + m_orign.push_back(*origin); } } @@ -77,10 +77,10 @@ class VariableValue { m_value(o.m_value) { for (auto &i : o.m_orign) { - std::unique_ptr origin(new VariableOrigin()); - origin->m_offset = i->m_offset; - origin->m_length = i->m_length; - m_orign.push_back(std::move(origin)); + VariableOrigin *origin(new VariableOrigin()); + origin->m_offset = i.m_offset; + origin->m_length = i.m_length; + m_orign.push_back(*origin); } } @@ -109,7 +109,7 @@ class VariableValue { } - void addOrigin(std::unique_ptr origin) { + void addOrigin(VariableOrigin origin) { m_orign.push_back(std::move(origin)); } diff --git a/src/actions/set_var.cc b/src/actions/set_var.cc index a69a16e6..e650981e 100644 --- a/src/actions/set_var.cc +++ b/src/actions/set_var.cc @@ -113,15 +113,12 @@ bool SetVar::execute(Transaction *t) const noexcept { } try { - std::vector l; + std::vector> l; m_variable->evaluate(t, &l); if (l.size() == 0) { value = 0; } else { value = stoi(l[0]->getValue()); - for (auto &i : l) { - delete i; - } } } catch (...) { value = 0; diff --git a/src/anchored_set_variable.cc b/src/anchored_set_variable.cc index 9b2c495c..d74d2467 100644 --- a/src/anchored_set_variable.cc +++ b/src/anchored_set_variable.cc @@ -36,59 +36,45 @@ AnchoredSetVariable::AnchoredSetVariable(Transaction *t, } -AnchoredSetVariable::~AnchoredSetVariable() { - unset(); -} - - void AnchoredSetVariable::unset() { - for (const auto& x : *this) { - VariableValue *var = x.second; - delete var; - } clear(); } void AnchoredSetVariable::set(const std::string &key, const std::string &value, size_t offset, size_t len) { - std::unique_ptr origin(new VariableOrigin()); - std::string *v = new std::string(value); - VariableValue *var = new VariableValue(&m_name, &key, v); - delete v; + auto var = std::make_shared(&m_name, &key, &value); - origin->m_offset = offset; - origin->m_length = len; + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = len; var->addOrigin(std::move(origin)); - emplace(key, var); + emplace(key, std::move(var)); } void AnchoredSetVariable::set(const std::string &key, const std::string &value, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); - std::string *v = new std::string(value); - VariableValue *var = new VariableValue(&m_name, &key, v); - delete v; + auto var = std::make_shared(&m_name, &key, &value); - origin->m_offset = offset; - origin->m_length = value.size(); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = value.size(); var->addOrigin(std::move(origin)); - emplace(key, var); + emplace(key, std::move(var)); } void AnchoredSetVariable::set(const std::string &key, const bpstd::string_view &value, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); - std::string *v = new std::string(value.c_str()); - VariableValue *var = new VariableValue(&m_name, &key, v); - delete v; + std::string v(value.c_str()); + auto var = std::make_shared(&m_name, &key, &v); - origin->m_offset = offset; - origin->m_length = value.size(); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = value.size(); var->addOrigin(std::move(origin)); emplace(key, var); @@ -97,13 +83,12 @@ void AnchoredSetVariable::set(const std::string &key, void AnchoredSetVariable::set(const std::string &key, const char *value, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); - std::string *v = new std::string(value); - VariableValue *var = new VariableValue(&m_name, &key, v); - delete v; + std::string v(value); + auto var = std::make_shared(&m_name, &key, &v); - origin->m_offset = offset; - origin->m_length = strlen(value); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = strlen(value); var->addOrigin(std::move(origin)); emplace(key, var); @@ -111,19 +96,19 @@ void AnchoredSetVariable::set(const std::string &key, void AnchoredSetVariable::resolve( - std::vector *l) { + std::vector> *l) { for (const auto& x : *this) { - l->insert(l->begin(), new VariableValue(x.second)); + l->insert(l->begin(), x.second); } } void AnchoredSetVariable::resolve( - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) { for (const auto& x : *this) { if (!ke.toOmit(x.first)) { - l->insert(l->begin(), new VariableValue(x.second)); + l->insert(l->begin(), x.second); } else { ms_dbg_a(m_transaction, 7, "Excluding key: " + x.first + " from target value."); @@ -133,10 +118,10 @@ void AnchoredSetVariable::resolve( void AnchoredSetVariable::resolve(const std::string &key, - std::vector *l) { + std::vector> *l) { auto range = this->equal_range(key); for (auto it = range.first; it != range.second; ++it) { - l->push_back(new VariableValue(it->second)); + l->push_back(it->second); } } @@ -154,19 +139,19 @@ std::unique_ptr AnchoredSetVariable::resolveFirst( void AnchoredSetVariable::resolveRegularExpression(Utils::Regex *r, - std::vector *l) { + std::vector> *l) { for (const auto& x : *this) { int ret = Utils::regex_search(x.first, *r); if (ret <= 0) { continue; } - l->insert(l->begin(), new VariableValue(x.second)); + l->insert(l->begin(), x.second); } } void AnchoredSetVariable::resolveRegularExpression(Utils::Regex *r, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) { for (const auto& x : *this) { int ret = Utils::regex_search(x.first, *r); @@ -174,7 +159,7 @@ void AnchoredSetVariable::resolveRegularExpression(Utils::Regex *r, continue; } if (!ke.toOmit(x.first)) { - l->insert(l->begin(), new VariableValue(x.second)); + l->insert(l->begin(), x.second); } else { ms_dbg_a(m_transaction, 7, "Excluding key: " + x.first + " from target value."); diff --git a/src/anchored_variable.cc b/src/anchored_variable.cc index 7699dd5e..741a53f9 100644 --- a/src/anchored_variable.cc +++ b/src/anchored_variable.cc @@ -32,22 +32,11 @@ AnchoredVariable::AnchoredVariable(Transaction *t, const std::string &name) : m_transaction(t), m_offset(0), - m_name(""), + m_name(name), m_value(""), - m_var(NULL) { - m_name.append(name); - m_var = new VariableValue(&m_name); + m_var(&name) { } - -AnchoredVariable::~AnchoredVariable() { - if (m_var) { - delete (m_var); - m_var = NULL; - } -} - - void AnchoredVariable::unset() { m_value.clear(); } @@ -55,92 +44,86 @@ void AnchoredVariable::unset() { void AnchoredVariable::set(const std::string &a, size_t offset, size_t offsetLen) { - std::unique_ptr origin(new VariableOrigin()); m_offset = offset; m_value.assign(a.c_str(), a.size()); - origin->m_offset = offset; - origin->m_length = offsetLen; - m_var->addOrigin(std::move(origin)); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = offsetLen; + m_var.addOrigin(std::move(origin)); } void AnchoredVariable::set(const std::string &a, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); - m_offset = offset; m_value.assign(a.c_str(), a.size()); - origin->m_offset = offset; - origin->m_length = m_value.size(); - m_var->addOrigin(std::move(origin)); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = m_value.size(); + m_var.addOrigin(std::move(origin)); } void AnchoredVariable::set(const char *a, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); + VariableOrigin origin; m_offset = offset; m_value.assign(a, strlen(a)); - origin->m_offset = offset; - origin->m_length = m_value.size(); - m_var->addOrigin(std::move(origin)); + origin.m_offset = offset; + origin.m_length = m_value.size(); + m_var.addOrigin(std::move(origin)); } void AnchoredVariable::set(const bpstd::string_view &a, size_t offset) { - std::unique_ptr origin(new VariableOrigin()); + VariableOrigin origin; m_offset = offset; m_value.assign(a.c_str(), a.size()); - origin->m_offset = offset; - origin->m_length = m_value.size(); + origin.m_offset = offset; + origin.m_length = m_value.size(); - m_var->addOrigin(std::move(origin)); + m_var.addOrigin(std::move(origin)); } void AnchoredVariable::append(const std::string &a, size_t offset, bool spaceSeparator) { - std::unique_ptr origin( - new VariableOrigin()); - if (spaceSeparator && !m_value.empty()) { m_value.append(" " + a); } else { m_value.append(a); } m_offset = offset; - origin->m_offset = offset; - origin->m_length = a.size(); - m_var->addOrigin(std::move(origin)); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = a.size(); + m_var.addOrigin(std::move(origin)); } void AnchoredVariable::append(const std::string &a, size_t offset, bool spaceSeparator, int size) { - std::unique_ptr origin( - new VariableOrigin()); - if (spaceSeparator && !m_value.empty()) { m_value.append(" " + a); } else { m_value.append(a); } m_offset = offset; - origin->m_offset = offset; - origin->m_length = size; - m_var->addOrigin(std::move(origin)); + VariableOrigin origin; + origin.m_offset = offset; + origin.m_length = size; + m_var.addOrigin(std::move(origin)); } -void AnchoredVariable::evaluate(std::vector *l) { +void AnchoredVariable::evaluate(std::vector> *l) { if (m_name.empty()) { return; } - m_var->setValue(m_value); - VariableValue *m_var2 = new VariableValue(m_var); - l->push_back(m_var2); + m_var.setValue(m_value); + l->push_back(std::make_shared(m_var)); } diff --git a/src/collection/backend/in_memory-per_process.cc b/src/collection/backend/in_memory-per_process.cc index 717f9982..229eb1ca 100644 --- a/src/collection/backend/in_memory-per_process.cc +++ b/src/collection/backend/in_memory-per_process.cc @@ -86,17 +86,17 @@ void InMemoryPerProcess::del(const std::string& key) { void InMemoryPerProcess::resolveSingleMatch(const std::string& var, - std::vector *l) { + std::vector> *l) { auto range = this->equal_range(var); for (auto it = range.first; it != range.second; ++it) { - l->push_back(new VariableValue(&m_name, &it->first, &it->second)); + l->push_back(std::make_shared(&m_name, &it->first, &it->second)); } } void InMemoryPerProcess::resolveMultiMatches(const std::string& var, - std::vector *l, variables::KeyExclusions &ke) { + std::vector> *l, variables::KeyExclusions &ke) { size_t keySize = var.size(); l->reserve(15); @@ -105,8 +105,7 @@ void InMemoryPerProcess::resolveMultiMatches(const std::string& var, if (ke.toOmit(i.first)) { continue; } - l->insert(l->begin(), new VariableValue(&m_name, &i.first, - &i.second)); + l->insert(l->begin(), std::make_shared(&m_name, &i.first, &i.second)); } } else { auto range = this->equal_range(var); @@ -114,15 +113,14 @@ void InMemoryPerProcess::resolveMultiMatches(const std::string& var, if (ke.toOmit(var)) { continue; } - l->insert(l->begin(), new VariableValue(&m_name, &var, - &it->second)); + l->insert(l->begin(), std::make_shared(&m_name, &var, &it->second)); } } } void InMemoryPerProcess::resolveRegularExpression(const std::string& var, - std::vector *l, variables::KeyExclusions &ke) { + std::vector> *l, variables::KeyExclusions &ke) { //if (var.find(":") == std::string::npos) { // return; @@ -155,7 +153,7 @@ void InMemoryPerProcess::resolveRegularExpression(const std::string& var, if (ke.toOmit(x.first)) { continue; } - l->insert(l->begin(), new VariableValue(&m_name, &x.first, &x.second)); + l->insert(l->begin(), std::make_shared(&m_name, &x.first, &x.second)); } } diff --git a/src/collection/backend/in_memory-per_process.h b/src/collection/backend/in_memory-per_process.h index 0550fbc7..8955667b 100644 --- a/src/collection/backend/in_memory-per_process.h +++ b/src/collection/backend/in_memory-per_process.h @@ -87,12 +87,12 @@ class InMemoryPerProcess : std::unique_ptr resolveFirst(const std::string& var) override; void resolveSingleMatch(const std::string& var, - std::vector *l) override; + std::vector> *l) override; void resolveMultiMatches(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) override; void resolveRegularExpression(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) override; private: diff --git a/src/collection/backend/lmdb.cc b/src/collection/backend/lmdb.cc index 55afb1c5..8470028f 100644 --- a/src/collection/backend/lmdb.cc +++ b/src/collection/backend/lmdb.cc @@ -262,7 +262,7 @@ end_txn: void LMDB::resolveSingleMatch(const std::string& var, - std::vector *l) { + std::vector> *l) { int rc; MDB_txn *txn; MDB_dbi dbi; @@ -287,11 +287,10 @@ void LMDB::resolveSingleMatch(const std::string& var, mdb_cursor_open(txn, dbi, &cursor); while ((rc = mdb_cursor_get(cursor, &mdb_key, &mdb_value_ret, MDB_NEXT_DUP)) == 0) { - std::string *a = new std::string( + std::string a( reinterpret_cast(mdb_value_ret.mv_data), mdb_value_ret.mv_size); - VariableValue *v = new VariableValue(&var, a); - l->push_back(v); + l->emplace_back(&var, &a); } mdb_cursor_close(cursor); @@ -466,7 +465,7 @@ end_txn: void LMDB::resolveMultiMatches(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) { MDB_val key, data; MDB_txn *txn = NULL; @@ -496,7 +495,7 @@ void LMDB::resolveMultiMatches(const std::string& var, if (keySize == 0) { while ((rc = mdb_cursor_get(cursor, &key, &data, MDB_NEXT)) == 0) { - l->insert(l->begin(), new VariableValue( + l->insert(l->begin(), std::make_shared( &m_name, new std::string(reinterpret_cast(key.mv_data), key.mv_size), @@ -507,7 +506,7 @@ void LMDB::resolveMultiMatches(const std::string& var, while ((rc = mdb_cursor_get(cursor, &key, &data, MDB_NEXT)) == 0) { char *a = reinterpret_cast(key.mv_data); if (strncmp(var.c_str(), a, keySize) == 0) { - l->insert(l->begin(), new VariableValue( + l->insert(l->begin(), std::make_shared( &m_name, new std::string(reinterpret_cast(key.mv_data), key.mv_size), @@ -528,7 +527,7 @@ end_txn: void LMDB::resolveRegularExpression(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) { MDB_val key, data; MDB_txn *txn = NULL; diff --git a/src/collection/backend/lmdb.h b/src/collection/backend/lmdb.h index d31a8d95..f0044acb 100644 --- a/src/collection/backend/lmdb.h +++ b/src/collection/backend/lmdb.h @@ -66,12 +66,12 @@ class LMDB : std::unique_ptr resolveFirst(const std::string& var) override; void resolveSingleMatch(const std::string& var, - std::vector *l) override; + std::vector> *l) override; void resolveMultiMatches(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) override; void resolveRegularExpression(const std::string& var, - std::vector *l, + std::vector> *l, variables::KeyExclusions &ke) override; private: diff --git a/src/engine/lua.cc b/src/engine/lua.cc index 86da6fd1..065e8c11 100644 --- a/src/engine/lua.cc +++ b/src/engine/lua.cc @@ -286,7 +286,7 @@ int Lua::getvars(lua_State *L) { const char *varname(NULL); Transaction *t(NULL); void *z(NULL); - std::vector l; + std::vector> l; int idx = 1; /* Retrieve parameters. */ @@ -315,10 +315,6 @@ int Lua::getvars(lua_State *L) { idx++; } - for (const VariableValue * i : l) { - delete i; - } - return 1; } diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index 5bd46098..c73c8d03 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -265,14 +265,16 @@ bool RuleWithOperator::evaluate(Transaction *trans) const { getFinalVars(&vars, &exclusion, trans); + std::vector> e; for (auto &var : vars) { - std::vector e; if (!var) { continue; } + e.clear(); var->evaluate(trans, &e); - for (const VariableValue *v : e) { + for (const auto &vv : e) { TransformationsResults transformationsResults; + const VariableValue *v = vv.get(); const std::string &value = v->getValue(); const std::string &key = v->getKeyWithCollection(); @@ -283,8 +285,6 @@ bool RuleWithOperator::evaluate(Transaction *trans) const { return m.first == getId() && m.second == v->getKeyWithCollection(); }) != trans->m_ruleRemoveTargetById.end() ) { - delete v; - v = NULL; continue; } if (exclusion.contains(v) || @@ -294,8 +294,6 @@ bool RuleWithOperator::evaluate(Transaction *trans) const { return containsTag(m.first, trans) && m.second == v->getKeyWithCollection(); }) != trans->m_ruleRemoveTargetByTag.end() ) { - delete v; - v = NULL; continue; } @@ -317,8 +315,8 @@ bool RuleWithOperator::evaluate(Transaction *trans) const { trans->messageGetLast()->m_match = m_operator->resolveMatchMessage(trans, key, value); - for (auto &i : v->getOrigin()) { - trans->messageGetLast()->m_reference.append(i->toText()); + for (const auto &i : v->getOrigin()) { + trans->messageGetLast()->m_reference.append(i.toText()); } auto iter2 = transformationsResults.begin(); @@ -344,11 +342,7 @@ bool RuleWithOperator::evaluate(Transaction *trans) const { iter++; } - delete v; - v = NULL; } - e.clear(); - e.reserve(4); } if (globalRet == false) { diff --git a/src/run_time_string.cc b/src/run_time_string.cc index 3a95c7dc..5068a976 100644 --- a/src/run_time_string.cc +++ b/src/run_time_string.cc @@ -54,14 +54,11 @@ std::string RunTimeString::evaluate(Transaction *transaction) { if (element->m_string.size() > 0) { retString.append(element->m_string); } else if (element->m_variable != nullptr && transaction != nullptr) { - std::vector l; + std::vector> l; element->m_variable->evaluate(transaction, &l); if (!l.empty()) { retString.append(l[0]->getValue()); } - for (auto &i : l) { - delete i; - } } } return retString; diff --git a/src/transaction.cc b/src/transaction.cc index 6368563f..91d0c27f 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -962,11 +962,10 @@ int Transaction::processRequestBody() { * computationally intensive. */ std::string fullRequest; - std::vector l; + std::vector> l; m_variableRequestHeaders.resolve(&l); - for (auto &h : l) { + for (const auto &h : l) { fullRequest = fullRequest + h->getKey() + ": " + h->getValue() + "\n"; - delete h; } fullRequest = fullRequest + "\n\n"; @@ -1492,7 +1491,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename, ss << utils::string::dash_if_empty(this->m_clientIpAddress->c_str()) << " "; /** TODO: Check variable */ variables::RemoteUser *r = new variables::RemoteUser("REMOTE_USER"); - std::vector l; + std::vector> l; r->evaluate(this, &l); delete r; @@ -1555,7 +1554,7 @@ std::string Transaction::toOldAuditLogFormat(int parts, audit_log << std::endl; if (parts & audit_log::AuditLog::BAuditLogPart) { - std::vector l; + std::vector> l; audit_log << "--" << trailer << "-" << "B--" << std::endl; audit_log << utils::string::dash_if_empty( m_variableRequestMethod.evaluate()); @@ -1563,11 +1562,10 @@ std::string Transaction::toOldAuditLogFormat(int parts, audit_log << this->m_httpVersion.c_str() << std::endl; m_variableRequestHeaders.resolve(&l); - for (auto &h : l) { + for (const auto &h : l) { size_t pos = strlen("REQUEST_HEADERS:"); audit_log << h->getKeyWithCollection().c_str() + pos << ": "; audit_log << h->getValue().c_str() << std::endl; - delete h; } audit_log << std::endl; } @@ -1595,16 +1593,15 @@ std::string Transaction::toOldAuditLogFormat(int parts, audit_log << std::endl; } if (parts & audit_log::AuditLog::FAuditLogPart) { - std::vector l; + std::vector> l; audit_log << "--" << trailer << "-" << "F--" << std::endl; audit_log << "HTTP/" << m_httpVersion.c_str() << " "; audit_log << this->m_httpCodeReturned << std::endl; m_variableResponseHeaders.resolve(&l); - for (auto &h : l) { + for (const auto &h : l) { audit_log << h->getKey().c_str() << ": "; audit_log << h->getValue().c_str() << std::endl; - delete h; } } audit_log << std::endl; @@ -1697,15 +1694,14 @@ std::string Transaction::toJSON(int parts) { /* request headers */ if (parts & audit_log::AuditLog::BAuditLogPart) { - std::vector l; + std::vector> l; yajl_gen_string(g, reinterpret_cast("headers"), strlen("headers")); yajl_gen_map_open(g); m_variableRequestHeaders.resolve(&l); - for (auto &h : l) { + for (const auto &h : l) { LOGFY_ADD(h->getKey().c_str(), h->getValue().c_str()); - delete h; } /* end: request headers */ @@ -1727,15 +1723,14 @@ std::string Transaction::toJSON(int parts) { /* response headers */ if (parts & audit_log::AuditLog::FAuditLogPart) { - std::vector l; + std::vector> l; yajl_gen_string(g, reinterpret_cast("headers"), strlen("headers")); yajl_gen_map_open(g); m_variableResponseHeaders.resolve(&l); - for (auto &h : l) { + for (const auto &h : l) { LOGFY_ADD(h->getKey().c_str(), h->getValue().c_str()); - delete h; } /* end: response headers */ diff --git a/src/variables/duration.cc b/src/variables/duration.cc index cfbf98c9..aaf9782a 100644 --- a/src/variables/duration.cc +++ b/src/variables/duration.cc @@ -28,13 +28,12 @@ namespace modsecurity { namespace variables { void Duration::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { double e = utils::cpu_seconds() - transaction->m_creationTimeStamp; transaction->m_variableDuration.assign(std::to_string(e)); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableDuration)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableDuration)); } diff --git a/src/variables/duration.h b/src/variables/duration.h index ff97affc..992117d9 100644 --- a/src/variables/duration.h +++ b/src/variables/duration.h @@ -35,7 +35,7 @@ class Duration : public Variable { m_retName("DURATION") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/env.cc b/src/variables/env.cc index 10b34076..7de87d84 100644 --- a/src/variables/env.cc +++ b/src/variables/env.cc @@ -33,7 +33,7 @@ namespace modsecurity { namespace variables { void Env::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { for (char **current = environ; *current; current++) { std::string env = std::string(*current); size_t pos = env.find_first_of("="); @@ -51,8 +51,7 @@ void Env::evaluate(Transaction *transaction, continue; } if (!m_keyExclusion.toOmit(x.first)) { - l->push_back(new VariableValue(&m_collectionName, &x.first, - &x.second)); + l->emplace_back(std::make_shared(&m_collectionName, &x.first, &x.second)); } } } diff --git a/src/variables/env.h b/src/variables/env.h index b41101b6..92591240 100644 --- a/src/variables/env.h +++ b/src/variables/env.h @@ -34,7 +34,7 @@ class Env : public Variable { : Variable(_name) { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; }; } // namespace variables diff --git a/src/variables/global.h b/src/variables/global.h index 6aaacbbc..4d32f55f 100644 --- a/src/variables/global.h +++ b/src/variables/global.h @@ -40,7 +40,7 @@ class Global_DictElement : public Variable { m_dictElement("GLOBAL:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_global_collection->resolveMultiMatches( m_name, t->m_collections.m_global_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -56,7 +56,7 @@ class Global_NoDictElement : public Variable { : Variable("GLOBAL") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_global_collection->resolveMultiMatches("", t->m_collections.m_global_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -71,7 +71,7 @@ class Global_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_global_collection->resolveRegularExpression( m_dictElement, t->m_collections.m_global_collection_key, @@ -92,7 +92,7 @@ class Global_DynamicElement : public VariableWithRunTimeString { { }; void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_global_collection->resolveMultiMatches( string, diff --git a/src/variables/highest_severity.cc b/src/variables/highest_severity.cc index 068797f6..c2db0f21 100644 --- a/src/variables/highest_severity.cc +++ b/src/variables/highest_severity.cc @@ -27,11 +27,10 @@ namespace modsecurity { namespace variables { void HighestSeverity::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { transaction->m_variableHighestSeverityAction.assign( std::to_string(transaction->m_highestSeverityAction)); - l->push_back(new VariableValue(m_fullName.get(), - &transaction->m_variableHighestSeverityAction)); + l->push_back(std::make_shared(m_fullName.get(), &transaction->m_variableHighestSeverityAction)); } diff --git a/src/variables/highest_severity.h b/src/variables/highest_severity.h index 2479d6c1..bbc8af3b 100644 --- a/src/variables/highest_severity.h +++ b/src/variables/highest_severity.h @@ -35,7 +35,7 @@ class HighestSeverity : public Variable { { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; }; diff --git a/src/variables/ip.h b/src/variables/ip.h index 1622e033..12f076d6 100644 --- a/src/variables/ip.h +++ b/src/variables/ip.h @@ -40,7 +40,7 @@ class Ip_DictElement : public Variable { m_dictElement("IP:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_ip_collection->resolveMultiMatches( m_name, t->m_collections.m_ip_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -56,7 +56,7 @@ class Ip_NoDictElement : public Variable { : Variable("IP") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_ip_collection->resolveMultiMatches("", t->m_collections.m_ip_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -71,7 +71,7 @@ class Ip_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_ip_collection->resolveRegularExpression( m_dictElement, t->m_collections.m_ip_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -91,7 +91,7 @@ class Ip_DynamicElement : public VariableWithRunTimeString { { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_ip_collection->resolveMultiMatches( string, diff --git a/src/variables/modsec_build.cc b/src/variables/modsec_build.cc index d296fa2d..ec509a42 100644 --- a/src/variables/modsec_build.cc +++ b/src/variables/modsec_build.cc @@ -25,9 +25,9 @@ namespace modsecurity { namespace variables { void ModsecBuild::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { - l->push_back(new VariableValue(&m_retName, &m_build)); + l->push_back(std::make_shared(&m_retName, &m_build)); } diff --git a/src/variables/modsec_build.h b/src/variables/modsec_build.h index 0ab101e5..6fa75856 100644 --- a/src/variables/modsec_build.h +++ b/src/variables/modsec_build.h @@ -44,7 +44,7 @@ class ModsecBuild : public Variable { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_build; std::string m_retName; diff --git a/src/variables/remote_user.cc b/src/variables/remote_user.cc index 25f09b9f..3d13e43b 100644 --- a/src/variables/remote_user.cc +++ b/src/variables/remote_user.cc @@ -37,21 +37,19 @@ namespace variables { void RemoteUser::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { size_t pos; std::string base64; - VariableValue *var; std::string header; - std::vector *l2 = \ - new std::vector(); - transaction->m_variableRequestHeaders.resolve("authorization", l2); + std::vector> l2; + transaction->m_variableRequestHeaders.resolve("authorization", &l2); - if (l2->size() < 1) { - goto clear; + if (l2.size() < 1) { + return; } - header = std::string(l2->at(0)->getValue()); + header = std::string(l2.at(0)->getValue()); if (header.compare(0, 6, "Basic ") == 0) { base64 = std::string(header, 6, header.length()); @@ -61,27 +59,16 @@ void RemoteUser::evaluate(Transaction *transaction, pos = base64.find(":"); if (pos == std::string::npos) { - goto clear; + return; } transaction->m_variableRemoteUser.assign(std::string(base64, 0, pos)); - var = new VariableValue(&l2->at(0)->getKeyWithCollection(), - &transaction->m_variableRemoteUser); + auto var = std::make_shared(&l2[0]->getKeyWithCollection(), &transaction->m_variableRemoteUser); - for (auto &i : l2->at(0)->getOrigin()) { - std::unique_ptr origin(new VariableOrigin()); - origin->m_offset = i->m_offset; - origin->m_length = i->m_length; - var->addOrigin(std::move(origin)); + for (auto &i : l2[0]->getOrigin()) { + var->addOrigin(i); } - l->push_back(var); - -clear: - for (auto &a : *l2) { - delete a; - } - l2->clear(); - delete l2; + l->push_back(std::move(var)); } diff --git a/src/variables/remote_user.h b/src/variables/remote_user.h index 370c892c..af2c121f 100644 --- a/src/variables/remote_user.h +++ b/src/variables/remote_user.h @@ -37,7 +37,7 @@ class RemoteUser : public Variable { m_retName("REMOTE_USER") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/resource.h b/src/variables/resource.h index edcc4a4a..f3ad152b 100644 --- a/src/variables/resource.h +++ b/src/variables/resource.h @@ -40,7 +40,7 @@ class Resource_DictElement : public Variable { m_dictElement("RESOURCE:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_resource_collection->resolveMultiMatches( m_name, t->m_collections.m_resource_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -56,7 +56,7 @@ class Resource_NoDictElement : public Variable { : Variable("RESOURCE") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_resource_collection->resolveMultiMatches(m_name, t->m_collections.m_resource_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -71,7 +71,7 @@ class Resource_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_resource_collection->resolveRegularExpression( m_dictElement, t->m_collections.m_resource_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -91,7 +91,7 @@ class Resource_DynamicElement : public VariableWithRunTimeString { { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_resource_collection->resolveMultiMatches( string, diff --git a/src/variables/rule.h b/src/variables/rule.h index 475b3c31..b09089d0 100644 --- a/src/variables/rule.h +++ b/src/variables/rule.h @@ -56,97 +56,83 @@ class Rule_DictElement : public RuleVariable, public VariableDictElement { static void id(Transaction *t, const RuleWithActions *rule, - std::vector *l) { - std::unique_ptr origin(new VariableOrigin()); - std::string *a = new std::string(std::to_string(rule->getId())); - VariableValue *var = new VariableValue(&m_rule, &m_rule_id, - a - ); - delete a; - origin->m_offset = 0; - origin->m_length = 0; + std::vector> *l) { + std::string a = std::to_string(rule->getId()); + auto var = std::make_shared(&m_rule, &m_rule_id, &a); + VariableOrigin origin; + origin.m_offset = 0; + origin.m_length = 0; var->addOrigin(std::move(origin)); - l->push_back(var); + l->push_back(std::move(var)); } static void rev(Transaction *t, const RuleWithActions *rule, - std::vector *l) { + std::vector> *l) { if (rule->hasRevisionAction()) { - std::unique_ptr origin(new VariableOrigin()); - std::string *a = new std::string(rule->getRevision()); - VariableValue *var = new VariableValue(&m_rule, &m_rule_rev, - a - ); - delete a; - origin->m_offset = 0; - origin->m_length = 0; + std::string a(rule->getRevision()); + auto var = std::make_shared(&m_rule, &m_rule_rev, &a); + VariableOrigin origin; + origin.m_offset = 0; + origin.m_length = 0; var->addOrigin(std::move(origin)); l->push_back(var); + l->push_back(std::move(var)); } } static void severity(Transaction *t, const RuleWithActions *rule, - std::vector *l) { + std::vector> *l) { if (rule->hasSeverityAction()) { - std::unique_ptr origin(new VariableOrigin()); - std::string *a = new std::string(std::to_string(rule->getSeverity())); - VariableValue *var = new VariableValue(&m_rule, &m_rule_severity, - a - ); - delete a; - origin->m_offset = 0; - origin->m_length = 0; + std::string a(std::to_string(rule->getSeverity())); + auto var = std::make_shared(&m_rule, &m_rule_severity, &a); + VariableOrigin origin; + origin.m_offset = 0; + origin.m_length = 0; var->addOrigin(std::move(origin)); - l->push_back(var); + l->push_back(std::move(var)); } } static void logData(Transaction *t, const RuleWithActions *rule, - std::vector *l) { + std::vector> *l) { if (rule->hasLogDataAction()) { - std::unique_ptr origin(new VariableOrigin()); - std::string *a = new std::string(rule->getLogData(t)); - VariableValue *var = new VariableValue(&m_rule, &m_rule_logdata, - a - ); - delete a; - origin->m_offset = 0; - origin->m_length = 0; + std::string a(rule->getLogData(t)); + auto var = std::make_shared(&m_rule, &m_rule_logdata, &a); + VariableOrigin origin; + origin.m_offset = 0; + origin.m_length = 0; var->addOrigin(std::move(origin)); - l->push_back(var); + l->push_back(std::move(var)); } } static void msg(Transaction *t, const RuleWithActions *rule, - std::vector *l) { + std::vector> *l) { if (rule->hasMessageAction()) { - std::unique_ptr origin(new VariableOrigin()); - std::string *a = new std::string(rule->getMessage(t)); - VariableValue *var = new VariableValue(&m_rule, &m_rule_msg, - a - ); - delete a; - origin->m_offset = 0; - origin->m_length = 0; + std::string a(rule->getMessage(t)); + auto var = std::make_shared(&m_rule, &m_rule_msg, &a); + VariableOrigin origin; + origin.m_offset = 0; + origin.m_length = 0; var->addOrigin(std::move(origin)); - l->push_back(var); + l->push_back(std::move(var)); } } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { if (m_dictElement == "id") { id(t, getRule(), l); @@ -194,7 +180,7 @@ class Rule_DictElementRegexp : public RuleVariable, public VariableRegex { void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { if (Utils::regex_search("id", m_r) > 0) { Rule_DictElement::id(t, getRule(), l); @@ -239,7 +225,7 @@ class Rule_NoDictElement : public RuleVariable, public Variable { void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { Rule_DictElement::id(t, getRule(), l); Rule_DictElement::rev(t, getRule(), l); Rule_DictElement::severity(t, getRule(), l); diff --git a/src/variables/session.h b/src/variables/session.h index ce810080..0ac59bc8 100644 --- a/src/variables/session.h +++ b/src/variables/session.h @@ -39,7 +39,7 @@ class Session_DictElement : public Variable { m_dictElement("SESSION:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_session_collection->resolveMultiMatches( m_name, t->m_collections.m_session_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -55,7 +55,7 @@ class Session_NoDictElement : public Variable { : Variable("SESSION") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_session_collection->resolveMultiMatches("", t->m_collections.m_session_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -70,7 +70,7 @@ class Session_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_session_collection->resolveRegularExpression( m_dictElement, t->m_collections.m_session_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -90,7 +90,7 @@ class Session_DynamicElement : public VariableWithRunTimeString { { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_session_collection->resolveMultiMatches( string, diff --git a/src/variables/time.cc b/src/variables/time.cc index 63872e9e..57457a6e 100644 --- a/src/variables/time.cc +++ b/src/variables/time.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void Time::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; @@ -47,8 +47,7 @@ void Time::evaluate(Transaction *transaction, strftime(tstr, 200, "%H:%M:%S", &timeinfo); transaction->m_variableTime.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTime)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTime)); } diff --git a/src/variables/time.h b/src/variables/time.h index 5499527f..2ef1fe72 100644 --- a/src/variables/time.h +++ b/src/variables/time.h @@ -36,7 +36,7 @@ class Time : public Variable { m_retName("TIME") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_day.cc b/src/variables/time_day.cc index 08c77047..23cd3ea4 100644 --- a/src/variables/time_day.cc +++ b/src/variables/time_day.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeDay::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeDay::evaluate(Transaction *transaction, transaction->m_variableTimeDay.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeDay)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeDay)); } diff --git a/src/variables/time_day.h b/src/variables/time_day.h index c977670a..532a5962 100644 --- a/src/variables/time_day.h +++ b/src/variables/time_day.h @@ -35,7 +35,7 @@ class TimeDay : public Variable { m_retName("TIME_DAY") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_epoch.cc b/src/variables/time_epoch.cc index a96b4e0d..75c40ab3 100644 --- a/src/variables/time_epoch.cc +++ b/src/variables/time_epoch.cc @@ -34,11 +34,10 @@ namespace modsecurity { namespace variables { void TimeEpoch::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { transaction->m_variableTimeEpoch.assign( std::to_string(std::time(nullptr))); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeEpoch)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeEpoch)); } diff --git a/src/variables/time_epoch.h b/src/variables/time_epoch.h index 322b4480..cdd9d828 100644 --- a/src/variables/time_epoch.h +++ b/src/variables/time_epoch.h @@ -35,7 +35,7 @@ class TimeEpoch : public Variable { m_retName("TIME_EPOCH") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_hour.cc b/src/variables/time_hour.cc index 9d0e842a..9c149e1c 100644 --- a/src/variables/time_hour.cc +++ b/src/variables/time_hour.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeHour::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeHour::evaluate(Transaction *transaction, transaction->m_variableTimeHour.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeHour)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeHour)); } diff --git a/src/variables/time_hour.h b/src/variables/time_hour.h index 222de215..cfe1a420 100644 --- a/src/variables/time_hour.h +++ b/src/variables/time_hour.h @@ -35,7 +35,7 @@ class TimeHour : public Variable { m_retName("TIME_HOUR") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_min.cc b/src/variables/time_min.cc index 693fbf7c..3711f19e 100644 --- a/src/variables/time_min.cc +++ b/src/variables/time_min.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeMin::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeMin::evaluate(Transaction *transaction, transaction->m_variableTimeMin.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeMin)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeMin)); } diff --git a/src/variables/time_min.h b/src/variables/time_min.h index 272ae8f2..a2923008 100644 --- a/src/variables/time_min.h +++ b/src/variables/time_min.h @@ -35,7 +35,7 @@ class TimeMin : public Variable { m_retName("TIME_MIN") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_mon.cc b/src/variables/time_mon.cc index 1f70f3ed..5af0f37d 100644 --- a/src/variables/time_mon.cc +++ b/src/variables/time_mon.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeMon::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -49,8 +49,7 @@ void TimeMon::evaluate(Transaction *transaction, transaction->m_variableTimeMin.assign(std::to_string(a)); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeMin)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeMin)); } diff --git a/src/variables/time_mon.h b/src/variables/time_mon.h index 80cd631d..3da58036 100644 --- a/src/variables/time_mon.h +++ b/src/variables/time_mon.h @@ -35,7 +35,7 @@ class TimeMon : public Variable { m_retName("TIME_MON") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_sec.cc b/src/variables/time_sec.cc index 8869082d..e448ad76 100644 --- a/src/variables/time_sec.cc +++ b/src/variables/time_sec.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeSec::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeSec::evaluate(Transaction *transaction, transaction->m_variableTimeSec.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeSec)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeSec)); } diff --git a/src/variables/time_sec.h b/src/variables/time_sec.h index 0813785e..9245cbac 100644 --- a/src/variables/time_sec.h +++ b/src/variables/time_sec.h @@ -35,7 +35,7 @@ class TimeSec : public Variable { m_retName("TIME_SEC") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_wday.cc b/src/variables/time_wday.cc index 04f91ac7..ab8e310b 100644 --- a/src/variables/time_wday.cc +++ b/src/variables/time_wday.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeWDay::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeWDay::evaluate(Transaction *transaction, transaction->m_variableTimeWDay.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeWDay)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeWDay)); } diff --git a/src/variables/time_wday.h b/src/variables/time_wday.h index 5b258ea8..68bc459d 100644 --- a/src/variables/time_wday.h +++ b/src/variables/time_wday.h @@ -35,7 +35,7 @@ class TimeWDay : public Variable { m_retName("TIME_WDAY") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/time_year.cc b/src/variables/time_year.cc index 0a39c93a..bccb1542 100644 --- a/src/variables/time_year.cc +++ b/src/variables/time_year.cc @@ -34,7 +34,7 @@ namespace modsecurity { namespace variables { void TimeYear::evaluate(Transaction *transaction, - std::vector *l) { + std::vector> *l) { char tstr[200]; struct tm timeinfo; time_t timer; @@ -47,8 +47,7 @@ void TimeYear::evaluate(Transaction *transaction, transaction->m_variableTimeYear.assign(tstr); - l->push_back(new VariableValue(&m_retName, - &transaction->m_variableTimeYear)); + l->push_back(std::make_shared(&m_retName, &transaction->m_variableTimeYear)); } diff --git a/src/variables/time_year.h b/src/variables/time_year.h index f14eb4a5..4a9cf9f4 100644 --- a/src/variables/time_year.h +++ b/src/variables/time_year.h @@ -35,7 +35,7 @@ class TimeYear : public Variable { m_retName("TIME_YEAR") { } void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; std::string m_retName; }; diff --git a/src/variables/tx.h b/src/variables/tx.h index ab426b04..3b2ce073 100644 --- a/src/variables/tx.h +++ b/src/variables/tx.h @@ -40,7 +40,7 @@ class Tx_DictElement : public Variable { m_dictElement("TX:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_tx_collection->resolveMultiMatches( m_name, l, m_keyExclusion); } @@ -55,7 +55,7 @@ class Tx_NoDictElement : public Variable { : Variable("TX") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_tx_collection->resolveMultiMatches("", l, m_keyExclusion); } @@ -69,7 +69,7 @@ class Tx_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_tx_collection->resolveRegularExpression( m_dictElement, l, m_keyExclusion); } @@ -88,7 +88,7 @@ class Tx_DynamicElement : public VariableWithRunTimeString { { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_tx_collection->resolveMultiMatches(string, l, m_keyExclusion); diff --git a/src/variables/user.h b/src/variables/user.h index b8d758a5..188df8fe 100644 --- a/src/variables/user.h +++ b/src/variables/user.h @@ -40,7 +40,7 @@ class User_DictElement : public Variable { m_dictElement("USER:" + dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_user_collection->resolveMultiMatches( m_name, t->m_collections.m_user_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -56,7 +56,7 @@ class User_NoDictElement : public Variable { : Variable("USER") { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_user_collection->resolveMultiMatches(m_name, t->m_collections.m_user_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -71,7 +71,7 @@ class User_DictElementRegexp : public VariableRegex { m_dictElement(dictElement) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { t->m_collections.m_user_collection->resolveRegularExpression( m_dictElement, t->m_collections.m_user_collection_key, t->m_rules->m_secWebAppId.m_value, l, m_keyExclusion); @@ -91,7 +91,7 @@ class User_DynamicElement : public VariableWithRunTimeString { { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { std::string string = m_string->evaluate(t); t->m_collections.m_user_collection->resolveMultiMatches( string, diff --git a/src/variables/variable.h b/src/variables/variable.h index 62c1f786..c8ebb4e6 100644 --- a/src/variables/variable.h +++ b/src/variables/variable.h @@ -48,7 +48,7 @@ class n ## _DictElementRegexp : public VariableRegex { \ : VariableRegex(#N, regex) { } \ \ void evaluate(Transaction *transaction, \ - std::vector *l) override { \ + std::vector> *l) override { \ transaction-> e .resolveRegularExpression(&m_r, l, \ m_keyExclusion); \ } \ @@ -62,7 +62,7 @@ class n ## _DictElement : public VariableDictElement { \ : VariableDictElement(#N, dictElement) { } \ \ void evaluate(Transaction *transaction, \ - std::vector *l) override { \ + std::vector> *l) override { \ transaction-> e .resolve(m_dictElement, l); \ } \ }; @@ -75,7 +75,7 @@ class n ## _NoDictElement : public Variable { \ : Variable(#N) { } \ \ void evaluate(Transaction *transaction, \ - std::vector *l) override { \ + std::vector> *l) override { \ transaction-> e .resolve(l, m_keyExclusion); \ } \ }; @@ -88,7 +88,7 @@ class n : public Variable { \ : Variable(#N) { } \ \ void evaluate(Transaction *transaction, \ - std::vector *l) override { \ + std::vector> *l) override { \ transaction-> e .evaluate(l); \ } \ }; @@ -186,7 +186,7 @@ class VariableMonkeyResolution { static void stringMatchResolveMulti(Transaction *t, const std::string &variable, - std::vector *l) { + std::vector> *l) { size_t collection = variable.find("."); if (collection == std::string::npos) { collection = variable.find(":"); @@ -567,7 +567,7 @@ class Variable : public VariableMonkeyResolution { virtual void evaluate(Transaction *t, - std::vector *l) = 0; + std::vector> *l) = 0; bool inline belongsToCollection(Variable *var) { @@ -675,7 +675,7 @@ class VariableModificatorExclusion : public Variable { m_base(std::move(var)) { } void evaluate(Transaction *t, - std::vector *l) override { + std::vector> *l) override { m_base->evaluate(t, l); } @@ -692,25 +692,14 @@ class VariableModificatorCount : public Variable { } void evaluate(Transaction *t, - std::vector *l) override { - std::vector reslIn; - VariableValue *val = NULL; - int count = 0; + std::vector> *l) override { + std::vector> reslIn; m_base->evaluate(t, &reslIn); + auto count = reslIn.size(); - for (const VariableValue *a : reslIn) { - count++; - delete a; - a = NULL; - } - reslIn.clear(); - - std::string *res = new std::string(std::to_string(count)); - val = new VariableValue(m_fullName.get(), res); - delete res; - - l->push_back(val); + std::string res(std::to_string(count)); + l->push_back(std::make_shared(m_fullName.get(), &res)); return; } diff --git a/src/variables/web_app_id.h b/src/variables/web_app_id.h index 68a87858..e36daf5c 100644 --- a/src/variables/web_app_id.h +++ b/src/variables/web_app_id.h @@ -36,9 +36,10 @@ class WebAppId : public Variable { : Variable("WEBAPPID") { } void evaluate(Transaction *transaction, - std::vector *l) override { + std::vector> *l) override { + const std::string name("WEBAPPID"); const std::string rname = transaction->m_rules->m_secWebAppId.m_value; - l->push_back(new VariableValue(&m_name, &rname)); + l->push_back(std::make_shared(&m_name, &rname)); } }; diff --git a/src/variables/xml.cc b/src/variables/xml.cc index af4026f3..e343a87e 100644 --- a/src/variables/xml.cc +++ b/src/variables/xml.cc @@ -50,11 +50,11 @@ namespace variables { #ifndef WITH_LIBXML2 void XML_WithNSPath::evaluate(Transaction *t, - std::vector *l) { } + std::vector> *l) { } #else void XML_WithNSPath::evaluate(Transaction *t, - std::vector *l) { + std::vector> *l) { xmlXPathContextPtr xpathCtx; xmlXPathObjectPtr xpathObj; xmlNodeSetPtr nodes; @@ -123,13 +123,10 @@ void XML_WithNSPath::evaluate(Transaction *t, content = reinterpret_cast( xmlNodeGetContent(nodes->nodeTab[i])); if (content != NULL) { - std::string *a = new std::string(content); - VariableValue *var = new VariableValue(m_fullName.get(), - a); + std::string a(content); if (!m_keyExclusion.toOmit(*m_fullName)) { - l->push_back(var); + l->push_back(std::make_shared(m_fullName.get(), &a)); } - delete a; xmlFree(content); } } diff --git a/src/variables/xml.h b/src/variables/xml.h index fae54895..90437621 100644 --- a/src/variables/xml.h +++ b/src/variables/xml.h @@ -40,7 +40,7 @@ class XML_WithoutNSPath : public RuleVariable, public Variable { : RuleVariable(), Variable("XML"), m_plain("[XML document tree]"), - m_var(&m_name, &m_plain) + m_var(std::make_shared(&m_name, &m_plain)) { }; XML_WithoutNSPath(const XML_WithoutNSPath &r) @@ -51,8 +51,8 @@ class XML_WithoutNSPath : public RuleVariable, public Variable { { }; void evaluate(Transaction *transaction, - std::vector *l) override { - l->push_back(new VariableValue(&m_var)); + std::vector> *l) override { + l->push_back(m_var); } virtual variables::Variable *clone() override { @@ -60,7 +60,7 @@ class XML_WithoutNSPath : public RuleVariable, public Variable { }; std::string m_plain; - VariableValue m_var; + std::shared_ptr m_var; }; class XML_WithNSPath : public RuleVariable, public VariableDictElement { @@ -76,7 +76,7 @@ class XML_WithNSPath : public RuleVariable, public VariableDictElement { { }; void evaluate(Transaction *transaction, - std::vector *l) override; + std::vector> *l) override; virtual Variable *clone() override { return new XML_WithNSPath(*this);