Skip to content

Commit 1e14d64

Browse files
WGH-zimmerle
authored andcommitted
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 d3512e5 commit 1e14d64

File tree

7 files changed

+67
-66
lines changed

7 files changed

+67
-66
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
@@ -2867,10 +2867,10 @@ namespace yy {
28672867
#line 1472 "seclang-parser.yy"
28682868
{
28692869
std::string error;
2870-
double ruleId;
2871-
try {
2872-
ruleId = std::stod(yystack_[1].value.as < std::string > ());
2873-
} catch (...) {
2870+
std::istringstream iss(yystack_[1].value.as < std::string > ());
2871+
RuleId ruleId;
2872+
iss >> ruleId;
2873+
if (iss.fail()) {
28742874
std::stringstream ss;
28752875
ss << "SecRuleUpdateTargetById: failed to load:";
28762876
ss << "The input \"" + yystack_[1].value.as < std::string > () + "\" does not ";
@@ -2897,10 +2897,10 @@ namespace yy {
28972897
#line 1498 "seclang-parser.yy"
28982898
{
28992899
std::string error;
2900-
double ruleId;
2901-
try {
2902-
ruleId = std::stod(yystack_[1].value.as < std::string > ());
2903-
} catch (...) {
2900+
std::istringstream iss(yystack_[1].value.as < std::string > ());
2901+
RuleId ruleId;
2902+
iss >> ruleId;
2903+
if (iss.fail()) {
29042904
std::stringstream ss;
29052905
ss << "SecRuleUpdateActionById: failed to load:";
29062906
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
@@ -1471,10 +1471,10 @@ expression:
14711471
| CONFIG_SEC_RULE_UPDATE_TARGET_BY_ID variables_pre_process
14721472
{
14731473
std::string error;
1474-
double ruleId;
1475-
try {
1476-
ruleId = std::stod($1);
1477-
} catch (...) {
1474+
std::istringstream iss($1);
1475+
RuleId ruleId;
1476+
iss >> ruleId;
1477+
if (iss.fail()) {
14781478
std::stringstream ss;
14791479
ss << "SecRuleUpdateTargetById: failed to load:";
14801480
ss << "The input \"" + $1 + "\" does not ";
@@ -1497,10 +1497,10 @@ expression:
14971497
| CONFIG_SEC_RULE_UPDATE_ACTION_BY_ID actions
14981498
{
14991499
std::string error;
1500-
double ruleId;
1501-
try {
1502-
ruleId = std::stod($1);
1503-
} catch (...) {
1500+
std::istringstream iss($1);
1501+
RuleId ruleId;
1502+
iss >> ruleId;
1503+
if (iss.fail()) {
15041504
std::stringstream ss;
15051505
ss << "SecRuleUpdateActionById: failed to load:";
15061506
ss << "The input \"" + $1 + "\" does not ";

src/parser/seclang-scanner.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11

2-
#line 2 "seclang-scanner.cc"
2+
#line 3 "seclang-scanner.cc"
33

44
#define YY_INT_ALIGNED short int
55

@@ -5150,15 +5150,15 @@ static std::stack<int> YY_PREVIOUS_STATE;
51505150
#define BEGIN_PREVIOUS() { BEGIN(YY_PREVIOUS_STATE.top()); YY_PREVIOUS_STATE.pop(); }
51515151

51525152
// The location of the current token.
5153-
#line 5153 "seclang-scanner.cc"
5153+
#line 5154 "seclang-scanner.cc"
51545154
#define YY_NO_INPUT 1
51555155

51565156
#line 491 "seclang-scanner.ll"
51575157
// Code run each time a pattern is matched.
51585158
# define YY_USER_ACTION driver.loc.back()->columns (yyleng);
51595159

5160-
#line 5160 "seclang-scanner.cc"
51615160
#line 5161 "seclang-scanner.cc"
5161+
#line 5162 "seclang-scanner.cc"
51625162

51635163
#define INITIAL 0
51645164
#define EXPECTING_ACTION_PREDICATE_VARIABLE 1
@@ -5480,7 +5480,7 @@ YY_DECL
54805480
// Code run each time yylex is called.
54815481
driver.loc.back()->step();
54825482

5483-
#line 5483 "seclang-scanner.cc"
5483+
#line 5484 "seclang-scanner.cc"
54845484

54855485
while ( /*CONSTCOND*/1 ) /* loops until end-of-file is reached */
54865486
{
@@ -8567,7 +8567,7 @@ YY_RULE_SETUP
85678567
#line 1330 "seclang-scanner.ll"
85688568
ECHO;
85698569
YY_BREAK
8570-
#line 8570 "seclang-scanner.cc"
8570+
#line 8571 "seclang-scanner.cc"
85718571

85728572
case YY_END_OF_BUFFER:
85738573
{

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.get());
5050
m_action_transformation_update_target_by_id.emplace(
51-
std::pair<double,
51+
std::pair<RuleId,
5252
std::unique_ptr<actions::transformations::Transformation>>(id, std::unique_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)