Simplify and reduce code duplication in Transaction constructors

- Leverage delegating constructor to avoid code duplication between the
  two available Transaction constructors.
  - The constructor without 'id' argument delegates to the one that
    receives it by providing `nullptr` as a value, which is used to
    flag that an id needs to be generated.
- Simplified constructor by removing member initialization where the
  default constructor will be invoked.

Author: eduar-hte

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;
@@ -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,7 +426,7 @@ 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.
@@ -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::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,

src/audit_log/writer/parallel.cc

@@ -49,7 +49,7 @@ Parallel::~Parallel() {
 }
 
 
-inline std::string Parallel::logFilePath(time_t *t,
+inline std::string Parallel::logFilePath(const time_t *t,
     int part) {
     std::string name;
 

src/audit_log/writer/parallel.h

@@ -65,7 +65,7 @@ class Parallel : public Writer {
      YearMonthDayAndTimeFileName = 8,
     };
 
-    static inline std::string logFilePath(time_t *t, int part);
+    static inline std::string logFilePath(const time_t *t, int part);
 };
 
 }  // namespace writer

src/transaction.cc

@@ -102,90 +102,23 @@ namespace modsecurity {
  * @endcode
  *
  */
-Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
-    : m_creationTimeStamp(utils::cpu_seconds()),
-    m_clientIpAddress(""),
-    m_httpVersion(""),
-    m_serverIpAddress(""),
-    m_requestHostName(""),
-    m_uri(""),
-    m_uri_no_query_string_decoded(""),
-    m_ARGScombinedSizeDouble(0),
-    m_clientPort(0),
-    m_highestSeverityAction(255),
-    m_httpCodeReturned(200),
-    m_serverPort(0),
-    m_ms(ms),
-    m_requestBodyType(UnknownFormat),
-    m_requestBodyProcessor(UnknownFormat),
-    m_rules(rules),
-    m_ruleRemoveById(),
-    m_ruleRemoveByIdRange(),
-    m_ruleRemoveByTag(),
-    m_ruleRemoveTargetByTag(),
-    m_ruleRemoveTargetById(),
-    m_requestBodyAccess(RulesSet::PropertyNotSetConfigBoolean),
-    m_auditLogModifier(),
-    m_ctlAuditEngine(AuditLog::AuditLogStatus::NotSetLogStatus),
-    m_rulesMessages(),
-    m_requestBody(),
-    m_responseBody(),
-    /* m_id(), */
-    m_skip_next(0),
-    m_allowType(modsecurity::actions::disruptive::NoneAllowType),
-    m_uri_decoded(""),
-    m_actions(),
-    m_it(),
-    m_timeStamp(std::time(NULL)),
-    m_collections(ms->m_global_collection, ms->m_ip_collection,
-        ms->m_session_collection, ms->m_user_collection,
-        ms->m_resource_collection),
-    m_matched(),
-#ifdef WITH_LIBXML2
-    m_xml(new RequestBodyProcessor::XML(this)),
-#else
-    m_xml(NULL),
-#endif
-#ifdef WITH_YAJL
-    m_json(new RequestBodyProcessor::JSON(this)),
-#else
-    m_json(NULL),
-#endif
-    m_secRuleEngine(RulesSetProperties::PropertyNotSetRuleEngine),
-    m_variableDuration(""),
-    m_variableEnvs(),
-    m_variableHighestSeverityAction(""),
-    m_variableRemoteUser(""),
-    m_variableTime(""),
-    m_variableTimeDay(""),
-    m_variableTimeEpoch(""),
-    m_variableTimeHour(""),
-    m_variableTimeMin(""),
-    m_variableTimeSec(""),
-    m_variableTimeWDay(""),
-    m_variableTimeYear(""),
-    m_logCbData(logCbData),
-    TransactionAnchoredVariables(this) {
-    m_id = std::to_string(m_timeStamp) +
-           std::to_string(modsecurity::utils::generate_transaction_unique_id());
 
-    m_variableUrlEncodedError.set("0", 0);
-    m_variableMscPcreError.set("0", 0);
-    m_variableMscPcreLimitsExceeded.set("0", 0);
+static std::string get_id(const char *id, const time_t timestamp) {
+    return (id == nullptr) ?
+        std::to_string(timestamp) +
+            std::to_string(modsecurity::utils::generate_transaction_unique_id())
+        : id;
+}
 
-    ms_dbg(4, "Initializing transaction");
+Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
+    : Transaction(ms, rules, nullptr, logCbData) { }
 
-    intervention::clean(&m_it);
-}
+Transaction::Transaction(ModSecurity *ms, RulesSet *rules, const char *id, void *logCbData)
+    : Transaction(ms, rules, id, logCbData, std::time(nullptr)) { }
 
-Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCbData)
+Transaction::Transaction(ModSecurity *ms, RulesSet *rules, const char *id,
+    void *logCbData, const time_t timestamp)
     : m_creationTimeStamp(utils::cpu_seconds()),
