Skip to content

Commit 99ce977

Browse files
airweenairween
authored
Merge pull request #3253 from eduar-hte/rule-message
Simplified handling of RuleMessage by removing usage of std::shared_ptr
2 parents 9a1155c + 75d31a4 commit 99ce977

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

95 files changed

+432
-525
lines changed

‎build/win32/CMakeLists.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,13 @@ endfunction()
171171

172172
# unit tests
173173
file(GLOB unitTestSources ${BASE_DIR}/test/unit/*.cc)
174-
add_executable(unit_tests ${unitTestSources})
174+
add_executable(unit_tests ${unitTestSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
175175
setTestTargetProperties(unit_tests)
176176
target_compile_options(unit_tests PRIVATE /wd4805)
177177

178178
# regression tests
179179
file(GLOB regressionTestsSources ${BASE_DIR}/test/regression/*.cc)
180-
add_executable(regression_tests ${regressionTestsSources})
180+
add_executable(regression_tests ${regressionTestsSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
181181
setTestTargetProperties(regression_tests)
182182

183183
macro(add_regression_test_capability compile_definition flag)

‎examples/reading_logs_via_rule_message/reading_logs_via_rule_message.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,13 @@ class ReadingLogsViaRuleMessage {
160160
std::cout << std::endl;
161161
if (ruleMessage->m_isDisruptive) {
162162
std::cout << " * Disruptive action: ";
163-
std::cout << modsecurity::RuleMessage::log(ruleMessage);
163+
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
164164
std::cout << std::endl;
165165
std::cout << " ** %d is meant to be informed by the webserver.";
166166
std::cout << std::endl;
167167
} else {
168168
std::cout << " * Match, but no disruptive action: ";
169-
std::cout << modsecurity::RuleMessage::log(ruleMessage);
169+
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
170170
std::cout << std::endl;
171171
}
172172
}

‎examples/using_bodies_in_chunks/simple_request.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ static void logCb(void *data, const void *ruleMessagev) {
8181
std::cout << std::endl;
8282
if (ruleMessage->m_isDisruptive) {
8383
std::cout << " * Disruptive action: ";
84-
std::cout << modsecurity::RuleMessage::log(ruleMessage);
84+
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
8585
std::cout << std::endl;
8686
std::cout << " ** %d is meant to be informed by the webserver.";
8787
std::cout << std::endl;
8888
} else {
8989
std::cout << " * Match, but no disruptive action: ";
90-
std::cout << modsecurity::RuleMessage::log(ruleMessage);
90+
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
9191
std::cout << std::endl;
9292
}
9393
}

‎headers/modsecurity/actions/action.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Action {
8989

9090
virtual bool evaluate(RuleWithActions *rule, Transaction *transaction);
9191
virtual bool evaluate(RuleWithActions *rule, Transaction *transaction,
92-
std::shared_ptr<RuleMessage> ruleMessage) {
92+
RuleMessage &ruleMessage) {
9393
return evaluate(rule, transaction);
9494
}
9595
virtual bool init(std::string *error) { return true; }

‎headers/modsecurity/modsecurity.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ class ModSecurity {
292292
*/
293293
void setServerLogCb(ModSecLogCb cb, int properties);
294294

295-
void serverLog(void *data, std::shared_ptr<RuleMessage> rm);
295+
void serverLog(void *data, const RuleMessage &rm);
296296

297297
const std::string& getConnectorInformation() const;
298298

‎headers/modsecurity/rule.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ class Rule {
7878

7979
virtual bool evaluate(Transaction *transaction) = 0;
8080

81-
virtual bool evaluate(Transaction *transaction,
82-
std::shared_ptr<RuleMessage> rm) = 0;
81+
virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) = 0;
8382

8483
const std::string& getFileName() const {
8584
return m_fileName;

‎headers/modsecurity/rule_marker.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@ class RuleMarker : public Rule {
4242

4343
RuleMarker &operator=(const RuleMarker &r) = delete;
4444

45-
virtual bool evaluate(Transaction *transaction,
46-
std::shared_ptr<RuleMessage> rm) override {
45+
virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override {
4746
return evaluate(transaction);
4847
}
4948

‎headers/modsecurity/rule_message.h

+30-28
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,6 @@
1313
*
1414
*/
1515

16-
#ifdef __cplusplus
17-
#include <stack>
18-
#include <vector>
19-
#include <string>
20-
#include <list>
21-
#include <cstring>
22-
#endif
23-
2416
#ifndef HEADERS_MODSECURITY_RULE_MESSAGE_H_
2517
#define HEADERS_MODSECURITY_RULE_MESSAGE_H_
2618

@@ -31,8 +23,10 @@
3123

3224
#ifdef __cplusplus
3325

34-
namespace modsecurity {
26+
#include <string>
27+
#include <list>
3528

29+
namespace modsecurity {
3630

3731

3832
class RuleMessage {
@@ -45,43 +39,51 @@ class RuleMessage {
4539
RuleMessage(const RuleWithActions &rule, const Transaction &trans) :
4640
m_rule(rule),
4741
m_transaction(trans)
48-
{ }
42+
{
43+
reset(true);
44+
}
4945

5046
RuleMessage(const RuleMessage &ruleMessage) = default;
5147
RuleMessage &operator=(const RuleMessage &ruleMessage) = delete;
5248

53-
void clean() {
54-
m_data = "";
55-
m_match = "";
49+
void reset(const bool resetSaveMessage)
50+
{
51+
m_data.clear();
5652
m_isDisruptive = false;
57-
m_reference = "";
53+
m_match.clear();
54+
m_message.clear();
55+
m_noAuditLog = false;
56+
m_reference.clear();
57+
if (resetSaveMessage == true)
58+
m_saveMessage = true;
5859
m_severity = 0;
60+
m_tags.clear();
5961
}
6062

61-
std::string log() {
62-
return log(this, 0);
63+
std::string log() const {
64+
return log(*this, 0);
6365
}
64-
std::string log(int props) {
65-
return log(this, props);
66+
std::string log(int props) const {
67+
return log(*this, props);
6668
}
67-
std::string log(int props, int responseCode) {
68-
return log(this, props, responseCode);
69+
std::string log(int props, int responseCode) const {
70+
return log(*this, props, responseCode);
6971
}
70-
std::string errorLog() {
71-
return log(this,
72-
ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
72+
std::string errorLog() const {
73+
return log(*this,
74+
ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
7375
}
7476

75-
static std::string log(const RuleMessage *rm, int props, int code);
76-
static std::string log(const RuleMessage *rm, int props) {
77+
static std::string log(const RuleMessage &rm, int props, int code);
78+
static std::string log(const RuleMessage &rm, int props) {
7779
return log(rm, props, -1);
7880
}
79-
static std::string log(const RuleMessage *rm) {
81+
static std::string log(const RuleMessage &rm) {
8082
return log(rm, 0);
8183
}
8284

83-
static std::string _details(const RuleMessage *rm);
84-
static std::string _errorLogTail(const RuleMessage *rm);
85+
static std::string _details(const RuleMessage &rm);
86+
static std::string _errorLogTail(const RuleMessage &rm);
8587

8688
int getPhase() const { return m_rule.getPhase() - 1; }
8789

‎headers/modsecurity/rule_unconditional.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class RuleUnconditional : public RuleWithActions {
3636
public:
3737
using RuleWithActions::RuleWithActions;
3838

39-
virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
39+
virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
4040
};
4141

4242

‎headers/modsecurity/rule_with_actions.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,21 @@ class RuleWithActions : public Rule {
5151

5252
virtual bool evaluate(Transaction *transaction) override;
5353

54-
virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
54+
virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
5555

5656
void executeActionsIndependentOfChainedRuleResult(
5757
Transaction *trasn,
5858
bool *containsDisruptive,
59-
std::shared_ptr<RuleMessage> ruleMessage);
59+
RuleMessage &ruleMessage);
6060

6161
void executeActionsAfterFullMatch(
6262
Transaction *trasn,
6363
bool containsDisruptive,
64-
std::shared_ptr<RuleMessage> ruleMessage);
64+
RuleMessage &ruleMessage);
6565

6666
void executeAction(Transaction *trans,
6767
bool containsBlock,
68-
std::shared_ptr<RuleMessage> ruleMessage,
68+
RuleMessage &ruleMessage,
6969
actions::Action *a,
7070
bool context);
7171

@@ -74,7 +74,7 @@ class RuleWithActions : public Rule {
7474
const Transaction *trasn, const std::string &value, TransformationResults &ret);
7575

7676
void performLogging(Transaction *trans,
77-
std::shared_ptr<RuleMessage> ruleMessage,
77+
RuleMessage &ruleMessage,
7878
bool lastLog = true,
7979
bool chainedParentNull = false) const;
8080

‎headers/modsecurity/rule_with_operator.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,15 @@ class RuleWithOperator : public RuleWithActions {
4747

4848
~RuleWithOperator() override;
4949

50-
bool evaluate(Transaction *transaction,
51-
std::shared_ptr<RuleMessage> rm) override;
50+
bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;
5251

5352
void getVariablesExceptions(Transaction &t,
5453
variables::Variables *exclusion, variables::Variables *addition);
5554
inline void getFinalVars(variables::Variables *vars,
5655
variables::Variables *eclusion, Transaction *trans);
5756

5857
bool executeOperatorAt(Transaction *trasn, const std::string &key,
59-
const std::string &value, std::shared_ptr<RuleMessage> rm);
58+
const std::string &value, RuleMessage &ruleMessage);
6059

6160
static void updateMatchedVars(Transaction *trasn, const std::string &key,
6261
const std::string &value);

‎headers/modsecurity/transaction.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
407407
#ifndef NO_LOGS
408408
void debug(int, const std::string &) const; // cppcheck-suppress functionStatic
409409
#endif
410-
void serverLog(std::shared_ptr<RuleMessage> rm);
410+
void serverLog(const RuleMessage &rm);
411411

412412
int getRuleEngineState() const;
413413

‎src/actions/audit_log.cc

+3-4
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,10 @@ namespace modsecurity {
2727
namespace actions {
2828

2929

30-
bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction,
31-
std::shared_ptr<RuleMessage> rm) {
32-
rm->m_noAuditLog = false;
30+
bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
31+
ruleMessage.m_noAuditLog = false;
3332
ms_dbg_a(transaction, 9, "Saving transaction to logs");
34-
rm->m_saveMessage = true;
33+
ruleMessage.m_saveMessage = true;
3534

3635
return true;
3736
}

‎src/actions/audit_log.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ class AuditLog : public Action {
3535
explicit AuditLog(const std::string &action)
3636
: Action(action) { }
3737

38-
bool evaluate(RuleWithActions *rule, Transaction *transaction,
39-
std::shared_ptr<RuleMessage> rm) override;
38+
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
4039
};
4140

4241

‎src/actions/block.cc

+2-3
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,14 @@ namespace modsecurity {
2929
namespace actions {
3030

3131

32-
bool Block::evaluate(RuleWithActions *rule, Transaction *transaction,
33-
std::shared_ptr<RuleMessage> rm) {
32+
bool Block::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
3433
ms_dbg_a(transaction, 8, "Marking request as disruptive.");
3534

3635
for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
3736
if (a->isDisruptive() == false) {
3837
continue;
3938
}
40-
a->evaluate(rule, transaction, rm);
39+
a->evaluate(rule, transaction, ruleMessage);
4140
}
4241

4342
return true;

‎src/actions/block.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ class Block : public Action {
3535
public:
3636
explicit Block(const std::string &action) : Action(action) { }
3737

38-
bool evaluate(RuleWithActions *rule, Transaction *transaction,
39-
std::shared_ptr<RuleMessage> rm) override;
38+
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
4039
};
4140

4241

‎src/actions/data/status.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ bool Status::init(std::string *error) {
3939

4040

4141
bool Status::evaluate(RuleWithActions *rule, Transaction *transaction,
42-
std::shared_ptr<RuleMessage> rm) {
42+
RuleMessage &ruleMessage) {
4343
transaction->m_it.status = m_status;
4444
return true;
4545
}

‎src/actions/data/status.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ class Status : public Action {
3737
: Action(action), m_status(0) { }
3838

3939
bool init(std::string *error) override;
40-
bool evaluate(RuleWithActions *rule, Transaction *transaction,
41-
std::shared_ptr<RuleMessage> rm) override;
40+
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
4241

4342
int m_status;
4443
};

‎src/actions/disruptive/deny.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace disruptive {
2929

3030

3131
bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
32-
std::shared_ptr<RuleMessage> rm) {
32+
RuleMessage &ruleMessage) {
3333
ms_dbg_a(transaction, 8, "Running action deny");
3434

3535
if (transaction->m_it.status == 200) {
@@ -38,9 +38,9 @@ bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
3838

3939
transaction->m_it.disruptive = true;
4040
intervention::freeLog(&transaction->m_it);
41-
rm->m_isDisruptive = true;
41+
ruleMessage.m_isDisruptive = true;
4242
transaction->m_it.log = strdup(
43-
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
43+
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
4444

4545
return true;
4646
}

‎src/actions/disruptive/deny.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ class Deny : public Action {
3333
public:
3434
explicit Deny(const std::string &action) : Action(action) { }
3535

36-
bool evaluate(RuleWithActions *rule, Transaction *transaction,
37-
std::shared_ptr<RuleMessage> rm) override;
36+
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
3837
bool isDisruptive() override { return true; }
3938
};
4039

‎src/actions/disruptive/drop.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace disruptive {
3333

3434

3535
bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
36-
std::shared_ptr<RuleMessage> rm) {
36+
RuleMessage &ruleMessage) {
3737
ms_dbg_a(transaction, 8, "Running action drop " \
3838
"[executing deny instead of drop.]");
3939

@@ -43,9 +43,9 @@ bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
4343

4444
transaction->m_it.disruptive = true;
4545
intervention::freeLog(&transaction->m_it);
46-
rm->m_isDisruptive = true;
46+
ruleMessage.m_isDisruptive = true;
4747
transaction->m_it.log = strdup(
48-
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
48+
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
4949

5050
return true;
5151
}

‎src/actions/disruptive/drop.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ class Drop : public Action {
3232
public:
3333
explicit Drop(const std::string &action) : Action(action) { }
3434

35-
bool evaluate(RuleWithActions *rule, Transaction *transaction,
36-
std::shared_ptr<RuleMessage> rm) override;
35+
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
3736
bool isDisruptive() override { return true; }
3837
};
3938

‎src/actions/disruptive/pass.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ namespace disruptive {
3030

3131

3232
bool Pass::evaluate(RuleWithActions *rule, Transaction *transaction,
33-
std::shared_ptr<RuleMessage> rm) {
33+
RuleMessage &ruleMessage) {
3434
intervention::free(&transaction->m_it);
3535
intervention::reset(&transaction->m_it);
3636

0 commit comments

Comments
 (0)