Merge pull request #3231 from eduar-hte/remove-copies-transformations

Remove unnecessary heap allocated copies in Transformation actions

Author: airween

headers/modsecurity/actions/action.h

@@ -13,41 +13,66 @@
  *
  */
 
-#ifdef __cplusplus
-
-#include <string>
-#include <iostream>
-#include <memory>
-
-#endif
-
-#include "modsecurity/intervention.h"
-#include "modsecurity/rule.h"
-#include "modsecurity/rule_with_actions.h"
-
 #ifndef HEADERS_MODSECURITY_ACTIONS_ACTION_H_
 #define HEADERS_MODSECURITY_ACTIONS_ACTION_H_
 
 #ifdef __cplusplus
 
+#include <string>
+#include <memory>
+
 namespace modsecurity {
 class Transaction;
 class RuleWithOperator;
+class RuleWithActions;
+class RuleMessage;
 
 namespace actions {
 
 
 class Action {
  public:
+    /**
+     *
+     * Define the action kind regarding to the execution time.
+     * 
+     * 
+     */
+    enum class Kind {
+    /**
+     *
+     * Action that are executed while loading the configuration. For instance
+     * the rule ID or the rule phase.
+     *
+     */
+     ConfigurationKind,
+    /**
+     *
+     * Those are actions that demands to be executed before call the operator.
+     * For instance the tranformations.
+     *
+     *
+     */
+     RunTimeBeforeMatchAttemptKind,
+    /**
+     *
+     * Actions that are executed after the execution of the operator, only if
+     * the operator returned Match (or True). For instance the disruptive
+     * actions.
+     *
+     */
+     RunTimeOnlyIfMatchKind,
+    };
+
     explicit Action(const std::string& _action)
         : m_isNone(false),
         temporaryAction(false),
-        action_kind(2),
+        action_kind(Kind::RunTimeOnlyIfMatchKind),
         m_name(nullptr),
         m_parser_payload("") {
             set_name_and_payload(_action);
         }
-    explicit Action(const std::string& _action, int kind)
+    explicit Action(const std::string& _action, Kind kind)
         : m_isNone(false),
         temporaryAction(false),
         action_kind(kind),
@@ -74,8 +99,6 @@ class Action {
 
     virtual ~Action() { }
 
-    virtual std::string evaluate(const std::string &exp,
-        Transaction *transaction);
     virtual bool evaluate(RuleWithActions *rule, Transaction *transaction);
     virtual bool evaluate(RuleWithActions *rule, Transaction *transaction,
         std::shared_ptr<RuleMessage> ruleMessage) {
@@ -87,9 +110,9 @@ class Action {
 
     void set_name_and_payload(const std::string& data) {
         size_t pos = data.find(":");
-        std::string t = "t:";
+        const char t[] = "t:";
 
-        if (data.compare(0, t.length(), t) == 0) {
+        if (data.compare(0, std::size(t) - 1, t) == 0) {
             pos = data.find(":", 2);
         }
 
@@ -109,41 +132,9 @@ class Action {
 
     bool m_isNone;
     bool temporaryAction;
-    int action_kind;
+    Kind action_kind;
     std::shared_ptr<std::string> m_name;
     std::string m_parser_payload;
-
-    /**
-     *
-     * Define the action kind regarding to the execution time.
-     * 
-     * 
-     */
-    enum Kind {
-    /**
-     *
-     * Action that are executed while loading the configuration. For instance
-     * the rule ID or the rule phase.
-     *
-     */
-     ConfigurationKind,
-    /**
-     *
-     * Those are actions that demands to be executed before call the operator.
-     * For instance the tranformations.
-     *
-     *
-     */
-     RunTimeBeforeMatchAttemptKind,
-    /**
-     *
-     * Actions that are executed after the execution of the operator, only if
-     * the operator returned Match (or True). For instance the disruptive
-     * actions.
-     *
-     */
-     RunTimeOnlyIfMatchKind,
-    };
  };
 
 

headers/modsecurity/rule.h

@@ -52,7 +52,7 @@ namespace operators {
 class Operator;
 }
 
-using TransformationResult = std::pair<std::shared_ptr<std::string>,
+using TransformationResult = std::pair<std::string,
     std::shared_ptr<std::string>>;
 using TransformationResults = std::list<TransformationResult>;
 

headers/modsecurity/rule_with_actions.h

@@ -119,16 +119,7 @@ class RuleWithActions : public Rule {
 
 
     void executeTransformations(
-        Transaction *trasn, const std::string &value, TransformationResults &ret);
-
-    inline void executeTransformation(
-        actions::transformations::Transformation *a,
-        std::shared_ptr<std::string> *value,
-        Transaction *trans,
-        TransformationResults *ret,
-        std::string *path,
-        int *nth) const;
-
+        const Transaction *trasn, const std::string &value, TransformationResults &ret);
 
     void performLogging(Transaction *trans,
         std::shared_ptr<RuleMessage> ruleMessage,
@@ -166,6 +157,14 @@ class RuleWithActions : public Rule {
     RuleWithActions *m_chainedRuleParent;
 
  private:
+    inline void executeTransformation(
+        const actions::transformations::Transformation &a,
+        std::string &value,
+        const Transaction *trans,
+        TransformationResults &ret,
+        std::string &path,
+        int &nth) const;
+
     /* actions */
     actions::Action *m_disruptiveAction;
     actions::LogData *m_logData;

src/Makefile.am

@@ -247,11 +247,9 @@ UTILS = \
 	utils/geo_lookup.cc \
 	utils/https_client.cc \
 	utils/ip_tree.cc \
-	utils/md5.cc \
 	utils/msc_tree.cc \
 	utils/random.cc \
 	utils/regex.cc \
-	utils/sha1.cc \
 	utils/system.cc \
 	utils/shared_files.cc
 

src/actions/accuracy.cc

@@ -15,16 +15,10 @@
 
 #include "src/actions/accuracy.h"
 
-#include <iostream>
-#include <string>
+#include "modsecurity/rule_with_actions.h"
 
-#include "modsecurity/actions/action.h"
-#include "modsecurity/transaction.h"
-#include "modsecurity/rule.h"
 
-
-namespace modsecurity {
-namespace actions {
+namespace modsecurity::actions {
 
 
 bool Accuracy::init(std::string *error) {
@@ -45,5 +39,4 @@ bool Accuracy::evaluate(RuleWithActions *rule, Transaction *transaction) {
 }
 
 
-}  // namespace actions
-}  // namespace modsecurity
+}  // namespace modsecurity::actions

src/actions/accuracy.h

@@ -30,7 +30,7 @@ namespace actions {
 class Accuracy : public Action {
  public:
     explicit Accuracy(const std::string &action) 
-        : Action(action, ConfigurationKind),
+        : Action(action, Kind::ConfigurationKind),
         m_accuracy(0) { }
 
     bool evaluate(RuleWithActions *rule, Transaction *transaction) override;

src/actions/action.cc

@@ -45,12 +45,6 @@ namespace modsecurity {
 namespace actions {
 
 
-std::string Action::evaluate(const std::string &value,
-    Transaction *transaction) {
-    return value;
-}
-
-
 bool Action::evaluate(RuleWithActions *rule, Transaction *transaction) {
     return true;
 }

src/actions/audit_log.h

@@ -33,7 +33,7 @@ namespace actions {
 class AuditLog : public Action {
  public:
     explicit AuditLog(const std::string &action) 
-        : Action(action, RunTimeOnlyIfMatchKind) { }
+        : Action(action) { }
 
     bool evaluate(RuleWithActions *rule, Transaction *transaction,
         std::shared_ptr<RuleMessage> rm) override;

src/actions/capture.h

@@ -29,7 +29,7 @@ namespace actions {
 class Capture : public Action {
  public:
     explicit Capture(const std::string &action) 
-        : Action(action, RunTimeOnlyIfMatchKind) { }
+        : Action(action) { }
 
     bool evaluate(RuleWithActions *rule, Transaction *transaction) override;
 };

src/actions/chain.cc

@@ -15,14 +15,9 @@
 
 #include "src/actions/chain.h"
 
-#include <iostream>
-#include <string>
+#include "modsecurity/rule_with_actions.h"
 
-#include "modsecurity/transaction.h"
-#include "modsecurity/rule.h"
-
-namespace modsecurity {
-namespace actions {
+namespace modsecurity::actions {
 
 
 bool Chain::evaluate(RuleWithActions *rule, Transaction *transaction) {
@@ -31,5 +26,4 @@ bool Chain::evaluate(RuleWithActions *rule, Transaction *transaction) {
 }
 
 
-}  // namespace actions
-}  // namespace modsecurity
+}  // namespace modsecurity::actions