Skip to content

Commit 47dd9c5

Browse files
zimmerlezimmerle
committed
Refactoring on the VariableValue class
1 parent cbd15ec commit 47dd9c5

File tree

16 files changed

+149
-114
lines changed

16 files changed

+149
-114
lines changed

‎headers/modsecurity/variable_value.h‎

Lines changed: 61 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -37,59 +37,31 @@ namespace modsecurity {
3737
class Collection;
3838
class VariableValue {
3939
public:
40-
explicit VariableValue(const std::string *key)
41-
: m_key(""),
42-
m_value("") {
43-
m_key.assign(*key);
44-
m_keyWithCollection = std::make_shared<std::string>(*key);
45-
}
46-
47-
VariableValue(const std::string *key, const std::string *value)
48-
: m_key(""),
49-
m_value("") {
50-
m_key.assign(*key);
51-
m_value.assign(*value);
52-
m_keyWithCollection = std::make_shared<std::string>(*key);
53-
}
54-
55-
VariableValue()
56-
: m_key(""),
57-
m_value("") {
58-
m_keyWithCollection = std::make_shared<std::string>(m_key);
59-
}
60-
61-
VariableValue(const std::string *a, const std::string *b,
62-
const std::string *c)
63-
: m_key(*a + ":" + *b),
64-
m_value(*c) {
65-
m_keyWithCollection = std::make_shared<std::string>(*a + ":" + *b);
66-
}
67-
68-
explicit VariableValue(std::shared_ptr<std::string> fullName)
69-
: m_key(""),
70-
m_value("") {
71-
m_keyWithCollection = fullName;
72-
m_key.assign(*fullName.get());
73-
}
74-
75-
VariableValue(std::shared_ptr<std::string> fullName,
40+
using Origins = std::list<std::unique_ptr<VariableOrigin>>;
41+
42+
VariableValue(const std::string *key,
43+
const std::string *value = nullptr)
44+
: m_key(*key),
45+
m_keyWithCollection(*key),
46+
m_collection(""),
47+
m_value(value != nullptr?*value:"")
48+
{ }
49+
50+
VariableValue(const std::string *collection,
51+
const std::string *key,
7652
const std::string *value)
77-
: m_key(""),
78-
m_value("") {
79-
m_value.assign(*value);
80-
m_keyWithCollection = fullName;
81-
m_key.assign(*fullName.get());
82-
}
83-
53+
: m_key(*key),
54+
m_keyWithCollection(*collection + ":" + *key),
55+
m_collection(*collection),
56+
m_value(*value)
57+
{ }
8458

8559
explicit VariableValue(const VariableValue *o) :
86-
m_key(""),
87-
m_value("") {
88-
m_key.assign(o->m_key);
89-
m_value.assign(o->m_value);
90-
m_col.assign(o->m_col);
91-
m_keyWithCollection = o->m_keyWithCollection;
92-
60+
m_key(o->m_key),
61+
m_value(o->m_value),
62+
m_collection(o->m_collection),
63+
m_keyWithCollection(o->m_keyWithCollection)
64+
{
9365
for (auto &i : o->m_orign) {
9466
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
9567
origin->m_offset = i->m_offset;
@@ -98,11 +70,47 @@ class VariableValue {
9870
}
9971
}
10072

73+
74+
const std::string& getKey() const {
75+
return m_key;
76+
}
77+
78+
79+
const std::string& getKeyWithCollection() const {
80+
return m_keyWithCollection;
81+
}
82+
83+
84+
const std::string& getCollection() const {
85+
return m_collection;
86+
}
87+
88+
89+
const std::string& getValue() const {
90+
return m_value;
91+
}
92+
93+
94+
void setValue(const std::string &value) {
95+
m_value = value;
96+
}
97+
98+
99+
void addOrigin(std::unique_ptr<VariableOrigin> origin) {
100+
m_orign.push_back(std::move(origin));
101+
}
102+
103+
104+
const Origins& getOrigin() const {
105+
return m_orign;
106+
}
107+
108+
private:
109+
Origins m_orign;
110+
std::string m_collection;
101111
std::string m_key;
112+
std::string m_keyWithCollection;
102113
std::string m_value;
103-
std::string m_col;
104-
std::shared_ptr<std::string> m_keyWithCollection;
105-
std::list<std::unique_ptr<VariableOrigin>> m_orign;
106114
};
107115

108116
} // namespace modsecurity

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ VARIABLES = \
8888
variables/highest_severity.cc \
8989
variables/modsec_build.cc \
9090
variables/remote_user.cc \
91+
variables/rule.cc \
9192
variables/time.cc \
9293
variables/time_day.cc \
9394
variables/time_epoch.cc \

‎src/actions/set_var.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ bool SetVar::evaluate(Rule *rule, Transaction *t) {
117117
if (l.size() == 0) {
118118
value = 0;
119119
} else {
120-
value = stoi(l[0]->m_value);
120+
value = stoi(l[0]->getValue());
121121
for (auto &i : l) {
122122
delete i;
123123
}

‎src/anchored_set_variable.cc‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,13 @@ void AnchoredSetVariable::set(const std::string &key,
5353
const std::string &value, size_t offset, size_t len) {
5454
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
5555
std::string *v = new std::string(value);
56-
VariableValue *var = new VariableValue(std::make_shared<std::string>(
57-
m_name + ":" + key), v);
56+
VariableValue *var = new VariableValue(&m_name, &key, v);
5857
delete v;
5958

6059
origin->m_offset = offset;
6160
origin->m_length = len;
6261

63-
var->m_orign.push_back(std::move(origin));
62+
var->addOrigin(std::move(origin));
6463
emplace(key, var);
6564
}
6665

@@ -69,14 +68,13 @@ void AnchoredSetVariable::set(const std::string &key,
6968
const std::string &value, size_t offset) {
7069
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
7170
std::string *v = new std::string(value);
72-
VariableValue *var = new VariableValue(std::make_shared<std::string>(
73-
m_name + ":" + key), v);
71+
VariableValue *var = new VariableValue(&m_name, &key, v);
7472
delete v;
7573

7674
origin->m_offset = offset;
7775
origin->m_length = value.size();
7876

79-
var->m_orign.push_back(std::move(origin));
77+
var->addOrigin(std::move(origin));
8078
emplace(key, var);
8179
}
8280

@@ -117,7 +115,7 @@ std::unique_ptr<std::string> AnchoredSetVariable::resolveFirst(
117115
auto range = equal_range(key);
118116
for (auto it = range.first; it != range.second; ++it) {
119117
std::unique_ptr<std::string> b(new std::string());
120-
b->assign(it->second->m_value);
118+
b->assign(it->second->getValue());
121119
return b;
122120
}
123121
return nullptr;

‎src/anchored_variable.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void AnchoredVariable::set(const std::string &a, size_t offset,
6060
m_value.assign(a.c_str(), a.size());
6161
origin->m_offset = offset;
6262
origin->m_length = offsetLen;
63-
m_var->m_orign.push_back(std::move(origin));
63+
m_var->addOrigin(std::move(origin));
6464
}
6565

6666

@@ -71,7 +71,7 @@ void AnchoredVariable::set(const std::string &a, size_t offset) {
7171
m_value.assign(a.c_str(), a.size());
7272
origin->m_offset = offset;
7373
origin->m_length = m_value.size();
74-
m_var->m_orign.push_back(std::move(origin));
74+
m_var->addOrigin(std::move(origin));
7575
}
7676

7777

@@ -88,7 +88,7 @@ void AnchoredVariable::append(const std::string &a, size_t offset,
8888
m_offset = offset;
8989
origin->m_offset = offset;
9090
origin->m_length = a.size();
91-
m_var->m_orign.push_back(std::move(origin));
91+
m_var->addOrigin(std::move(origin));
9292
}
9393

9494

@@ -105,7 +105,7 @@ void AnchoredVariable::append(const std::string &a, size_t offset,
105105
m_offset = offset;
106106
origin->m_offset = offset;
107107
origin->m_length = size;
108-
m_var->m_orign.push_back(std::move(origin));
108+
m_var->addOrigin(std::move(origin));
109109
}
110110

111111

@@ -114,7 +114,7 @@ void AnchoredVariable::evaluate(std::vector<const VariableValue *> *l) {
114114
return;
115115
}
116116

117-
m_var->m_value.assign(m_value);
117+
m_var->setValue(m_value);
118118
VariableValue *m_var2 = new VariableValue(m_var);
119119
l->push_back(m_var2);
120120
}

‎src/engine/lua.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,11 +294,11 @@ int Lua::getvars(lua_State *L) {
294294
lua_newtable(L);
295295

296296
lua_pushstring(L, "name");
297-
lua_pushlstring(L, i->m_key.c_str(), i->m_key.size());
297+
lua_pushlstring(L, i->getKeyWithCollection().c_str(), i->getKeyWithCollection().size());
298298
lua_settable(L, -3);
299299

300300
lua_pushstring(L, "value");
301-
lua_pushlstring(L, i->m_value.c_str(), i->m_value.size());
301+
lua_pushlstring(L, i->getValue().c_str(), i->getValue().size());
302302
lua_settable(L, -3);
303303

304304
lua_settable(L, -3);

‎src/rule.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -696,25 +696,25 @@ bool Rule::evaluate(Transaction *trans,
696696
}
697697
var->evaluate(trans, this, &e);
698698
for (const VariableValue *v : e) {
699-
const std::string &value = v->m_value;
700-
const std::string &key = v->m_key;
699+
const std::string &value = v->getValue();
700+
const std::string &key = v->getKeyWithCollection();
701701

702-
if (exclusion.contains(v->m_key) ||
702+
if (exclusion.contains(v->getKeyWithCollection()) ||
703703
std::find_if(trans->m_ruleRemoveTargetById.begin(),
704704
trans->m_ruleRemoveTargetById.end(),
705705
[&, v, this](std::pair<int, std::string> &m) -> bool {
706-
return m.first == m_ruleId && m.second == v->m_key;
706+
return m.first == m_ruleId && m.second == v->getKeyWithCollection();
707707
}) != trans->m_ruleRemoveTargetById.end()
708708
) {
709709
delete v;
710710
v = NULL;
711711
continue;
712712
}
713-
if (exclusion.contains(v->m_key) ||
713+
if (exclusion.contains(v->getKeyWithCollection()) ||
714714
std::find_if(trans->m_ruleRemoveTargetByTag.begin(),
715715
trans->m_ruleRemoveTargetByTag.end(),
716716
[&, v, trans, this](std::pair<std::string, std::string> &m) -> bool {
717-
return containsTag(m.first, trans) && m.second == v->m_key;
717+
return containsTag(m.first, trans) && m.second == v->getKeyWithCollection();
718718
}) != trans->m_ruleRemoveTargetByTag.end()
719719
) {
720720
delete v;
@@ -736,7 +736,7 @@ bool Rule::evaluate(Transaction *trans,
736736
if (ret == true) {
737737
ruleMessage->m_match = m_op->resolveMatchMessage(trans,
738738
key, value);
739-
for (auto &i : v->m_orign) {
739+
for (auto &i : v->getOrigin()) {
740740
ruleMessage->m_reference.append(i->toText());
741741
}
742742

‎src/run_time_string.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ std::string RunTimeString::evaluate(Transaction *t, Rule *r) {
6060
std::vector<const VariableValue *> l;
6161
z->m_var->evaluate(t, r, &l);
6262
if (l.size() > 0) {
63-
s.append(l[0]->m_value);
63+
s.append(l[0]->getValue());
6464
}
6565
for (auto &i : l) {
6666
delete i;

‎src/transaction.cc‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -820,9 +820,7 @@ int Transaction::processRequestBody() {
820820
std::vector<const VariableValue *> l;
821821
m_variableRequestHeaders.resolve(&l);
822822
for (auto &a : l) {
823-
std::string z(a->m_key, 16, a->m_key.length() - 16);
824-
z = z + ": " + a->m_value;
825-
fullRequest = fullRequest + z + "\n";
823+
fullRequest = fullRequest + a->getKey() + ": " + a->getValue() + "\n";
826824
delete a;
827825
}
828826

@@ -1443,8 +1441,8 @@ std::string Transaction::toOldAuditLogFormat(int parts,
14431441
m_variableRequestHeaders.resolve(&l);
14441442
for (auto &h : l) {
14451443
size_t pos = strlen("REQUEST_HEADERS:");
1446-
audit_log << h->m_key.c_str() + pos << ": ";
1447-
audit_log << h->m_value.c_str() << std::endl;
1444+
audit_log << h->getKeyWithCollection().c_str() + pos << ": ";
1445+
audit_log << h->getValue().c_str() << std::endl;
14481446
delete h;
14491447
}
14501448
audit_log << std::endl;
@@ -1480,9 +1478,8 @@ std::string Transaction::toOldAuditLogFormat(int parts,
14801478
audit_log << this->m_httpCodeReturned << std::endl;
14811479
m_variableResponseHeaders.resolve(&l);
14821480
for (auto &h : l) {
1483-
size_t pos = strlen("RESPONSE_HEADERS:");
1484-
audit_log << h->m_key.c_str() + pos << ": ";
1485-
audit_log << h->m_value.c_str() << std::endl;
1481+
audit_log << h->getKey().c_str() << ": ";
1482+
audit_log << h->getValue().c_str() << std::endl;
14861483
delete h;
14871484
}
14881485
}
@@ -1580,8 +1577,7 @@ std::string Transaction::toJSON(int parts) {
15801577

15811578
m_variableRequestHeaders.resolve(&l);
15821579
for (auto &h : l) {
1583-
size_t pos = strlen("REQUEST_HEADERS:");
1584-
LOGFY_ADD(h->m_key.c_str() + pos, h->m_value.c_str());
1580+
LOGFY_ADD(h->getKey().c_str(), h->getValue().c_str());
15851581
delete h;
15861582
}
15871583

@@ -1611,8 +1607,7 @@ std::string Transaction::toJSON(int parts) {
16111607

16121608
m_variableResponseHeaders.resolve(&l);
16131609
for (auto &h : l) {
1614-
size_t pos = strlen("RESPONSE_HEADERS:");
1615-
LOGFY_ADD(h->m_key.c_str() + pos, h->m_value.c_str());
1610+
LOGFY_ADD(h->getKey().c_str(), h->getValue().c_str());
16161611
delete h;
16171612
}
16181613

‎src/variables/highest_severity.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ void HighestSeverity::evaluate(Transaction *transaction,
3131
std::vector<const VariableValue *> *l) {
3232
transaction->m_variableHighestSeverityAction.assign(
3333
std::to_string(transaction->m_highestSeverityAction));
34-
l->push_back(new VariableValue(m_fullName,
34+
l->push_back(new VariableValue(m_fullName.get(),
3535
&transaction->m_variableHighestSeverityAction));
3636
}
3737

0 commit comments

Comments
 (0)