From a609249d640a56c4f8cc974aa1b00f56d45912a9 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 23 Jan 2019 10:29:36 -0300 Subject: [PATCH] Makes m_id a shared pointer --- headers/modsecurity/rule_message.h | 6 +-- headers/modsecurity/transaction.h | 4 +- src/audit_log/writer/parallel.cc | 2 +- src/request_body_processor/multipart.cc | 2 +- src/rule_message.cc | 70 +++++++++++-------------- src/transaction.cc | 19 +++---- 6 files changed, 49 insertions(+), 54 deletions(-) diff --git a/headers/modsecurity/rule_message.h b/headers/modsecurity/rule_message.h index 0ddfc883..1a7550c3 100644 --- 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 m_clientIpAddress; std::string m_data; - std::string m_id; + std::shared_ptr m_id; bool m_isDisruptive; std::string m_match; int m_maturity; diff --git a/headers/modsecurity/transaction.h b/headers/modsecurity/transaction.h index 3773957e..ba547a87 100644 --- 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 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 index 44d4fe5b..155ecdfd 100644 --- 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 index 713884b8..032cfd3c 100644 --- 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 index e0aaf9b6..f7a44093 100644 --- 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; +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) + "\"]"; - 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) + "\"]"); 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 index cd20cd91..4045fb10 100644 --- 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( + 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(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("request"),