Merge pull request #3248 from eduar-hte/simplified-constructors

Simplified constructors, copy constructors & assignment operators

headers/modsecurity/actions/action.h

@@ -81,21 +81,9 @@ class Action {
             set_name_and_payload(_action);
         }
 
-    Action(const Action &a)
-        : m_isNone(a.m_isNone),
-        temporaryAction(a.temporaryAction),
-        action_kind(a.action_kind),
-        m_name(a.m_name),
-        m_parser_payload(a.m_parser_payload) { }
+    Action(const Action &a) = delete;
 
-    Action &operator=(const Action& a) {
-        m_isNone = a.m_isNone;
-        temporaryAction = a.temporaryAction;
-        action_kind = a.action_kind;
-        m_name = a.m_name;
-        m_parser_payload = a.m_parser_payload;
-        return *this;
-    }
+    Action &operator=(const Action& a) = delete;
 
     virtual ~Action() { }
 

headers/modsecurity/rule.h

@@ -13,15 +13,6 @@
  *
  */
 
-#ifdef __cplusplus
-#include <stack>
-#include <vector>
-#include <string>
-#include <list>
-#include <memory>
-#include <utility>
-#endif
-
 #ifndef HEADERS_MODSECURITY_RULE_H_
 #define HEADERS_MODSECURITY_RULE_H_
 
@@ -31,6 +22,12 @@
 
 #ifdef __cplusplus
 
