Makes m_id a shared pointer

owasp-modsecurity · Mar 27, 2020 · a609249 · a609249
1 parent 343b86c
commit a609249
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 55 deletions.
diff --git a/headers/modsecurity/rule_message.h b/headers/modsecurity/rule_message.h
@@ -88,13 +88,13 @@ class RuleMessage {
         return log(rm, 0);
     }
 
-    static std::string _details(const RuleMessage *rm);
-    static std::string _errorLogTail(const RuleMessage *rm);
+    static inline void _details(const RuleMessage *rm, std::string *msg);
+    static inline void _errorLogTail(const RuleMessage *rm, std::string *msg);
 
     int m_accuracy;
     std::shared_ptr<std::string> m_clientIpAddress;
     std::string m_data;
-    std::string m_id;
+    std::shared_ptr<std::string> m_id;
     bool m_isDisruptive;
     std::string m_match;
     int m_maturity;

diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h
@@ -52,7 +52,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, m_uri, c); \
+          m_rules->debug(b, *m_id.get(), m_uri, c); \
       } \
   } while (0);
 #else
@@ -516,7 +516,7 @@ class Transaction : public TransactionAnchoredVariables {
      * 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;
+    std::shared_ptr<std::string> m_id;
 
     /**
      * Holds the SecMarker name that this transaction should wait to perform

diff --git a/src/audit_log/writer/parallel.cc b/src/audit_log/writer/parallel.cc
@@ -119,7 +119,7 @@ bool Parallel::write(Transaction *transaction, int parts, std::string *error) {
     }
 
     std::string logPath = m_audit->m_storage_dir;
-    fileName = logPath + fileName + "-" + transaction->m_id;
+    fileName = logPath + fileName + "-" + *transaction->m_id.get();
 
     if (logPath.empty()) {
         error->assign("Log path is not valid.");

diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc
@@ -469,7 +469,7 @@ int Multipart::tmp_file_name(std::string *filename) const {
 
     memset(tstr, '\0', 300);
     strftime(tstr, 299, "/%Y%m%d-%H%M%S", &timeinfo);
-    path = path + tstr + "-" + m_transaction->m_id;
+    path = path + tstr + "-" + *m_transaction->m_id.get();
     path = path + "-file-XXXXXX";
 
     tmp = strdup(path.c_str());

diff --git a/src/rule_message.cc b/src/rule_message.cc
@@ -23,69 +23,63 @@
 namespace modsecurity {
 
 
-std::string RuleMessage::_details(const RuleMessage *rm) {
-    std::string msg;
-
-    msg.append(" [file \"" + std::string(*rm->m_ruleFile.get()) + "\"]");
-    msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
-    msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
-    msg.append(" [rev \"" + rm->m_rev + "\"]");
-    msg.append(" [msg \"" + rm->m_message + "\"]");
-    msg.append(" [data \"" + utils::string::limitTo(200, rm->m_data) + "\"]");
-    msg.append(" [severity \"" +
-        std::to_string(rm->m_severity) + "\"]");
-    msg.append(" [ver \"" + rm->m_ver + "\"]");
-    msg.append(" [maturity \"" + std::to_string(rm->m_maturity) + "\"]");
-    msg.append(" [accuracy \"" + std::to_string(rm->m_accuracy) + "\"]");
+inline void RuleMessage::_details(const RuleMessage *rm, std::string *msg) {
+    *msg += " [file \"" + std::string(*rm->m_ruleFile.get()) + "\"]" \
+        " [line \"" + std::to_string(rm->m_ruleLine) + "\"]" \
+        " [id \"" + std::to_string(rm->m_ruleId) + "\"]" \
+        " [rev \"" + rm->m_rev + "\"]" \
+        " [msg \"" + rm->m_message + "\"]" \
+        " [data \"" + utils::string::limitTo(200, rm->m_data) + "\"]" \
+        " [severity \"" + std::to_string(rm->m_severity) + "\"]" \
+        " [ver \"" + rm->m_ver + "\"]" \
+        " [maturity \"" + std::to_string(rm->m_maturity) + "\"]" \
+        " [accuracy \"" + std::to_string(rm->m_accuracy) + "\"]";
+
     for (auto &a : rm->m_tags) {
-        msg.append(" [tag \"" + a + "\"]");
+        *msg += " [tag \"" + a + "\"]";
     }
-    msg.append(" [hostname \"" + *rm->m_serverIpAddress.get() \
-        + "\"]");
-    msg.append(" [uri \"" + utils::string::limitTo(200, *rm->m_uriNoQueryStringDecoded.get()) + "\"]");
-    msg.append(" [unique_id \"" + rm->m_id + "\"]");
-    msg.append(" [ref \"" + utils::string::limitTo(200, rm->m_reference) + "\"]");
 
-    return msg;
+    *msg += " [hostname \"" + *rm->m_serverIpAddress.get() + "\"]" \
+        " [uri \"" + *rm->m_uriNoQueryStringDecoded.get() + "\"]" \
+        " [unique_id \"" + *rm->m_id.get() + "\"]" \
+        " [ref \"" + utils::string::limitTo(200, rm->m_reference) + "\"]";
 }
 
 
-std::string RuleMessage::_errorLogTail(const RuleMessage *rm) {
-    std::string msg;
-
-    msg.append("[hostname \"" + *rm->m_serverIpAddress.get() + "\"]");
-    msg.append(" [uri \"" + utils::string::limitTo(200, *rm->m_uriNoQueryStringDecoded.get()) + "\"]");
-    msg.append(" [unique_id \"" + rm->m_id + "\"]");
-
-    return msg;
+inline void RuleMessage::_errorLogTail(const RuleMessage *rm,
+    std::string *msg) {
+    *msg += " [hostname \"" + *rm->m_serverIpAddress.get() + "\"]" \
+        " [uri \"" + utils::string::limitTo(200,
+            *rm->m_uriNoQueryStringDecoded.get()) + "\"]" \
+        " [unique_id \"" + *rm->m_id.get() + "\"]";
 }
 
 
 std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
     std::string msg("");
+    msg.reserve(2048);
 
     if (props & ClientLogMessageInfo) {
-        msg.append("[client " + std::string(*rm->m_clientIpAddress.get()) + "] ");
+        msg += "[client " + std::string(*rm->m_clientIpAddress.get()) + "] ";
     }
 
     if (rm->m_isDisruptive) {
-        msg.append("ModSecurity: Access denied with code ");
+        msg += "ModSecurity: Access denied with code ";
         if (code == -1) {
-            msg.append("%d");
+            msg += "%d";
         } else {
-            msg.append(std::to_string(code));
+            msg += std::to_string(code);
         }
-        msg.append(" (phase ");
-        msg.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
+        msg += " (phase " + std::to_string(rm->m_rule->m_phase - 1) + "). ";
     } else {
-        msg.append("ModSecurity: Warning. ");
+        msg += "ModSecurity: Warning. ";
     }
 
-    msg.append(rm->m_match);
-    msg.append(_details(rm));
+    msg += (rm->m_match);
+    _details(rm, &msg);
 
     if (props & ErrorLogTailLogMessageInfo) {
-        msg.append(" " + _errorLogTail(rm));
+        _errorLogTail(rm, &msg);
     }
 
     return modsecurity::utils::string::toHexIfNeeded(msg);

diff --git a/src/transaction.cc b/src/transaction.cc
@@ -125,7 +125,7 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
     m_rulesMessages(),
     m_requestBody(),
     m_responseBody(),
-    m_id(),
+    /* m_id(), */
     m_marker(""),
     m_skip_next(0),
     m_allowType(modsecurity::actions::disruptive::NoneAllowType),
@@ -162,8 +162,9 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, void *logCbData)
     m_variableTimeYear(""),
     m_logCbData(logCbData),
     TransactionAnchoredVariables(this) {
-    m_id = std::to_string(this->m_timeStamp) + \
-        std::to_string(modsecurity::utils::generate_transaction_unique_id());
+    m_id = std::unique_ptr<std::string>(
+        new std::string(
+            std::to_string(m_timeStamp)));
 
     m_variableUrlEncodedError.set("0", 0);
 
@@ -198,7 +199,7 @@ Transaction::Transaction(ModSecurity *ms, RulesSet *rules, char *id, void *logCb
     m_rulesMessages(),
     m_requestBody(),
     m_responseBody(),
-    m_id(std::string(id)),
+    m_id(std::unique_ptr<std::string>(new std::string(id))),
     m_marker(""),
     m_skip_next(0),
     m_allowType(modsecurity::actions::disruptive::NoneAllowType),
@@ -282,7 +283,7 @@ void Transaction::debug(int level, std::string message) const {
         return;
     }
 
-    m_rules->debug(level, m_id, m_uri, message);
+    m_rules->debug(level, *m_id.get(), m_uri, message);
 }
 #endif
 
@@ -318,7 +319,7 @@ int Transaction::processConnection(const char *client, int cPort,
 
 
     m_variableRemoteHost.set(*m_clientIpAddress.get(), m_variableOffset);
-    m_variableUniqueID.set(m_id, m_variableOffset);
+    m_variableUniqueID.set(*m_id.get(), m_variableOffset);
     m_variableRemoteAddr.set(*m_clientIpAddress.get(), m_variableOffset);
     m_variableServerAddr.set(*m_serverIpAddress.get(), m_variableOffset);
     m_variableServerPort.set(std::to_string(this->m_serverPort),
@@ -1496,7 +1497,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename,
     ss << utils::string::dash_if_empty(
         m_variableRequestHeaders.resolveFirst("User-Agent").get());
     ss << "\" ";
-    ss << this->m_id << " ";
+    ss << *m_id.get() << " ";
     /** TODO: Check variable */
     ss << utils::string::dash_if_empty(
         m_variableRequestHeaders.resolveFirst("REFERER").get()) << " ";
@@ -1522,7 +1523,7 @@ std::string Transaction::toOldAuditLogFormat(int parts,
     audit_log << "--" << trailer << "-" << "A--" << std::endl;
     strftime(tstr, 299, "[%d/%b/%Y:%H:%M:%S %z]", &timeinfo);
     audit_log << tstr;
-    audit_log << " " << this->m_id.c_str();
+    audit_log << " " << m_id->c_str();
     audit_log << " " << this->m_clientIpAddress;
     audit_log << " " << this->m_clientPort;
     audit_log << " " << m_serverIpAddress;
@@ -1648,7 +1649,7 @@ std::string Transaction::toJSON(int parts) {
     LOGFY_ADD_NUM("client_port", m_clientPort);
     LOGFY_ADD("host_ip", m_serverIpAddress->c_str());
     LOGFY_ADD_NUM("host_port", m_serverPort);
-    LOGFY_ADD("unique_id", this->m_id.c_str());
+    LOGFY_ADD("unique_id", m_id->c_str());
 
     /* request */
     yajl_gen_string(g, reinterpret_cast<const unsigned char*>("request"),