Remove unnecessary heap allocated copies when doing transformations.

- Updated transformations to work inplace on the std::string buffer of
  the value where the transformation is being applied, instead of
  copying to a temporary string.
- Make inplace transformations methods inline and moved them out of the
  class interface as almost all of them are not used outside of them.
- 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).
- 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.
- Simplified & shared implementation of several transformations,
  removing duplicate code.

Author: eduar-hte

headers/modsecurity/actions/action.h

@@ -74,8 +74,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) {

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

@@ -248,11 +248,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/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/transformations/base64_decode.cc

@@ -13,33 +13,19 @@
  *
  */
 
-#include "src/actions/transformations/base64_decode.h"
+#include "base64_decode.h"
 
-#include <iostream>
-#include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
-
-#include "modsecurity/transaction.h"
-#include "src/actions/transformations/transformation.h"
 #include "src/utils/base64.h"
 
 
-namespace modsecurity {
-namespace actions {
-namespace transformations {
-
+namespace modsecurity::actions::transformations {
 
-std::string Base64Decode::evaluate(const std::string &value,
-    Transaction *transaction) {
-    std::string ret = Utils::Base64::decode(value);
 
-    return ret;
+bool Base64Decode::transform(std::string &value, const Transaction *trans) const {
+    if (value.empty()) return false;
+    value = Utils::Base64::decode(value);
+    return true;
 }
 
 
-}  // namespace transformations
-}  // namespace actions
-}  // namespace modsecurity
+}  // namespace modsecurity::actions::transformations

src/actions/transformations/base64_decode.h

@@ -13,33 +13,21 @@
  *
  */
 
-#include <string>
-
-#include "modsecurity/actions/action.h"
-#include "src/actions/transformations/transformation.h"
-
 #ifndef SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_H_
 #define SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_H_
 
-#ifdef __cplusplus
-namespace modsecurity {
-class Transaction;
+#include "transformation.h"
 
-namespace actions {
-namespace transformations {
+namespace modsecurity::actions::transformations {
 
 class Base64Decode : public Transformation {
  public:
-    explicit Base64Decode(const std::string &action)  : Transformation(action) { }
+    explicit Base64Decode(const std::string &action)
+        : Transformation(action) { }
 
-    std::string evaluate(const std::string &exp,
-        Transaction *transaction) override;
+    bool transform(std::string &value, const Transaction *trans) const override;
 };
 
-}  // namespace transformations
-}  // namespace actions
-}  // namespace modsecurity
-
-#endif
+}  // namespace modsecurity::actions::transformations
 
 #endif  // SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_H_

src/actions/transformations/base64_decode_ext.cc

@@ -13,33 +13,19 @@
  *
  */
 
-#include "src/actions/transformations/base64_decode_ext.h"
+#include "base64_decode_ext.h"
 
-#include <iostream>
-#include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
-
-#include "modsecurity/transaction.h"
-#include "src/actions/transformations/transformation.h"
 #include "src/utils/base64.h"
 
 
-namespace modsecurity {
-namespace actions {
-namespace transformations {
-
+namespace modsecurity::actions::transformations {
 
-std::string Base64DecodeExt::evaluate(const std::string &value,
-    Transaction *transaction) {
-    std::string ret = Utils::Base64::decode_forgiven(value);
 
-    return ret;
+bool Base64DecodeExt::transform(std::string &value, const Transaction *trans) const {
+    if (value.empty()) return false;
+    value = Utils::Base64::decode_forgiven(value);
+    return true;
 }
 
 
-}  // namespace transformations
-}  // namespace actions
-}  // namespace modsecurity
+}  // namespace modsecurity::actions::transformations

src/actions/transformations/base64_decode_ext.h

@@ -13,33 +13,21 @@
  *
  */
 
-#include <string>
-
-#include "modsecurity/actions/action.h"
-#include "src/actions/transformations/transformation.h"
-
 #ifndef SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_EXT_H_
 #define SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_EXT_H_
 
-#ifdef __cplusplus
-namespace modsecurity {
-class Transaction;
+#include "transformation.h"
 
-namespace actions {
-namespace transformations {
+namespace modsecurity::actions::transformations {
 
 class Base64DecodeExt : public Transformation {
  public:
-    explicit Base64DecodeExt(const std::string &action)  : Transformation(action) { }
+    explicit Base64DecodeExt(const std::string &action)
+        : Transformation(action) { }
 
-    std::string evaluate(const std::string &exp,
-        Transaction *transaction) override;
+    bool transform(std::string &value, const Transaction *trans) const override;
 };
 
-}  // namespace transformations
-}  // namespace actions
-}  // namespace modsecurity
-
-#endif
+}  // namespace modsecurity::actions::transformations
 
 #endif  // SRC_ACTIONS_TRANSFORMATIONS_BASE64_DECODE_EXT_H_

src/actions/transformations/base64_encode.cc

@@ -13,33 +13,19 @@
  *
  */
 
-#include "src/actions/transformations/base64_encode.h"
+#include "base64_encode.h"
 
-#include <iostream>
-#include <string>
-#include <algorithm>
-#include <functional>
-#include <cctype>
-#include <locale>
-
-#include "modsecurity/transaction.h"
-#include "src/actions/transformations/transformation.h"
 #include "src/utils/base64.h"
 
 
-namespace modsecurity {
-namespace actions {
-namespace transformations {
-
+namespace modsecurity::actions::transformations {
 
-std::string Base64Encode::evaluate(const std::string &value,
-    Transaction *transaction) {
-    std::string ret = Utils::Base64::encode(value);
 
-    return ret;
+bool Base64Encode::transform(std::string &value, const Transaction *trans) const {
+    if (value.empty()) return false;
+    value = Utils::Base64::encode(value);
+    return true;
 }
 
 
-}  // namespace transformations
-}  // namespace actions
-}  // namespace modsecurity
+}  // namespace modsecurity::actions::transformations