- Because the lifetime of the RuleMessage instances do not extend beyond
the lifetime of the enclosing RuleWithActions & Transaction,
RuleMessage can just reference it and simplify its definition.
- Additionally, make the references const to show that it doesn't modify it.
- Replace RuleMessage copy constructor with default implementations.
- Removed unused RuleMessage assignment operator (which cannot be implemented
now that it has reference members).
- Removed constructor from RuleMessage pointer.
- Addressed Sonarcloud suggestions: Do not use the constructor's
initializer list for data member "xxx". Use the in-class initializer
instead.
- Some of the Transformation classes would initialize their Action's
action_kind using the default (using Transformation constructor
without an action_kind parameter).
- Others, however, would use that constructor and initialize action_kind
manually in their constructor, but setting the default value
(RunTimeBeforeMatchAttemptKind = 1), which was redundant.
- Removed unused Transformation constructor to specify action_kind.
- Converted Action::Kind into an 'enum class' to require using the enum
constants (instead of integer values, which are difficult to track in
the codebase and change)
- This function already expects these arguments not to be null pointers,
doesn't validate them and just dereference them.
- In order to make this explicit and enforced by the compiler, they're
now passed as references.
- Renamed Transformation::evaluate to Transformation::transform to avoid
confusion with Action's overload methods.
- Updated Transformation::transform signature to receive the value by
reference and perform the transformation inline, if possible.
- Some transformations still need to use a temporary std::string to
perform their work, and then copy the result back.
- Made Transformation::transform methods const and updated Transaction
parameter to be const.
- Transaction parameter could not be removed because it's used by just
a single transformation, UrlDecodeUni.
- Removed std::string Action::evaluate(const std::string &exp,
Transaction *transaction); which was only implemented by
Transformation but was not used from the base class, but only after
downcasting to Transformation, so it can just be declared there (and
not pollute other actions with a default member implementation -that
does nothing- which is never called).
- SonarCloud analysis identified standalone `throw;` calls without accompanying `try-catch` blocks, used inconsistently as placeholders or for premature termination under specific conditions.
- Removed these `throw;` instances to prevent potential runtime issues in future development phases, where such configurations might inadvertently be created.
- Introduced `assert` statements as a more appropriate mechanism for asserting preconditions in the affected class member functions, ensuring clearer intent and safer code behavior during development.
- Refactor action_kind processing to use switch() instead of if-else chains; add assertion in default case.
- Fix SonarCloud issue: Make this variable a const reference.
https://sonarcloud.io/project/issues?resolved=false&pullRequest=3104&id=owasp-modsecurity_ModSecurity&open=AY8Vpgy4f6U6E7VKL4Cn
- RuleWithActions::evaluate(Transaction *transaction)
- Removed temporary rm local variable used to immediately create
std::shared_ptr<RuleMessage>.
- Leverage std::make_shared & auto to simplify code.
Meta-actions can only be used in non-chained rules or in the chain starter
rule of a rule chain. The m_chainedRuleParent member of the RuleWithActions
class is NULL only if the rule is not chained or if it is the chain starter
rule of a rule chain.
Fixes#2867.
