From f7ab746c3b3fedc5dd77efa380c1cbd38e65123a Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Fri, 15 Mar 2019 01:35:07 -0300 Subject: [PATCH] Removes RuleMessage from action execute signature --- headers/modsecurity/actions/action.h | 4 ---- src/actions/audit_log.cc | 3 +-- src/actions/audit_log.h | 3 +-- src/actions/block.cc | 3 +-- src/actions/block.h | 3 +-- src/actions/data/status.cc | 3 +-- src/actions/data/status.h | 3 +-- src/actions/disruptive/deny.cc | 7 +++---- src/actions/disruptive/deny.h | 3 +-- src/actions/disruptive/drop.cc | 7 +++---- src/actions/disruptive/drop.h | 3 +-- src/actions/disruptive/pass.cc | 3 +-- src/actions/disruptive/pass.h | 3 +-- src/actions/disruptive/redirect.cc | 7 +++---- src/actions/disruptive/redirect.h | 3 +-- src/actions/log.cc | 3 +-- src/actions/log.h | 3 +-- src/actions/log_data.cc | 5 ++--- src/actions/log_data.h | 3 +-- src/actions/msg.cc | 5 ++--- src/actions/msg.h | 3 +-- src/actions/no_audit_log.cc | 3 +-- src/actions/no_audit_log.h | 3 +-- src/actions/no_log.cc | 3 +-- src/actions/no_log.h | 3 +-- src/actions/severity.cc | 3 +-- src/actions/severity.h | 3 +-- src/actions/tag.cc | 6 ++---- src/actions/tag.h | 3 +-- src/rule_with_actions.cc | 18 ++++++++---------- 30 files changed, 45 insertions(+), 80 deletions(-) diff --git a/headers/modsecurity/actions/action.h b/headers/modsecurity/actions/action.h index 2bde9fb7..7f9da3a9 100644 --- a/headers/modsecurity/actions/action.h +++ b/headers/modsecurity/actions/action.h @@ -78,10 +78,6 @@ class Action { virtual std::string execute(const std::string &exp, Transaction *transaction); virtual bool execute(RuleWithActions *rule, Transaction *transaction); - virtual bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &ruleMessage) { - return execute(rule, transaction); - } /** * This method is meant to be used by transformations — a particular diff --git a/src/actions/audit_log.cc b/src/actions/audit_log.cc index 3d2ae2a6..6562bcbe 100644 --- a/src/actions/audit_log.cc +++ b/src/actions/audit_log.cc @@ -27,8 +27,7 @@ namespace modsecurity { namespace actions { -bool AuditLog::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool AuditLog::execute(RuleWithActions *rule, Transaction *transaction) { transaction->messageSetNoAuditLog(false); return true; } diff --git a/src/actions/audit_log.h b/src/actions/audit_log.h index 9cbd704c..56026b8a 100644 --- a/src/actions/audit_log.h +++ b/src/actions/audit_log.h @@ -35,8 +35,7 @@ class AuditLog : public Action { explicit AuditLog(const std::string &action) : Action(action, RunTimeOnlyIfMatchKind) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; }; diff --git a/src/actions/block.cc b/src/actions/block.cc index 01b6c37f..62c6c153 100644 --- a/src/actions/block.cc +++ b/src/actions/block.cc @@ -29,8 +29,7 @@ namespace modsecurity { namespace actions { -bool Block::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Block::execute(RuleWithActions *rule, Transaction *transaction) { ms_dbg_a(transaction, 8, "Marking request as disruptive."); return true; } diff --git a/src/actions/block.h b/src/actions/block.h index 2417b3b7..154e8508 100644 --- a/src/actions/block.h +++ b/src/actions/block.h @@ -35,8 +35,7 @@ class Block : public Action { public: explicit Block(const std::string &action) : Action(action) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; }; diff --git a/src/actions/data/status.cc b/src/actions/data/status.cc index 9d6d0b8c..0a81a670 100644 --- a/src/actions/data/status.cc +++ b/src/actions/data/status.cc @@ -38,8 +38,7 @@ bool Status::init(std::string *error) { } -bool Status::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Status::execute(RuleWithActions *rule, Transaction *transaction) { transaction->m_it.status = m_status; return true; } diff --git a/src/actions/data/status.h b/src/actions/data/status.h index 4634e4b0..5aa294e1 100644 --- a/src/actions/data/status.h +++ b/src/actions/data/status.h @@ -37,8 +37,7 @@ class Status : public Action { m_status(0) { } bool init(std::string *error) override; - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; int m_status; }; diff --git a/src/actions/disruptive/deny.cc b/src/actions/disruptive/deny.cc index 1ce9b947..3a9ade8e 100644 --- a/src/actions/disruptive/deny.cc +++ b/src/actions/disruptive/deny.cc @@ -28,8 +28,7 @@ namespace actions { namespace disruptive { -bool Deny::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Deny::execute(RuleWithActions *rule, Transaction *transaction) { ms_dbg_a(transaction, 8, "Running action deny"); if (transaction->m_it.status == 200) { @@ -38,9 +37,9 @@ bool Deny::execute(RuleWithActions *rule, Transaction *transaction, transaction->m_it.disruptive = true; intervention::freeLog(&transaction->m_it); - rm.setRule(rule); + transaction->messageGetLast()->setRule(rule); transaction->m_it.log = strdup( - rm.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); + transaction->messageGetLast()->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); return true; } diff --git a/src/actions/disruptive/deny.h b/src/actions/disruptive/deny.h index db8c5e0a..28ba74c0 100644 --- a/src/actions/disruptive/deny.h +++ b/src/actions/disruptive/deny.h @@ -33,8 +33,7 @@ class Deny : public Action { public: explicit Deny(const std::string &action) : Action(action) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; bool isDisruptive() override { return true; } }; diff --git a/src/actions/disruptive/drop.cc b/src/actions/disruptive/drop.cc index a53cfa5b..fe6b94b5 100644 --- a/src/actions/disruptive/drop.cc +++ b/src/actions/disruptive/drop.cc @@ -32,8 +32,7 @@ namespace actions { namespace disruptive { -bool Drop::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Drop::execute(RuleWithActions *rule, Transaction *transaction) { ms_dbg_a(transaction, 8, "Running action drop " \ "[executing deny instead of drop.]"); @@ -43,9 +42,9 @@ bool Drop::execute(RuleWithActions *rule, Transaction *transaction, transaction->m_it.disruptive = true; intervention::freeLog(&transaction->m_it); - rm.setRule(rule); + transaction->messageGetLast()->setRule(rule); transaction->m_it.log = strdup( - rm.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); + transaction->messageGetLast()->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); return true; } diff --git a/src/actions/disruptive/drop.h b/src/actions/disruptive/drop.h index d25952a5..71394bca 100644 --- a/src/actions/disruptive/drop.h +++ b/src/actions/disruptive/drop.h @@ -32,8 +32,7 @@ class Drop : public Action { public: explicit Drop(const std::string &action) : Action(action) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; bool isDisruptive() override { return true; } }; diff --git a/src/actions/disruptive/pass.cc b/src/actions/disruptive/pass.cc index fd26f52a..fc683384 100644 --- a/src/actions/disruptive/pass.cc +++ b/src/actions/disruptive/pass.cc @@ -29,8 +29,7 @@ namespace actions { namespace disruptive { -bool Pass::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Pass::execute(RuleWithActions *rule, Transaction *transaction) { intervention::free(&transaction->m_it); intervention::reset(&transaction->m_it); diff --git a/src/actions/disruptive/pass.h b/src/actions/disruptive/pass.h index ba1f4d09..a21900a5 100644 --- a/src/actions/disruptive/pass.h +++ b/src/actions/disruptive/pass.h @@ -31,8 +31,7 @@ class Pass : public Action { public: explicit Pass(const std::string &action) : Action(action) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; bool isDisruptive() override { return true; } }; diff --git a/src/actions/disruptive/redirect.cc b/src/actions/disruptive/redirect.cc index 4d1f901b..ff464060 100644 --- a/src/actions/disruptive/redirect.cc +++ b/src/actions/disruptive/redirect.cc @@ -34,8 +34,7 @@ bool Redirect::init(std::string *error) { } -bool Redirect::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Redirect::execute(RuleWithActions *rule, Transaction *transaction) { std::string m_urlExpanded(m_string->evaluate(transaction)); /* if it was changed before, lets keep it. */ if (transaction->m_it.status == 200 @@ -47,9 +46,9 @@ bool Redirect::execute(RuleWithActions *rule, Transaction *transaction, transaction->m_it.url = strdup(m_urlExpanded.c_str()); transaction->m_it.disruptive = true; intervention::freeLog(&transaction->m_it); - rm.setRule(rule); + transaction->messageGetLast()->setRule(rule); transaction->m_it.log = strdup( - rm.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); + transaction->messageGetLast()->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str()); return true; } diff --git a/src/actions/disruptive/redirect.h b/src/actions/disruptive/redirect.h index a7719b99..2a1574a5 100644 --- a/src/actions/disruptive/redirect.h +++ b/src/actions/disruptive/redirect.h @@ -46,8 +46,7 @@ class Redirect : public Action { m_status(0), m_string(std::move(z)) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; bool init(std::string *error) override; bool isDisruptive() override { return true; } diff --git a/src/actions/log.cc b/src/actions/log.cc index 84a788d2..87dc252a 100644 --- a/src/actions/log.cc +++ b/src/actions/log.cc @@ -28,8 +28,7 @@ namespace modsecurity { namespace actions { -bool Log::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Log::execute(RuleWithActions *rule, Transaction *transaction) { return true; } diff --git a/src/actions/log.h b/src/actions/log.h index edbe0b77..d8c34838 100644 --- a/src/actions/log.h +++ b/src/actions/log.h @@ -33,8 +33,7 @@ class Log : public Action { explicit Log(const std::string &action) : Action(action, RunTimeOnlyIfMatchKind) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; }; } // namespace actions diff --git a/src/actions/log_data.cc b/src/actions/log_data.cc index 6e1dc8f2..9af821d4 100644 --- a/src/actions/log_data.cc +++ b/src/actions/log_data.cc @@ -29,9 +29,8 @@ namespace modsecurity { namespace actions { -bool LogData::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { - rm.m_data = data(transaction); +bool LogData::execute(RuleWithActions *rule, Transaction *transaction) { + transaction->messageGetLast()->m_data = data(transaction); return true; } diff --git a/src/actions/log_data.h b/src/actions/log_data.h index d5bb6cbd..2c29cd46 100644 --- a/src/actions/log_data.h +++ b/src/actions/log_data.h @@ -39,8 +39,7 @@ class LogData : public Action { : Action("logdata", RunTimeOnlyIfMatchKind), m_string(std::move(z)) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; std::string data(Transaction *Transaction); diff --git a/src/actions/msg.cc b/src/actions/msg.cc index b0cd98de..5a005f41 100644 --- a/src/actions/msg.cc +++ b/src/actions/msg.cc @@ -46,10 +46,9 @@ namespace modsecurity { namespace actions { -bool Msg::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Msg::execute(RuleWithActions *rule, Transaction *transaction) { std::string msg = data(transaction); - rm.m_message = msg; + transaction->messageGetLast()->m_message = msg; ms_dbg_a(transaction, 9, "Saving msg: " + msg); return true; diff --git a/src/actions/msg.h b/src/actions/msg.h index a2e200ed..a2044e05 100644 --- a/src/actions/msg.h +++ b/src/actions/msg.h @@ -40,8 +40,7 @@ class Msg : public Action { : Action("msg", RunTimeOnlyIfMatchKind), m_string(std::move(z)) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; std::string data(Transaction *Transaction); std::shared_ptr m_string; diff --git a/src/actions/no_audit_log.cc b/src/actions/no_audit_log.cc index 0e295ff8..8283059b 100644 --- a/src/actions/no_audit_log.cc +++ b/src/actions/no_audit_log.cc @@ -26,8 +26,7 @@ namespace modsecurity { namespace actions { -bool NoAuditLog::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool NoAuditLog::execute(RuleWithActions *rule, Transaction *transaction) { transaction->messageSetNoAuditLog(true); return true; } diff --git a/src/actions/no_audit_log.h b/src/actions/no_audit_log.h index 4b01dc74..22491171 100644 --- a/src/actions/no_audit_log.h +++ b/src/actions/no_audit_log.h @@ -35,8 +35,7 @@ class NoAuditLog : public Action { explicit NoAuditLog(const std::string &action) : Action(action, RunTimeOnlyIfMatchKind) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; }; } // namespace actions diff --git a/src/actions/no_log.cc b/src/actions/no_log.cc index c01a696f..4fadc190 100644 --- a/src/actions/no_log.cc +++ b/src/actions/no_log.cc @@ -29,8 +29,7 @@ namespace modsecurity { namespace actions { -bool NoLog::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool NoLog::execute(RuleWithActions *rule, Transaction *transaction) { return true; } diff --git a/src/actions/no_log.h b/src/actions/no_log.h index e6bc673c..bb42a4f6 100644 --- a/src/actions/no_log.h +++ b/src/actions/no_log.h @@ -33,8 +33,7 @@ class NoLog : public Action { explicit NoLog(const std::string &action) : Action(action, RunTimeOnlyIfMatchKind) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; }; } // namespace actions diff --git a/src/actions/severity.cc b/src/actions/severity.cc index 297573c8..14af0f91 100644 --- a/src/actions/severity.cc +++ b/src/actions/severity.cc @@ -71,8 +71,7 @@ bool Severity::init(std::string *error) { } -bool Severity::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Severity::execute(RuleWithActions *rule, Transaction *transaction) { return true; } diff --git a/src/actions/severity.h b/src/actions/severity.h index c9c656cb..a07193e8 100644 --- a/src/actions/severity.h +++ b/src/actions/severity.h @@ -35,8 +35,7 @@ class Severity : public Action { : Action(action), m_severity(0) { } - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; bool init(std::string *error) override; int m_severity; diff --git a/src/actions/tag.cc b/src/actions/tag.cc index e0012808..454ffea6 100644 --- a/src/actions/tag.cc +++ b/src/actions/tag.cc @@ -57,13 +57,11 @@ std::string Tag::getName(Transaction *transaction) { } -bool Tag::execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) { +bool Tag::execute(RuleWithActions *rule, Transaction *transaction) { std::string tag = getName(transaction); ms_dbg_a(transaction, 9, "Rule tag: " + tag); - rm.m_tags.push_back(tag); - + transaction->messageGetLast()->m_tags.push_back(tag); return true; } diff --git a/src/actions/tag.h b/src/actions/tag.h index e0af1012..799b948d 100644 --- a/src/actions/tag.h +++ b/src/actions/tag.h @@ -38,8 +38,7 @@ class Tag : public Action { std::string getName(Transaction *transaction); - bool execute(RuleWithActions *rule, Transaction *transaction, - RuleMessage &rm) override; + bool execute(RuleWithActions *rule, Transaction *transaction) override; protected: std::shared_ptr m_string; diff --git a/src/rule_with_actions.cc b/src/rule_with_actions.cc index a43d219a..23c0bd79 100644 --- a/src/rule_with_actions.cc +++ b/src/rule_with_actions.cc @@ -258,7 +258,7 @@ void RuleWithActions::executeActionsIndependentOfChainedRuleResult(Transaction * } else if (*a->m_name.get() == "setvar") { ms_dbg_a(trans, 4, "Running [independent] (non-disruptive) " \ "action: " + *a->m_name.get()); - a->execute(this, trans, *trans->messageGetLast()); + a->execute(this, trans); } } } @@ -282,7 +282,7 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans) { for (actions::Tag *a : getTagsActionPtr()) { ms_dbg_a(trans, 4, "Running (non-disruptive) action: " \ + *a->m_name.get()); - a->execute(this, trans, *trans->messageGetLast()); + a->execute(this, trans); } /** @@ -301,18 +301,17 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans) { } if (m_logData) { - m_logData->execute(this, trans, *trans->messageGetLast()); + m_logData->execute(this, trans); } else if (m_defaultActionLogData) { - m_defaultActionLogData->execute(this, trans, *trans->messageGetLast()); + m_defaultActionLogData->execute(this, trans); } if (m_msg) { - m_msg->execute(this, trans, *trans->messageGetLast()); + m_msg->execute(this, trans); } else if (m_defaultActionMsg) { - m_defaultActionMsg->execute(this, trans, *trans->messageGetLast()); + m_defaultActionMsg->execute(this, trans); } - for (auto &a : getMatchActionsPtr()) { if (!a->isDisruptive() && !(disruptiveAlreadyExecuted @@ -334,11 +333,10 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans) { void RuleWithActions::executeAction(Transaction *trans, Action *a, bool defaultContext) { - if (a->isDisruptive() == false && *a->m_name.get() != "block") { ms_dbg_a(trans, 9, "Running " \ "action: " + *a->m_name.get()); - a->execute(this, trans, *trans->messageGetLast()); + a->execute(this, trans); return; } @@ -351,7 +349,7 @@ void RuleWithActions::executeAction(Transaction *trans, if (trans->getRuleEngineState() == RulesSet::EnabledRuleEngine) { ms_dbg_a(trans, 4, "Running (disruptive) action: " + *a->m_name.get() + "."); - a->execute(this, trans, *trans->messageGetLast()); + a->execute(this, trans); return; }