-    m_clientIpAddress(""),
-    m_httpVersion(""),
-    m_serverIpAddress(""),
-    m_requestHostName(""),
-    m_uri(""),
-    m_uri_no_query_string_decoded(""),
     m_ARGScombinedSizeDouble(0),
     m_clientPort(0),
     m_highestSeverityAction(255),
@@ -195,54 +128,28 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCb
     m_requestBodyType(UnknownFormat),
     m_requestBodyProcessor(UnknownFormat),
     m_rules(rules),
-    m_ruleRemoveById(),
-    m_ruleRemoveByIdRange(),
-    m_ruleRemoveByTag(),
-    m_ruleRemoveTargetByTag(),
-    m_ruleRemoveTargetById(),
     m_requestBodyAccess(RulesSet::PropertyNotSetConfigBoolean),
-    m_auditLogModifier(),
     m_ctlAuditEngine(AuditLog::AuditLogStatus::NotSetLogStatus),
-    m_rulesMessages(),
-    m_requestBody(),
-    m_responseBody(),
-    m_id(id),
+    m_id(get_id(id, timestamp)),
     m_skip_next(0),
     m_allowType(modsecurity::actions::disruptive::NoneAllowType),
-    m_uri_decoded(""),
-    m_actions(),
-    m_it(),
-    m_timeStamp(std::time(NULL)),
+    m_timeStamp(timestamp),
     m_collections(ms->m_global_collection, ms->m_ip_collection,
         ms->m_session_collection, ms->m_user_collection,
         ms->m_resource_collection),
-    m_matched(),
 #ifdef WITH_LIBXML2
     m_xml(new RequestBodyProcessor::XML(this)),
 #else
-    m_xml(NULL),
+    m_xml(nullptr),
 #endif
 #ifdef WITH_YAJL
     m_json(new RequestBodyProcessor::JSON(this)),
 #else
-    m_json(NULL),
+    m_json(nullptr),
 #endif
     m_secRuleEngine(RulesSetProperties::PropertyNotSetRuleEngine),
-    m_variableDuration(""),
-    m_variableEnvs(),
-    m_variableHighestSeverityAction(""),
-    m_variableRemoteUser(""),
-    m_variableTime(""),
-    m_variableTimeDay(""),
-    m_variableTimeEpoch(""),
-    m_variableTimeHour(""),
-    m_variableTimeMin(""),
-    m_variableTimeSec(""),
-    m_variableTimeWDay(""),
-    m_variableTimeYear(""),
     m_logCbData(logCbData),
     TransactionAnchoredVariables(this) {
-
     m_variableUrlEncodedError.set("0", 0);
     m_variableMscPcreError.set("0", 0);
     m_variableMscPcreLimitsExceeded.set("0", 0);
@@ -1904,7 +1811,7 @@ extern "C" Transaction *msc_new_transaction(ModSecurity *ms,
     return new Transaction(ms, rules, logCbData);
 }
 extern "C" Transaction *msc_new_transaction_with_id(ModSecurity *ms,
-    RulesSet *rules, char *id, void *logCbData) {
+    RulesSet *rules, const char *id, void *logCbData) {
     return new Transaction(ms, rules, id, logCbData);
 }