From df51dd06ef9bd074a6d0fdc7b33d0e656a9f35a0 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 2b458768..f42991df 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 91dbc408..9a2d876f 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 9a5093ca..e6f5bffc 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 74d1d757..757c1d12 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 11acac54..db876304 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 fdc043a3..a31e5ffe 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 34840062..87a30059 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 450e699e..907e8874 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 44b7fb0f..182b2162 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 f5e3ff0f..848c05f9 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 7c0e21b6..2ff7d1f1 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 c336b1f4..232d3087 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 b005b063..b729ee21 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 e098ac94..e2a75c0d 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 a8bba959..6ffd495d 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 4907cf45..8619a8e7 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 9d2ebb06..7c90e761 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 70c52368..d4a52e43 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 6e458d89..175d780c 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 f4d7dc75..aae342b3 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 ecdc499b..3aecdb42 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 ed6b3bd7..fd857177 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 3fc167a8..1adcdd1b 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 684c1eb6..910ac761 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 644fd8d1..df07060b 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 30becd9b..cc0cd809 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 adf2d0e6..ed373859 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 f8be619f..2bf0d46d 100644 --- a/src/actions/tag.cc +++ b/src/actions/tag.cc @@ -56,13 +56,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 5c7eda3e..c976ebc0 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 39e394a7..f85ae451 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; }