From 35e825d6435dbef2c2de0daf6acc9faecbba5983 Mon Sep 17 00:00:00 2001 From: gberkes Date: Wed, 7 Aug 2024 17:55:30 +0200 Subject: [PATCH 1/2] Refactor: replaced 3 declarations with 3 structured binding declarations. This syntax is far more expressive and easier to understand than the old one. Refactor: flipped the conditions in "if (contains[Tag|Msg|Id]( ... " statements for clearer code. Refactor: moved "Variable *b" as an init-statement inside "if()" statements for stricter scope. Reference: https://sonarcloud.io/project/issues?open=AY8-ff1vm_fzkWiCOtCt&id=owasp-modsecurity_ModSecurity --- src/rule_with_operator.cc | 54 +++++++++++++++------------------------ 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index 3a5ff385..0eb5ea30 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -133,45 +133,33 @@ bool RuleWithOperator::executeOperatorAt(Transaction *trans, const std::string & void RuleWithOperator::getVariablesExceptions(Transaction *t, variables::Variables *exclusion, variables::Variables *addition) { - for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer - if (containsTag(*a.first.get(), t) == false) { - continue; - } - Variable *b = a.second.get(); - if (dynamic_cast(b)) { - exclusion->push_back( - dynamic_cast( - b)->m_base.get()); - } else { - addition->push_back(b); + for (const auto &[tag, v] : t->m_rules->m_exceptions.m_variable_update_target_by_tag) { // cppcheck-suppress ctunullpointer + if (containsTag(*tag.get(), t)) { + if (Variable *b{v.get()};dynamic_cast(b)) { + exclusion->push_back(dynamic_cast(b)->m_base.get()); + } else { + addition->push_back(b); + } } } - for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_msg) { - if (containsMsg(*a.first.get(), t) == false) { - continue; - } - Variable *b = a.second.get(); - if (dynamic_cast(b)) { - exclusion->push_back( - dynamic_cast( - b)->m_base.get()); - } else { - addition->push_back(b); + for (const auto &[msg, v] : t->m_rules->m_exceptions.m_variable_update_target_by_msg) { + if (containsMsg(*msg.get(), t)) { + if (Variable *b{v.get()}; dynamic_cast(b)) { + exclusion->push_back(dynamic_cast(b)->m_base.get()); + } else { + addition->push_back(b); + } } } - for (const auto &a : t->m_rules->m_exceptions.m_variable_update_target_by_id) { - if (m_ruleId != a.first) { - continue; - } - Variable *b = a.second.get(); - if (dynamic_cast(b)) { - exclusion->push_back( - dynamic_cast( - b)->m_base.get()); - } else { - addition->push_back(b); + for (const auto &[id, v] : t->m_rules->m_exceptions.m_variable_update_target_by_id) { + if (m_ruleId == id) { + if (Variable *b{v.get()};dynamic_cast(b)) { + exclusion->push_back(dynamic_cast(b)->m_base.get()); + } else { + addition->push_back(b); + } } } } From c50a397a87f79f076a778b3228957d7063eb444e Mon Sep 17 00:00:00 2001 From: gberkes Date: Wed, 7 Aug 2024 21:05:47 +0200 Subject: [PATCH 2/2] Suppress cppcheck false positive unassignedVariable warning. --- src/rule_with_operator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc index 0eb5ea30..d63129da 100644 --- a/src/rule_with_operator.cc +++ b/src/rule_with_operator.cc @@ -153,7 +153,7 @@ void RuleWithOperator::getVariablesExceptions(Transaction *t, } } - for (const auto &[id, v] : t->m_rules->m_exceptions.m_variable_update_target_by_id) { + for (const auto &[id, v] : t->m_rules->m_exceptions.m_variable_update_target_by_id) { // cppcheck-suppress unassignedVariable if (m_ruleId == id) { if (Variable *b{v.get()};dynamic_cast(b)) { exclusion->push_back(dynamic_cast(b)->m_base.get());