+#include <vector>
+#include <string>
+#include <list>
+#include <memory>
+#include <utility>
+
 namespace modsecurity {
 namespace variables {
 class Variable;
@@ -67,24 +64,15 @@ using MatchActions = std::vector<actions::Action *>;
 
 class Rule {
  public:
-    Rule(std::unique_ptr<std::string> fileName, int lineNumber)
-        : m_fileName(std::make_shared<std::string>(*fileName)),
+    Rule(const std::string &fileName, int lineNumber)
+        : m_fileName(fileName),
         m_lineNumber(lineNumber),
         m_phase(modsecurity::Phases::RequestHeadersPhase) {
         }
 
-    Rule(const Rule &other) :
-        m_fileName(other.m_fileName),
-        m_lineNumber(other.m_lineNumber),
-        m_phase(other.m_phase)
-    { }
+    Rule(const Rule &other) = delete;
 
-    Rule &operator=(const Rule& other) {
-        m_fileName = other.m_fileName;
-        m_lineNumber = other.m_lineNumber;
-        m_phase = other.m_phase;
-        return *this;
-    }
+    Rule &operator=(const Rule &other) = delete;
 
     virtual ~Rule() {}
 
@@ -93,7 +81,7 @@ class Rule {
     virtual bool evaluate(Transaction *transaction,
         std::shared_ptr<RuleMessage> rm) = 0;
 
-    std::shared_ptr<std::string> getFileName() const {
+    const std::string& getFileName() const {
         return m_fileName;
     }
 
@@ -105,18 +93,15 @@ class Rule {
     void setPhase(int phase) { m_phase = phase; }
 
     virtual std::string getReference() {
-        if (m_fileName) {
-            return *m_fileName + ":" + std::to_string(m_lineNumber);
-        }
-        return "<<no file>>:" + std::to_string(m_lineNumber);
+        return m_fileName + ":" + std::to_string(m_lineNumber);
     }
 
 
     virtual bool isMarker() { return false; }
 
  private:
-    std::shared_ptr<std::string> m_fileName;
-    int m_lineNumber;
+    const std::string m_fileName;
+    const int m_lineNumber;
     // FIXME: phase may not be neede to SecMarker.
     int m_phase;
 };

headers/modsecurity/rule_marker.h

@@ -13,15 +13,6 @@
  *
  */
 
-#ifdef __cplusplus
-#include <stack>
-#include <vector>
-#include <string>
-#include <list>
-#include <memory>
-#include <utility>
-#endif
-
 #ifndef HEADERS_MODSECURITY_RULE_MARKER_H_
 #define HEADERS_MODSECURITY_RULE_MARKER_H_
 
@@ -32,6 +23,9 @@
 
 #ifdef __cplusplus
 
+#include <string>
+#include <memory>
+
 namespace modsecurity {
 
 
@@ -39,48 +33,35 @@ class RuleMarker : public Rule {
  public:
     RuleMarker(
         const std::string &name,
-        std::unique_ptr<std::string> fileName,
+        const std::string &fileName,
         int lineNumber)
-        : Rule(std::move(fileName), lineNumber),
-        m_name(std::make_shared<std::string>(name)) { }
+        : Rule(fileName, lineNumber),
+        m_name(name) { }
 
-    RuleMarker(const RuleMarker& r) :
-        Rule(r),
-        m_name(r.m_name)
-    { }
-
-    RuleMarker &operator =(const RuleMarker& r) {
-        Rule::operator = (r);
-        m_name = r.m_name;
-        return *this;
-    }
+    RuleMarker(const RuleMarker &r) = delete;
 
+    RuleMarker &operator=(const RuleMarker &r) = delete;
+   
     virtual bool evaluate(Transaction *transaction,
         std::shared_ptr<RuleMessage> rm) override {
         return evaluate(transaction);
     }
 
     virtual bool evaluate(Transaction *transaction) override {
-        if (transaction->isInsideAMarker()) {
-            if (*transaction->getCurrentMarker() == *m_name) {
+        if (transaction->isInsideAMarker() &&
+            *transaction->getCurrentMarker() == m_name) {
                 transaction->removeMarker();
                 // FIXME: Move this to .cc
                 //        ms_dbg_a(transaction, 4, "Out of a SecMarker " + *m_name);
-            }
         }
 
         return true;
     };
 
-
-    std::shared_ptr<std::string> getName() {
-        return m_name;
-    }
-
     bool isMarker() override { return true; }
 
  private:
-    std::shared_ptr<std::string> m_name;
+    const std::string m_name;
 };
 
 

headers/modsecurity/rule_message.h

@@ -42,116 +42,13 @@ class RuleMessage {
         ClientLogMessageInfo = 4
     };
 
-    /**
-     *
-     * FIXME: RuleMessage is currently too big, doing a lot of
-     * unnecessary data duplication. Needs to be shrink down.
-     *
-     */
-    RuleMessage(RuleWithActions *rule, Transaction *trans) :
-        m_accuracy(rule->m_accuracy),
-        m_clientIpAddress(trans->m_clientIpAddress),
-        m_data(""),
-        m_id(trans->m_id),
-        m_isDisruptive(false),
-        m_match(""),
-        m_maturity(rule->m_maturity),
-        m_message(""),
-        m_noAuditLog(false),
-        m_phase(rule->getPhase() - 1),
-        m_reference(""),
-        m_rev(rule->m_rev),
+    RuleMessage(const RuleWithActions &rule, const Transaction &trans) :
         m_rule(rule),
-        m_ruleFile(rule->getFileName()),
-        m_ruleId(rule->m_ruleId),
-        m_ruleLine(rule->getLineNumber()),
-        m_saveMessage(true),
-        m_serverIpAddress(trans->m_serverIpAddress),
-        m_requestHostName(trans->m_requestHostName),
-        m_severity(0),
-        m_uriNoQueryStringDecoded(trans->m_uri_no_query_string_decoded),
-        m_ver(rule->m_ver),
-        m_tags()
+        m_transaction(trans)
     { }
 
-    explicit RuleMessage(RuleMessage *rule) :
-        m_accuracy(rule->m_accuracy),
-        m_clientIpAddress(rule->m_clientIpAddress),
-        m_data(rule->m_data),
-        m_id(rule->m_id),
-        m_isDisruptive(rule->m_isDisruptive),
-        m_match(rule->m_match),
-        m_maturity(rule->m_maturity),
-        m_message(rule->m_message),
-        m_noAuditLog(rule->m_noAuditLog),
-        m_phase(rule->m_phase),
-        m_reference(rule->m_reference),
-        m_rev(rule->m_rev),
-        m_rule(rule->m_rule),
-        m_ruleFile(rule->m_ruleFile),
-        m_ruleId(rule->m_ruleId),
-        m_ruleLine(rule->m_ruleLine),
-        m_saveMessage(rule->m_saveMessage),
-        m_serverIpAddress(rule->m_serverIpAddress),
-        m_requestHostName(rule->m_requestHostName),
-        m_severity(rule->m_severity),
-        m_uriNoQueryStringDecoded(rule->m_uriNoQueryStringDecoded),
-        m_ver(rule->m_ver),
-        m_tags(rule->m_tags)
-    { }
-
-    RuleMessage(const RuleMessage& ruleMessage)
-        : m_accuracy(ruleMessage.m_accuracy),
-        m_clientIpAddress(ruleMessage.m_clientIpAddress),
-        m_data(ruleMessage.m_data),
-        m_id(ruleMessage.m_id),
-        m_isDisruptive(ruleMessage.m_isDisruptive),
-        m_match(ruleMessage.m_match),
-        m_maturity(ruleMessage.m_maturity),
-        m_message(ruleMessage.m_message),
-        m_noAuditLog(ruleMessage.m_noAuditLog),
-        m_phase(ruleMessage.m_phase),
-        m_reference(ruleMessage.m_reference),
-        m_rev(ruleMessage.m_rev),
-        m_rule(ruleMessage.m_rule),
-        m_ruleFile(ruleMessage.m_ruleFile),
-        m_ruleId(ruleMessage.m_ruleId),
-        m_ruleLine(ruleMessage.m_ruleLine),
-        m_saveMessage(ruleMessage.m_saveMessage),
-        m_serverIpAddress(ruleMessage.m_serverIpAddress),
-        m_requestHostName(ruleMessage.m_requestHostName),
-        m_severity(ruleMessage.m_severity),
-        m_uriNoQueryStringDecoded(ruleMessage.m_uriNoQueryStringDecoded),
-        m_ver(ruleMessage.m_ver),
-        m_tags(ruleMessage.m_tags)
-    { }
-
-    RuleMessage &operator=(const RuleMessage& ruleMessage) {
-        m_accuracy = ruleMessage.m_accuracy;
-        m_clientIpAddress = ruleMessage.m_clientIpAddress;
-        m_data = ruleMessage.m_data;
-        m_id = ruleMessage.m_id;
-        m_isDisruptive = ruleMessage.m_isDisruptive;
-        m_match = ruleMessage.m_match;
-        m_maturity = ruleMessage.m_maturity;
-        m_message = ruleMessage.m_message;
-        m_noAuditLog = ruleMessage.m_noAuditLog;
-        m_phase = ruleMessage.m_phase;
-        m_reference = ruleMessage.m_reference;
-        m_rev = ruleMessage.m_rev;
-        m_rule = ruleMessage.m_rule;
-        m_ruleFile = ruleMessage.m_ruleFile;
-        m_ruleId = ruleMessage.m_ruleId;
-        m_ruleLine = ruleMessage.m_ruleLine;
-        m_saveMessage = ruleMessage.m_saveMessage;
-        m_serverIpAddress = ruleMessage.m_serverIpAddress;
-        m_requestHostName = ruleMessage.m_requestHostName;
-        m_severity = ruleMessage.m_severity;
-        m_uriNoQueryStringDecoded = ruleMessage.m_uriNoQueryStringDecoded;
-        m_ver = ruleMessage.m_ver;
-        m_tags = ruleMessage.m_tags;
-        return *this;
-    }
+    RuleMessage(const RuleMessage &ruleMessage) = default;
+    RuleMessage &operator=(const RuleMessage &ruleMessage) = delete;
 
     void clean() {
         m_data = "";
@@ -159,7 +56,6 @@ class RuleMessage {
         m_isDisruptive = false;
         m_reference = "";
         m_severity = 0;
-        m_ver = "";
     }
 
     std::string log() {
@@ -187,28 +83,18 @@ class RuleMessage {
     static std::string _details(const RuleMessage *rm);
     static std::string _errorLogTail(const RuleMessage *rm);
 
-    int m_accuracy;
-    std::shared_ptr<std::string> m_clientIpAddress;
+    int getPhase() const { return m_rule.getPhase() - 1; }
+
+    const RuleWithActions &m_rule;
+    const Transaction &m_transaction;
     std::string m_data;
-    std::shared_ptr<std::string> m_id;
-    bool m_isDisruptive;
+    bool m_isDisruptive = false;
     std::string m_match;
-    int m_maturity;
     std::string m_message;
-    bool m_noAuditLog;
-    int m_phase;
+    bool m_noAuditLog = false;
     std::string m_reference;
-    std::string m_rev;
-    RuleWithActions *m_rule;
-    std::shared_ptr<std::string> m_ruleFile;
-    int m_ruleId;
-    int m_ruleLine;
-    bool m_saveMessage;
-    std::shared_ptr<std::string> m_serverIpAddress;
-    std::shared_ptr<std::string> m_requestHostName;
-    int m_severity;
-    std::shared_ptr<std::string> m_uriNoQueryStringDecoded;
-    std::string m_ver;
+    bool m_saveMessage = true;
+    int m_severity = 0;
 
     std::list<std::string> m_tags;
 };

headers/modsecurity/rule_unconditional.h

@@ -13,15 +13,6 @@
  *
  */
 
-#ifdef __cplusplus
-#include <stack>
-#include <vector>
-#include <string>
-#include <list>
-#include <memory>
-#include <utility>
-#endif
-
 #ifndef HEADERS_MODSECURITY_RULE_UNCONDITIONAL_H_
 #define HEADERS_MODSECURITY_RULE_UNCONDITIONAL_H_
 
@@ -34,30 +25,18 @@
 
 #ifdef __cplusplus
 
+#include <vector>
+#include <string>
+#include <memory>
+
 namespace modsecurity {
 
 
 class RuleUnconditional : public RuleWithActions {
  public:
-    RuleUnconditional(
-        std::vector<actions::Action *> *actions,
-        Transformations *transformations,
-        std::unique_ptr<std::string> fileName,
-        int lineNumber)
-        : RuleWithActions(actions, transformations, std::move(fileName), lineNumber) { }
-
-    RuleUnconditional(const RuleUnconditional& r)
-        : RuleWithActions(r)
-    { }
-
-    RuleUnconditional &operator=(const RuleUnconditional& r) {
-        RuleWithActions::operator = (r);
-        return *this;
-    }
+    using RuleWithActions::RuleWithActions;
 
     virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
-
- private:
 };
 
 

headers/modsecurity/rule_with_actions.h

@@ -40,66 +40,18 @@ class RuleWithActions : public Rule {
     RuleWithActions(
         Actions *a,
         Transformations *t,
-        std::unique_ptr<std::string> fileName,
+        const std::string &fileName,
         int lineNumber);
 
-    ~RuleWithActions();
+    ~RuleWithActions() override;
 
-    RuleWithActions(const RuleWithActions& r)
-        : Rule(r),
-        m_rev(r.m_rev),
-        m_ver(r.m_ver),
-        m_accuracy(r.m_accuracy),
-        m_maturity(r.m_maturity),
-        m_ruleId(r.m_ruleId),
-        m_chainedRuleChild(r.m_chainedRuleChild),
-        m_chainedRuleParent(r.m_chainedRuleParent),
-        m_disruptiveAction(r.m_disruptiveAction),
-        m_logData(r.m_logData),
-        m_msg(r.m_msg),
-        m_severity(r.m_severity),
-        m_actionsRuntimePos(r.m_actionsRuntimePos),
-        m_actionsSetVar(r.m_actionsSetVar),
-        m_actionsTag(r.m_actionsTag),
-        m_transformations(r.m_transformations),
-        m_containsCaptureAction(r.m_containsCaptureAction),
-        m_containsMultiMatchAction(r.m_containsMultiMatchAction),
-        m_containsStaticBlockAction(r.m_containsStaticBlockAction),
-        m_isChained(r.m_isChained)
-    { }
+    RuleWithActions(const RuleWithActions &r) = delete;
 
-    RuleWithActions &operator=(const RuleWithActions& r) {
-        Rule::operator = (r);
-        m_rev = r.m_rev;
-        m_ver = r.m_ver;
-        m_accuracy = r.m_accuracy;
-        m_maturity = r.m_maturity;
-        m_ruleId = r.m_ruleId;
-        m_chainedRuleChild = r.m_chainedRuleChild;
-        m_chainedRuleParent = r.m_chainedRuleParent;
-
-        m_disruptiveAction = r.m_disruptiveAction;
-        m_logData = r.m_logData;
-        m_msg = r.m_msg;
-        m_severity = r.m_severity;
-        m_actionsRuntimePos = r.m_actionsRuntimePos;
-        m_actionsSetVar = r.m_actionsSetVar;
-        m_actionsTag = r.m_actionsTag;
-
-        m_transformations = r.m_transformations;
-
-        m_containsCaptureAction = r.m_containsCaptureAction;
-        m_containsMultiMatchAction = r.m_containsMultiMatchAction;
-        m_containsStaticBlockAction = r.m_containsStaticBlockAction;
-        m_isChained = r.m_isChained;
-
-        return *this;
-    }
-
-    virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
+    RuleWithActions &operator=(const RuleWithActions &r) = delete;
 
     virtual bool evaluate(Transaction *transaction) override;
 
+    virtual bool evaluate(Transaction *transaction, std::shared_ptr<RuleMessage> ruleMessage) override;
 
     void executeActionsIndependentOfChainedRuleResult(
         Transaction *trasn,
@@ -124,7 +76,7 @@ class RuleWithActions : public Rule {
     void performLogging(Transaction *trans,
         std::shared_ptr<RuleMessage> ruleMessage,
         bool lastLog = true,
-        bool chainedParentNull = false);
+        bool chainedParentNull = false) const;
 
     std::vector<actions::Action *> getActionsByName(const std::string& name,
         Transaction *t);

headers/modsecurity/rule_with_operator.h

@@ -42,10 +42,10 @@ class RuleWithOperator : public RuleWithActions {
         variables::Variables *variables,
         std::vector<actions::Action *> *actions,
         Transformations *transformations,
-        std::unique_ptr<std::string> fileName,
+        const std::string &fileName,
         int lineNumber);
 
-    virtual ~RuleWithOperator();
+    ~RuleWithOperator() override;
 
     bool evaluate(Transaction *transaction,
         std::shared_ptr<RuleMessage> rm) override;

headers/modsecurity/transaction.h

@@ -13,6 +13,9 @@
  *
  */
 
+#ifndef HEADERS_MODSECURITY_TRANSACTION_H_
+#define HEADERS_MODSECURITY_TRANSACTION_H_
+
 #ifdef __cplusplus
 #include <cassert>
 #include <ctime>
@@ -33,9 +36,6 @@
 #include <stdlib.h>
 #include <stddef.h>
 
-#ifndef HEADERS_MODSECURITY_TRANSACTION_H_
-#define HEADERS_MODSECURITY_TRANSACTION_H_
-
 #ifndef __cplusplus
 typedef struct ModSecurity_t ModSecurity;
 typedef struct Transaction_t Transaction;
@@ -57,7 +57,7 @@ typedef struct Rules_t RulesSet;
 #define ms_dbg(b, c) \
   do { \
       if (m_rules && m_rules->m_debugLog && m_rules->m_debugLog->m_debugLevel >= b) { \
-          m_rules->debug(b, *m_id.get(), m_uri, c); \
+          m_rules->debug(b, m_id, m_uri, c); \
       } \
   } while (0);
 #else
@@ -327,8 +327,8 @@ class TransactionSecMarkerManagement {
 /** @ingroup ModSecurity_CPP_API */
 class Transaction : public TransactionAnchoredVariables, public TransactionSecMarkerManagement {
  public:
-    Transaction(ModSecurity *transaction, RulesSet *rules, void *logCbData);
-    Transaction(ModSecurity *transaction, RulesSet *rules, char *id,
+    Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData);
+    Transaction(ModSecurity *ms, RulesSet *rules, const char *id,
         void *logCbData);
     ~Transaction();
 
@@ -426,12 +426,12 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
 	 *       need to be filled if there is no rule using the variable
 	 *       `duration'.
      */
-    clock_t m_creationTimeStamp;
+    const clock_t m_creationTimeStamp;
 
     /**
      * Holds the client IP address.
      */
-    std::shared_ptr<std::string> m_clientIpAddress;
+    std::string m_clientIpAddress;
 
     /**
      * Holds the HTTP version: 1.2, 2.0, 3.0 and so on....
@@ -441,12 +441,12 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
     /**
      * Holds the server IP Address
      */
-    std::shared_ptr<std::string> m_serverIpAddress;
+    std::string m_serverIpAddress;
 
     /**
      * Holds the request's hostname
      */
-    std::shared_ptr<std::string> m_requestHostName;
+    std::string m_requestHostName;
 
     /**
      * Holds the raw URI that was requested.
@@ -456,7 +456,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
     /**
      * Holds the URI that was requests (without the query string).
      */
-    std::shared_ptr<std::string> m_uri_no_query_string_decoded;
+    std::string m_uri_no_query_string_decoded;
 
     /**
      * Holds the combined size of all arguments, later used to fill the
@@ -505,7 +505,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
     /**
      * Rules object utilized during this specific transaction.
      */
-    RulesSet *m_rules;
+    RulesSet * const m_rules;
 
     /**
      *
@@ -568,7 +568,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
      * Contains the unique ID of the transaction. Use by the variable
 	 * `UNIQUE_ID'. This unique id is also saved as part of the AuditLog.
      */
-    std::shared_ptr<std::string> m_id;
+    const std::string m_id;
 
     /**
      * Holds the amount of rules that should be skipped. If bigger than 0 the
@@ -600,7 +600,7 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
      * TODO: m_timeStamp and m_creationTimeStamp may be merged into a single
      *       variable.
      */
-    time_t m_timeStamp;
+    const time_t m_timeStamp;
 
 
     /**
@@ -636,6 +636,10 @@ class Transaction : public TransactionAnchoredVariables, public TransactionSecMa
     std::vector<std::shared_ptr<RequestBodyProcessor::MultipartPartTmpFile>> m_multipartPartTmpFiles;
 
  private:
+
+    Transaction(ModSecurity *ms, RulesSet *rules, const char *id,
+        void *logCbData, const time_t timestamp);
+
     /**
      * Pointer to the callback function that will be called to fill
      * the web server (connector) log.
@@ -656,7 +660,7 @@ Transaction *msc_new_transaction(ModSecurity *ms,
 
 /** @ingroup ModSecurity_C_API */
 Transaction *msc_new_transaction_with_id(ModSecurity *ms,
-    RulesSet *rules, char *id, void *logCbData);
+    RulesSet *rules, const char *id, void *logCbData);
 
 /** @ingroup ModSecurity_C_API */
 int msc_process_connection(Transaction *transaction,