Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplified handling of RuleMessage by removing usage of std::shared_ptr #3253

Merged
merged 4 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build/win32/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,13 @@ endfunction()

# unit tests
file(GLOB unitTestSources ${BASE_DIR}/test/unit/*.cc)
add_executable(unit_tests ${unitTestSources})
add_executable(unit_tests ${unitTestSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
setTestTargetProperties(unit_tests)
target_compile_options(unit_tests PRIVATE /wd4805)

# regression tests
file(GLOB regressionTestsSources ${BASE_DIR}/test/regression/*.cc)
add_executable(regression_tests ${regressionTestsSources})
add_executable(regression_tests ${regressionTestsSources} ${BASE_DIR}/test/common/custom_debug_log.cc)
setTestTargetProperties(regression_tests)

macro(add_regression_test_capability compile_definition flag)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,13 @@ class ReadingLogsViaRuleMessage {
std::cout << std::endl;
if (ruleMessage->m_isDisruptive) {
std::cout << " * Disruptive action: ";
std::cout << modsecurity::RuleMessage::log(ruleMessage);
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
std::cout << std::endl;
std::cout << " ** %d is meant to be informed by the webserver.";
std::cout << std::endl;
} else {
std::cout << " * Match, but no disruptive action: ";
std::cout << modsecurity::RuleMessage::log(ruleMessage);
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
std::cout << std::endl;
}
}
Expand Down
4 changes: 2 additions & 2 deletions examples/using_bodies_in_chunks/simple_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ static void logCb(void *data, const void *ruleMessagev) {
std::cout << std::endl;
if (ruleMessage->m_isDisruptive) {
std::cout << " * Disruptive action: ";
std::cout << modsecurity::RuleMessage::log(ruleMessage);
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
std::cout << std::endl;
std::cout << " ** %d is meant to be informed by the webserver.";
std::cout << std::endl;
} else {
std::cout << " * Match, but no disruptive action: ";
std::cout << modsecurity::RuleMessage::log(ruleMessage);
std::cout << modsecurity::RuleMessage::log(*ruleMessage);
std::cout << std::endl;
}
}
Expand Down
2 changes: 1 addition & 1 deletion headers/modsecurity/actions/action.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class Action {

virtual bool evaluate(RuleWithActions *rule, Transaction *transaction);
virtual bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> ruleMessage) {
RuleMessage &ruleMessage) {
return evaluate(rule, transaction);
}
virtual bool init(std::string *error) { return true; }
Expand Down
2 changes: 1 addition & 1 deletion headers/modsecurity/modsecurity.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ class ModSecurity {
*/
void setServerLogCb(ModSecLogCb cb, int properties);

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

const std::string& getConnectorInformation() const;

Expand Down
3 changes: 1 addition & 2 deletions headers/modsecurity/rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ class Rule {

virtual bool evaluate(Transaction *transaction) = 0;

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

const std::string& getFileName() const {
return m_fileName;
Expand Down
3 changes: 1 addition & 2 deletions headers/modsecurity/rule_marker.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ class RuleMarker : public Rule {

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

virtual bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override {
virtual bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override {
return evaluate(transaction);
}

Expand Down
58 changes: 30 additions & 28 deletions headers/modsecurity/rule_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@
*
*/

#ifdef __cplusplus
#include <stack>
#include <vector>
#include <string>
#include <list>
#include <cstring>
#endif

#ifndef HEADERS_MODSECURITY_RULE_MESSAGE_H_
#define HEADERS_MODSECURITY_RULE_MESSAGE_H_

Expand All @@ -31,8 +23,10 @@

#ifdef __cplusplus

namespace modsecurity {
#include <string>
#include <list>

namespace modsecurity {


class RuleMessage {
Expand All @@ -45,43 +39,51 @@ class RuleMessage {
RuleMessage(const RuleWithActions &rule, const Transaction &trans) :
m_rule(rule),
m_transaction(trans)
{ }
{
reset(true);
}

RuleMessage(const RuleMessage &ruleMessage) = default;
RuleMessage &operator=(const RuleMessage &ruleMessage) = delete;

void clean() {
m_data = "";
m_match = "";
void reset(const bool resetSaveMessage)
{
m_data.clear();
m_isDisruptive = false;
m_reference = "";
m_match.clear();
m_message.clear();
m_noAuditLog = false;
m_reference.clear();
if (resetSaveMessage == true)
m_saveMessage = true;
m_severity = 0;
m_tags.clear();
}

std::string log() {
return log(this, 0);
std::string log() const {
return log(*this, 0);
}
std::string log(int props) {
return log(this, props);
std::string log(int props) const {
return log(*this, props);
}
std::string log(int props, int responseCode) {
return log(this, props, responseCode);
std::string log(int props, int responseCode) const {
return log(*this, props, responseCode);
}
std::string errorLog() {
return log(this,
ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
std::string errorLog() const {
return log(*this,
ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
}

static std::string log(const RuleMessage *rm, int props, int code);
static std::string log(const RuleMessage *rm, int props) {
static std::string log(const RuleMessage &rm, int props, int code);
static std::string log(const RuleMessage &rm, int props) {
return log(rm, props, -1);
}
static std::string log(const RuleMessage *rm) {
static std::string log(const RuleMessage &rm) {
return log(rm, 0);
}

static std::string _details(const RuleMessage *rm);
static std::string _errorLogTail(const RuleMessage *rm);
static std::string _details(const RuleMessage &rm);
static std::string _errorLogTail(const RuleMessage &rm);

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

Expand Down
2 changes: 1 addition & 1 deletion headers/modsecurity/rule_unconditional.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class RuleUnconditional : public RuleWithActions {
public:
using RuleWithActions::RuleWithActions;

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


Expand Down
10 changes: 5 additions & 5 deletions headers/modsecurity/rule_with_actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,21 @@ class RuleWithActions : public Rule {

virtual bool evaluate(Transaction *transaction) override;

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

void executeActionsIndependentOfChainedRuleResult(
Transaction *trasn,
bool *containsDisruptive,
std::shared_ptr<RuleMessage> ruleMessage);
RuleMessage &ruleMessage);

void executeActionsAfterFullMatch(
Transaction *trasn,
bool containsDisruptive,
std::shared_ptr<RuleMessage> ruleMessage);
RuleMessage &ruleMessage);

void executeAction(Transaction *trans,
bool containsBlock,
std::shared_ptr<RuleMessage> ruleMessage,
RuleMessage &ruleMessage,
actions::Action *a,
bool context);

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

void performLogging(Transaction *trans,
std::shared_ptr<RuleMessage> ruleMessage,
RuleMessage &ruleMessage,
bool lastLog = true,
bool chainedParentNull = false) const;

Expand Down
5 changes: 2 additions & 3 deletions headers/modsecurity/rule_with_operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,15 @@ class RuleWithOperator : public RuleWithActions {

~RuleWithOperator() override;

bool evaluate(Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(Transaction *transaction, RuleMessage &ruleMessage) override;

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

bool executeOperatorAt(Transaction *trasn, const std::string &key,
const std::string &value, std::shared_ptr<RuleMessage> rm);
const std::string &value, RuleMessage &ruleMessage);

static void updateMatchedVars(Transaction *trasn, const std::string &key,
const std::string &value);
Expand Down
2 changes: 1 addition & 1 deletion headers/modsecurity/transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
#ifndef NO_LOGS
void debug(int, const std::string &) const; // cppcheck-suppress functionStatic
#endif
void serverLog(std::shared_ptr<RuleMessage> rm);
void serverLog(const RuleMessage &rm);

int getRuleEngineState() const;

Expand Down
7 changes: 3 additions & 4 deletions src/actions/audit_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@ namespace modsecurity {
namespace actions {


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

return true;
}
Expand Down
3 changes: 1 addition & 2 deletions src/actions/audit_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class AuditLog : public Action {
explicit AuditLog(const std::string &action)
: Action(action) { }

bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};


Expand Down
5 changes: 2 additions & 3 deletions src/actions/block.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ namespace modsecurity {
namespace actions {


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

for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
if (a->isDisruptive() == false) {
continue;
}
a->evaluate(rule, transaction, rm);
a->evaluate(rule, transaction, ruleMessage);
}

return true;
Expand Down
3 changes: 1 addition & 2 deletions src/actions/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class Block : public Action {
public:
explicit Block(const std::string &action) : Action(action) { }

bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};


Expand Down
2 changes: 1 addition & 1 deletion src/actions/data/status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ bool Status::init(std::string *error) {


bool Status::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
transaction->m_it.status = m_status;
return true;
}
Expand Down
3 changes: 1 addition & 2 deletions src/actions/data/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class Status : public Action {
: Action(action), m_status(0) { }

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

int m_status;
};
Expand Down
6 changes: 3 additions & 3 deletions src/actions/disruptive/deny.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace disruptive {


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

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

transaction->m_it.disruptive = true;
intervention::freeLog(&transaction->m_it);
rm->m_isDisruptive = true;
ruleMessage.m_isDisruptive = true;
transaction->m_it.log = strdup(
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());

return true;
}
Expand Down
3 changes: 1 addition & 2 deletions src/actions/disruptive/deny.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class Deny : public Action {
public:
explicit Deny(const std::string &action) : Action(action) { }

bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool isDisruptive() override { return true; }
};

Expand Down
6 changes: 3 additions & 3 deletions src/actions/disruptive/drop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace disruptive {


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

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

transaction->m_it.disruptive = true;
intervention::freeLog(&transaction->m_it);
rm->m_isDisruptive = true;
ruleMessage.m_isDisruptive = true;
transaction->m_it.log = strdup(
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());

return true;
}
Expand Down
3 changes: 1 addition & 2 deletions src/actions/disruptive/drop.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ class Drop : public Action {
public:
explicit Drop(const std::string &action) : Action(action) { }

bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool isDisruptive() override { return true; }
};

Expand Down
2 changes: 1 addition & 1 deletion src/actions/disruptive/pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace disruptive {


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

Expand Down
Loading
Loading