Skip to content

Commit bbaffc8

Browse files
WGH-zimmerle
WGH-
authored and
zimmerle
committed
Make all "rule id" variables of type RuleId
Previously, ModSecurity inconsistently used RuleId, int and double for rule id variables in different places.
1 parent d023ddf commit bbaffc8

File tree

6 files changed

+62
-61
lines changed

6 files changed

+62
-61
lines changed

‎headers/modsecurity/rules_exceptions.h

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include <memory>
2929
#endif
3030

31+
#include "modsecurity/modsecurity.h"
32+
3133
#ifndef HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_
3234
#define HEADERS_MODSECURITY_RULES_EXCEPTIONS_H_
3335

@@ -51,9 +53,9 @@ class RulesExceptions {
5153
~RulesExceptions();
5254

5355
bool load(const std::string &data, std::string *error);
54-
bool addRange(int a, int b);
55-
bool addNumber(int a);
56-
bool contains(int a);
56+
bool addRange(RuleId a, RuleId b);
57+
bool addNumber(RuleId a);
58+
bool contains(RuleId a);
5759
bool merge(RulesExceptions *from);
5860

5961
bool loadRemoveRuleByMsg(const std::string &msg, std::string *error);
@@ -67,30 +69,30 @@ class RulesExceptions {
6769
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v,
6870
std::string *error);
6971

70-
bool loadUpdateTargetById(double id,
72+
bool loadUpdateTargetById(RuleId id,
7173
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > v,
7274
std::string *error);
7375

74-
bool loadUpdateActionById(double id,
76+
bool loadUpdateActionById(RuleId id,
7577
std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions,
7678
std::string *error);
7779

7880
std::unordered_multimap<std::shared_ptr<std::string>,
7981
std::shared_ptr<variables::Variable>> m_variable_update_target_by_tag;
8082
std::unordered_multimap<std::shared_ptr<std::string>,
8183
std::shared_ptr<variables::Variable>> m_variable_update_target_by_msg;
82-
std::unordered_multimap<double,
84+
std::unordered_multimap<RuleId,
8385
std::shared_ptr<variables::Variable>> m_variable_update_target_by_id;
84-
std::unordered_multimap<double,
86+
std::unordered_multimap<RuleId,
8587
std::shared_ptr<actions::transformations::Transformation>> m_action_transformation_update_target_by_id;
86-
std::unordered_multimap<double,
88+
std::unordered_multimap<RuleId,
8789
std::shared_ptr<actions::Action>> m_action_pos_update_target_by_id;
8890
std::list<std::string> m_remove_rule_by_msg;
8991
std::list<std::string> m_remove_rule_by_tag;
9092

9193
private:
92-
std::list<std::pair<int, int> > m_ranges;
93-
std::list<int> m_numbers;
94+
std::list<std::pair<RuleId, RuleId> > m_ranges;
95+
std::list<RuleId> m_numbers;
9496
};
9597

9698
} // namespace modsecurity

‎src/actions/rule_id.cc

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,16 @@ namespace actions {
2626
bool RuleId::init(std::string *error) {
2727
std::string a = m_parserPayload;
2828

29-
try {
30-
m_ruleId = std::stod(a);
31-
} catch (...) {
29+
std::istringstream iss(a);
30+
iss >> m_ruleId;
31+
if (iss.fail()) {
3232
m_ruleId = 0;
3333
error->assign("The input \"" + a + "\" does not " \
3434
"seems to be a valid rule id.");
3535
return false;
3636
}
3737

38-
std::ostringstream oss;
39-
oss << std::setprecision(40) << m_ruleId;
40-
if (a != oss.str() || m_ruleId < 0) {
38+
if (a != std::to_string(m_ruleId) || m_ruleId < 0) {
4139
error->assign("The input \"" + a + "\" does not seems " \
4240
"to be a valid rule id.");
4341
return false;

‎src/actions/rule_id.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class RuleId : public ActionTypeRuleMetaData {
4141
}
4242

4343
private:
44-
double m_ruleId;
44+
modsecurity::RuleId m_ruleId;
4545
};
4646

4747

‎src/parser/seclang-parser.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,10 +2880,10 @@ namespace yy {
28802880
#line 1480 "seclang-parser.yy"
28812881
{
28822882
std::string error;
2883-
double ruleId;
2884-
try {
2885-
ruleId = std::stod(yystack_[1].value.as < std::string > ());
2886-
} catch (...) {
2883+
std::istringstream iss(yystack_[1].value.as < std::string > ());
2884+
RuleId ruleId;
2885+
iss >> ruleId;
2886+
if (iss.fail()) {
28872887
std::stringstream ss;
28882888
ss << "SecRuleUpdateTargetById: failed to load:";
28892889
ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not ";
@@ -2910,10 +2910,10 @@ namespace yy {
29102910
#line 1506 "seclang-parser.yy"
29112911
{
29122912
std::string error;
2913-
double ruleId;
2914-
try {
2915-
ruleId = std::stod(yystack_[1].value.as < std::string > ());
2916-
} catch (...) {
2913+
std::istringstream iss(yystack_[1].value.as < std::string > ());
2914+
RuleId ruleId;
2915+
iss >> ruleId;
2916+
if (iss.fail()) {
29172917
std::stringstream ss;
29182918
ss << "SecRuleUpdateActionById: failed to load:";
29192919
ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not ";

‎src/parser/seclang-parser.yy

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,10 +1479,10 @@ expression:
14791479
| CONFIG_SEC_RULE_UPDATE_TARGET_BY_ID variables_pre_process
14801480
{
14811481
std::string error;
1482-
double ruleId;
1483-
try {
1484-
ruleId = std::stod($1);
1485-
} catch (...) {
1482+
std::istringstream iss($1);
1483+
RuleId ruleId;
1484+
iss >> ruleId;
1485+
if (iss.fail()) {
14861486
std::stringstream ss;
14871487
ss << "SecRuleUpdateTargetById: failed to load:";
14881488
ss << "The input \"" + $1 + "\" does not ";
@@ -1505,10 +1505,10 @@ expression:
15051505
| CONFIG_SEC_RULE_UPDATE_ACTION_BY_ID actions
15061506
{
15071507
std::string error;
1508-
double ruleId;
1509-
try {
1510-
ruleId = std::stod($1);
1511-
} catch (...) {
1508+
std::istringstream iss($1);
1509+
RuleId ruleId;
1510+
iss >> ruleId;
1511+
if (iss.fail()) {
15121512
std::stringstream ss;
15131513
ss << "SecRuleUpdateActionById: failed to load:";
15141514
ss << "The input \"" + $1 + "\" does not ";

‎src/rules_exceptions.cc

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ RulesExceptions::~RulesExceptions() {
3333
}
3434

3535

36-
bool RulesExceptions::loadUpdateActionById(double id,
36+
bool RulesExceptions::loadUpdateActionById(RuleId id,
3737
std::unique_ptr<std::vector<std::unique_ptr<actions::Action> > > actions,
3838
std::string *error) {
3939

@@ -48,14 +48,15 @@ bool RulesExceptions::loadUpdateActionById(double id,
4848
if (dynamic_cast<actions::transformations::Transformation *>(a.get())) {
4949
actions::transformations::Transformation *t = dynamic_cast<actions::transformations::Transformation *>(a.release());
5050
m_action_transformation_update_target_by_id.emplace(
51-
std::pair<double,
51+
std::pair<RuleId,
5252
std::shared_ptr<actions::transformations::Transformation>>(id, std::shared_ptr<actions::transformations::Transformation>(t))
5353
);
5454
continue;
55+
5556
}
5657

5758
m_action_pos_update_target_by_id.emplace(
58-
std::pair<double,
59+
std::pair<RuleId,
5960
std::unique_ptr<actions::Action>>(id , std::move(a))
6061
);
6162
}
@@ -111,13 +112,13 @@ bool RulesExceptions::loadUpdateTargetByTag(const std::string &tag,
111112
}
112113

113114

114-
bool RulesExceptions::loadUpdateTargetById(double id,
115+
bool RulesExceptions::loadUpdateTargetById(RuleId id,
115116
std::unique_ptr<std::vector<std::unique_ptr<variables::Variable> > > var,
116117
std::string *error) {
117118

118119
for (auto &i : *var) {
119120
m_variable_update_target_by_id.emplace(
120-
std::pair<double,
121+
std::pair<RuleId,
121122
std::unique_ptr<variables::Variable>>(id,
122123
std::move(i)));
123124
}
@@ -139,19 +140,18 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
139140
if (dash != std::string::npos) {
140141
std::string n1s = std::string(b, 0, dash);
141142
std::string n2s = std::string(b, dash + 1, b.size() - (dash + 1));
142-
int n1n = 0;
143-
int n2n = 0;
144-
try {
145-
n1n = std::stoi(n1s);
146-
added = true;
147-
} catch (...) {
143+
std::istringstream n1ss(n1s), n2ss(n2s);
144+
RuleId n1n = 0;
145+
RuleId n2n = 0;
146+
147+
n1ss >> n1n;
148+
if (n1ss.fail()) {
148149
error->assign("Not a number: " + n1s);
149150
return false;
150151
}
151-
try {
152-
n2n = std::stoi(n2s);
153-
added = true;
154-
} catch (...) {
152+
153+
n2ss >> n2n;
154+
if (n2ss.fail()) {
155155
error->assign("Not a number: " + n2s);
156156
return false;
157157
}
@@ -163,14 +163,15 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
163163
addRange(n1n, n2n);
164164
added = true;
165165
} else {
166-
try {
167-
int num = std::stoi(b);
168-
addNumber(num);
169-
added = true;
170-
} catch (...) {
166+
std::istringstream iss(b);
167+
RuleId num;
168+
iss >> num;
169+
if (iss.fail()) {
171170
error->assign("Not a number or range: " + b);
172171
return false;
173172
}
173+
addNumber(num);
174+
added = true;
174175
}
175176
}
176177

@@ -183,20 +184,20 @@ bool RulesExceptions::load(const std::string &a, std::string *error) {
183184
}
184185

185186

186-
bool RulesExceptions::addNumber(int a) {
187+
bool RulesExceptions::addNumber(RuleId a) {
187188
m_numbers.push_back(a);
188189
return true;
189190
}
190191

191192

192-
bool RulesExceptions::addRange(int a, int b) {
193+
bool RulesExceptions::addRange(RuleId a, RuleId b) {
193194
m_ranges.push_back(std::make_pair(a, b));
194195
return true;
195196
}
196197

197198

198-
bool RulesExceptions::contains(int a) {
199-
for (int z : m_numbers) {
199+
bool RulesExceptions::contains(RuleId a) {
200+
for (RuleId z : m_numbers) {
200201
if (a == z) {
201202
return true;
202203
}
@@ -213,7 +214,7 @@ bool RulesExceptions::contains(int a) {
213214

214215

215216
bool RulesExceptions::merge(RulesExceptions *from) {
216-
for (int a : from->m_numbers) {
217+
for (RuleId a : from->m_numbers) {
217218
bool ret = addNumber(a);
218219
if (ret == false) {
219220
return ret;
@@ -242,21 +243,21 @@ bool RulesExceptions::merge(RulesExceptions *from) {
242243

243244
for (auto &p : from->m_variable_update_target_by_id) {
244245
m_variable_update_target_by_id.emplace(
245-
std::pair<double,
246+
std::pair<RuleId,
246247
std::shared_ptr<variables::Variable>>(p.first,
247248
p.second));
248249
}
249250

250251
for (auto &p : from->m_action_pos_update_target_by_id) {
251252
m_action_pos_update_target_by_id.emplace(
252-
std::pair<double,
253+
std::pair<RuleId,
253254
std::shared_ptr<actions::Action>>(p.first,
254255
p.second));
255256
}
256257

257258
for (auto &p : from->m_action_transformation_update_target_by_id) {
258259
m_action_transformation_update_target_by_id.emplace(
259-
std::pair<double,
260+
std::pair<RuleId,
260261
std::shared_ptr<actions::transformations::Transformation>>(p.first,
261262
p.second));
262263
}

0 commit comments

Comments
 (0)