From b66224853b4e9d30e0a44d16b29d5ed3842a6b11 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Fri, 15 Feb 2019 21:49:46 -0300 Subject: [PATCH] Refactoring in Rule: Meaningful structures name --- headers/modsecurity/rule.h | 34 ++++++++++++++++------------------ src/parser/seclang-parser.cc | 2 +- src/rule.cc | 33 +++++++++++++++------------------ 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/headers/modsecurity/rule.h b/headers/modsecurity/rule.h index a5ca9f92..7543e46f 100644 --- a/headers/modsecurity/rule.h +++ b/headers/modsecurity/rule.h @@ -44,11 +44,18 @@ class Msg; class Rev; class SetVar; class Tag; +namespace transformations { +class Transformation; +} } namespace operators { class Operator; } +using TransformationResult = std::pair, + std::shared_ptr>; +using TransformationResults = std::list; + class Rule { public: Rule(operators::Operator *_op, @@ -68,13 +75,6 @@ class Rule { bool containsBlock, std::shared_ptr ruleMessage, actions::Action *a, bool context); - inline void executeTransformation(actions::Action *a, - std::shared_ptr *value, - Transaction *trans, - std::list, - std::shared_ptr>> *ret, - std::string *path, - int *nth) const; void getVariablesExceptions(Transaction *t, variables::Variables *exclusion, variables::Variables *addition); @@ -83,10 +83,6 @@ class Rule { void executeActionsAfterFullMatch(Transaction *trasn, bool containsDisruptive, std::shared_ptr ruleMessage); - std::list, - std::shared_ptr>> executeDefaultTransformations( - Transaction *trasn, const std::string &value); - bool executeOperatorAt(Transaction *trasn, const std::string &key, std::string value, std::shared_ptr rm); void executeActionsIndependentOfChainedRuleResult(Transaction *trasn, @@ -100,15 +96,17 @@ class Rule { bool containsTag(const std::string& name, Transaction *t); bool containsMsg(const std::string& name, Transaction *t); + void executeTransformations( - actions::Action *a, - std::shared_ptr newValue, - std::shared_ptr value, + Transaction *trasn, const std::string &value, TransformationResults &ret); + inline void executeTransformation( + actions::transformations::Transformation *a, + std::shared_ptr *value, Transaction *trans, - std::list, - std::shared_ptr>> *ret, - std::shared_ptr transStr, - int nth); + TransformationResults *ret, + std::string *path, + int *nth) const; + actions::Action *m_theDisruptiveAction; actions::LogData *m_logData; diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc index aecd5d14..e9580c76 100644 --- a/src/parser/seclang-parser.cc +++ b/src/parser/seclang-parser.cc @@ -2425,7 +2425,7 @@ namespace yy { } checkedActions.push_back(a); } else { - driver.error(yystack_[2].location, "The action '" + a->m_name + "' is not suitable to be part of the SecDefaultActions"); + driver.error(yystack_[2].location, "The action '" + *a->m_name.get() + "' is not suitable to be part of the SecDefaultActions"); YYERROR; } } diff --git a/src/rule.cc b/src/rule.cc index 53719f02..809b35aa 100644 --- a/src/rule.cc +++ b/src/rule.cc @@ -49,7 +49,7 @@ using operators::Operator; using actions::Action; using variables::Variable; using actions::transformations::None; - +using actions::transformations::Transformation; Rule::Rule(const std::string &marker) : m_theDisruptiveAction(nullptr), @@ -326,11 +326,11 @@ bool Rule::executeOperatorAt(Transaction *trans, const std::string &key, } -inline void Rule::executeTransformation(actions::Action *a, +inline void Rule::executeTransformation( + actions::transformations::Transformation *a, std::shared_ptr *value, Transaction *trans, - std::list, - std::shared_ptr>> *ret, + TransformationResults *ret, std::string *path, int *nth) const { @@ -359,15 +359,11 @@ inline void Rule::executeTransformation(actions::Action *a, } -std::list, - std::shared_ptr>> - Rule::executeDefaultTransformations( - Transaction *trans, const std::string &in) { +void Rule::executeTransformations( + Transaction *trans, const std::string &in, TransformationResults &ret) { int none = 0; int transformations = 0; std::string path(""); - std::list, - std::shared_ptr>> ret; std::shared_ptr value = std::shared_ptr(new std::string(in)); @@ -394,14 +390,17 @@ std::list, continue; } - executeTransformation(a.get(), &value, trans, &ret, &path, + // FIXME: here the object needs to be a transformation already. + Transformation *t = dynamic_cast(a.get()); + executeTransformation(t, &value, trans, &ret, &path, &transformations); } } for (Action *a : this->m_actionsRuntimePre) { if (none == 0) { - executeTransformation(a, &value, trans, &ret, &path, + Transformation *t = dynamic_cast(a); + executeTransformation(t, &value, trans, &ret, &path, &transformations); } if (a->m_isNone) { @@ -427,7 +426,8 @@ std::list, } actions::Action *a = dynamic_cast(b.second.get()); if (none == 0) { - executeTransformation(a, &value, trans, &ret, &path, + Transformation *t = dynamic_cast(a); + executeTransformation(t, &value, trans, &ret, &path, &transformations); } if (a->m_isNone) { @@ -446,8 +446,6 @@ std::list, std::shared_ptr(new std::string(*value)), std::shared_ptr(new std::string(path)))); } - - return ret; } @@ -711,10 +709,9 @@ bool Rule::evaluate(Transaction *trans, continue; } - std::list, - std::shared_ptr>> values; + TransformationResults values; - values = executeDefaultTransformations(trans, value); + executeTransformations(trans, value, values); for (const auto &valueTemp : values) { bool ret;