From 4db5cc7d2620edae64cf5642435c48b6c078d5ee Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Thu, 14 Jan 2016 11:58:40 -0300 Subject: [PATCH] Refactoring on Transaction class: adding comments and renaming variables --- src/actions/audit_log.cc | 2 +- src/actions/block.cc | 2 +- src/actions/ctl_audit_log_parts.cc | 2 +- src/actions/deny.cc | 2 +- src/actions/log.cc | 2 +- src/actions/log_data.cc | 2 +- src/actions/msg.cc | 2 +- src/actions/no_audit_log.cc | 2 +- src/actions/pass.cc | 2 +- src/actions/redirect.cc | 2 +- src/actions/severity.cc | 6 ++--- src/actions/status.cc | 2 +- src/actions/tag.cc | 2 +- src/audit_log.cc | 6 ++--- src/audit_log_writer.cc | 2 +- src/audit_log_writer_parallel.cc | 10 +++---- src/transaction.cc | 43 +++++++++++++++--------------- src/variables/duration.cc | 2 +- src/variables/highest_severity.cc | 2 +- 19 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/actions/audit_log.cc b/src/actions/audit_log.cc index a03589e0..1fc8253d 100644 --- a/src/actions/audit_log.cc +++ b/src/actions/audit_log.cc @@ -24,7 +24,7 @@ namespace modsecurity { namespace actions { bool AuditLog::evaluate(Rule *rule, Transaction *transaction) { - transaction->save_in_auditlog = true; + transaction->m_toBeSavedInAuditlogs = true; return true; } diff --git a/src/actions/block.cc b/src/actions/block.cc index 7c79a63d..eec0d37b 100644 --- a/src/actions/block.cc +++ b/src/actions/block.cc @@ -38,7 +38,7 @@ bool Block::evaluate(Rule *rule, Transaction *transaction) { #endif for (Action *a : rule->actions_runtime_pos) { if (a->isDisruptive() == true) { - transaction->actions.push_back(a); + transaction->m_actions.push_back(a); } } return true; diff --git a/src/actions/ctl_audit_log_parts.cc b/src/actions/ctl_audit_log_parts.cc index ce3c5b95..43e1cfe5 100644 --- a/src/actions/ctl_audit_log_parts.cc +++ b/src/actions/ctl_audit_log_parts.cc @@ -36,7 +36,7 @@ CtlAuditLogParts::CtlAuditLogParts(std::string action) } bool CtlAuditLogParts::evaluate(Rule *rule, Transaction *transaction) { - transaction->auditLogModifier.push_back( + transaction->m_auditLogModifier.push_back( std::make_pair(mPartsAction, mParts)); return true; } diff --git a/src/actions/deny.cc b/src/actions/deny.cc index e7c198e2..26434a33 100644 --- a/src/actions/deny.cc +++ b/src/actions/deny.cc @@ -34,7 +34,7 @@ bool Deny::evaluate(Rule *rule, Transaction *transaction) { #ifndef NO_LOGS transaction->debug(8, "Running action deny"); #endif - transaction->actions.push_back(this); + transaction->m_actions.push_back(this); return true; } diff --git a/src/actions/log.cc b/src/actions/log.cc index feb8ce87..11eb3076 100644 --- a/src/actions/log.cc +++ b/src/actions/log.cc @@ -24,7 +24,7 @@ namespace modsecurity { namespace actions { bool Log::evaluate(Rule *rule, Transaction *transaction) { - transaction->save_in_auditlog = true; + transaction->m_toBeSavedInAuditlogs = true; /* FIXME: transaction->serverLog("Something...."); */ transaction->debug(9, "Saving transaction to logs"); return true; diff --git a/src/actions/log_data.cc b/src/actions/log_data.cc index 1f1f6e0c..6562fbd0 100644 --- a/src/actions/log_data.cc +++ b/src/actions/log_data.cc @@ -39,7 +39,7 @@ bool LogData::evaluate(Rule *rule, Transaction *transaction) { #ifndef NO_LOGS transaction->debug(9, "Saving msg: " + msg); #endif - transaction->rulesMessages.push_back(msg); + transaction->m_rulesMessages.push_back(msg); transaction->serverLog(msg); return true; } diff --git a/src/actions/msg.cc b/src/actions/msg.cc index 635fbd11..a8f79541 100644 --- a/src/actions/msg.cc +++ b/src/actions/msg.cc @@ -39,7 +39,7 @@ bool Msg::evaluate(Rule *rule, Transaction *transaction) { #ifndef NO_LOGS transaction->debug(9, "Saving msg: " + msg); #endif - transaction->rulesMessages.push_back(msg); + transaction->m_rulesMessages.push_back(msg); transaction->serverLog(msg); return true; } diff --git a/src/actions/no_audit_log.cc b/src/actions/no_audit_log.cc index f832ff38..90189c50 100644 --- a/src/actions/no_audit_log.cc +++ b/src/actions/no_audit_log.cc @@ -24,7 +24,7 @@ namespace modsecurity { namespace actions { bool NoAuditLog::evaluate(Rule *rule, Transaction *transaction) { - transaction->do_not_save_in_auditlog = true; + transaction->m_toNotBeSavedInAuditLogs = true; return true; } diff --git a/src/actions/pass.cc b/src/actions/pass.cc index e7b0da5d..fb63d242 100644 --- a/src/actions/pass.cc +++ b/src/actions/pass.cc @@ -32,7 +32,7 @@ Pass::Pass(std::string action) bool Pass::evaluate(Rule *rule, Transaction *transaction) { - transaction->actions.clear(); + transaction->m_actions.clear(); return true; } diff --git a/src/actions/redirect.cc b/src/actions/redirect.cc index e609c103..7fd082a7 100644 --- a/src/actions/redirect.cc +++ b/src/actions/redirect.cc @@ -43,7 +43,7 @@ Redirect::Redirect(const std::string& action) bool Redirect::evaluate(Rule *rule, Transaction *transaction) { m_urlExpanded = MacroExpansion::expand(m_url, transaction); - transaction->actions.push_back(this); + transaction->m_actions.push_back(this); return true; } diff --git a/src/actions/severity.cc b/src/actions/severity.cc index 9a00f2d7..34c19001 100644 --- a/src/actions/severity.cc +++ b/src/actions/severity.cc @@ -54,11 +54,11 @@ bool Severity::evaluate(Rule *rule, Transaction *transaction) { #ifndef NO_LOGS transaction->debug(9, "This rule severity is: " + \ std::to_string(this->m_severity) + " current transaction is: " + \ - std::to_string(transaction->highest_severity)); + std::to_string(transaction->m_highestSeverityAction)); #endif - if (transaction->highest_severity > this->m_severity) { - transaction->highest_severity = this->m_severity; + if (transaction->m_highestSeverityAction > this->m_severity) { + transaction->m_highestSeverityAction = this->m_severity; } return true; } diff --git a/src/actions/status.cc b/src/actions/status.cc index d5e71aa3..77bb2d8f 100644 --- a/src/actions/status.cc +++ b/src/actions/status.cc @@ -34,7 +34,7 @@ Status::Status(std::string action) bool Status::evaluate(Rule *rule, Transaction *transaction) { - transaction->actions.push_back(this); + transaction->m_actions.push_back(this); return true; } diff --git a/src/actions/tag.cc b/src/actions/tag.cc index a5397f37..92b64b71 100644 --- a/src/actions/tag.cc +++ b/src/actions/tag.cc @@ -39,7 +39,7 @@ bool Tag::evaluate(Rule *rule, Transaction *transaction) { #ifndef NO_LOGS transaction->debug(9, "Rule tag: " + tag); #endif - transaction->ruleTags.push_back(tag); + transaction->m_ruleTags.push_back(tag); return true; } diff --git a/src/audit_log.cc b/src/audit_log.cc index 152d6abd..268a86e8 100644 --- a/src/audit_log.cc +++ b/src/audit_log.cc @@ -225,8 +225,8 @@ bool AuditLog::saveIfRelevant(Transaction *transaction) { bool AuditLog::saveIfRelevant(Transaction *transaction, int parts) { - if (this->isRelevant(transaction->httpCodeReturned) == false && - transaction->save_in_auditlog == false) { + if (this->isRelevant(transaction->m_httpCodeReturned) == false && + transaction->m_toBeSavedInAuditlogs == false) { return false; } @@ -235,7 +235,7 @@ bool AuditLog::saveIfRelevant(Transaction *transaction, int parts) { * we won't save it. * */ - if (transaction->do_not_save_in_auditlog == true) { + if (transaction->m_toNotBeSavedInAuditLogs == true) { return false; } diff --git a/src/audit_log_writer.cc b/src/audit_log_writer.cc index 3d2e77d2..33739d23 100644 --- a/src/audit_log_writer.cc +++ b/src/audit_log_writer.cc @@ -34,7 +34,7 @@ std::string AuditLogWriter::file_name(const std::string& unique_id) { * */ bool AuditLogWriter::write(Transaction *transaction, int parts) { - std::cout << transaction->to_json(0) << std::endl; + std::cout << transaction->toJSON(0) << std::endl; return true; } diff --git a/src/audit_log_writer_parallel.cc b/src/audit_log_writer_parallel.cc index 23964587..c8ed9e73 100644 --- a/src/audit_log_writer_parallel.cc +++ b/src/audit_log_writer_parallel.cc @@ -92,23 +92,23 @@ bool AuditLogWriterParallel::init() { bool AuditLogWriterParallel::write(Transaction *transaction, int parts) { FILE *fp; int fd; - std::string log = transaction->to_json(parts); - std::string fileName = logFilePath(&transaction->timeStamp, + std::string log = transaction->toJSON(parts); + std::string fileName = logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory | YearMonthDayAndTimeDirectory | YearMonthDayAndTimeFileName); std::string logPath = m_audit->m_storage_dir; - fileName = logPath + fileName + "-" + transaction->id; + fileName = logPath + fileName + "-" + transaction->m_id; if (logPath.empty()) { return false; } createDir((logPath + - logFilePath(&transaction->timeStamp, YearMonthDayDirectory)).c_str(), + logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory)).c_str(), m_audit->directoryPermission); createDir((logPath + - logFilePath(&transaction->timeStamp, YearMonthDayDirectory + logFilePath(&transaction->m_timeStamp, YearMonthDayDirectory | YearMonthDayAndTimeDirectory)).c_str(), m_audit->directoryPermission); diff --git a/src/transaction.cc b/src/transaction.cc index 170b424e..5b63ff7c 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -19,26 +19,27 @@ #include #include #endif + #include #include -#include -#include -#include -#include -#include -#include -#include #include +#include +#include +#include +#include +#include +#include +#include -#include "modsecurity/modsecurity.h" -#include "modsecurity/intervention.h" #include "actions/action.h" #include "actions/deny.h" -#include "src/utils.h" +#include "modsecurity/intervention.h" +#include "modsecurity/modsecurity.h" +#include "request_body_processor/multipart.h" #include "src/audit_log.h" #include "src/unique_id.h" -#include "request_body_processor/multipart.h" +#include "src/utils.h" using modsecurity::actions::Action; using modsecurity::RequestBodyProcessor::Multipart; @@ -89,7 +90,7 @@ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData) m_clientPort(0), m_serverPort(0), m_uri(""), - m_protocol(""), + m_method(""), m_httpVersion(""), m_rules(rules), m_toBeSavedInAuditlogs(false), @@ -107,7 +108,7 @@ Transaction::Transaction(ModSecurity *ms, Rules *rules, void *logCbData) m_responseHeadersNames(NULL), m_responseContentType(NULL), m_marker(""), - start(cpu_seconds()), + m_creationTimeStamp(cpu_seconds()), m_logCbData(logCbData), m_ms(ms) { m_id = std::to_string(this->m_timeStamp) + \ @@ -235,7 +236,7 @@ int Transaction::processConnection(const char *client, int cPort, * * @param transaction ModSecurity transaction. * @param uri Uri. - * @param protocol Protocol (GET, POST, PUT). + * @param method Method (GET, POST, PUT). * @param http_version Http version (1.0, 1.2, 2.0). * * @returns If the operation was successful or not. @@ -243,14 +244,14 @@ int Transaction::processConnection(const char *client, int cPort, * @retval false Operation failed. * */ -int Transaction::processURI(const char *uri, const char *protocol, +int Transaction::processURI(const char *uri, const char *method, const char *http_version) { #ifndef NO_LOGS debug(4, "Starting phase URI. (SecRules 0 + 1/2)"); #endif - m_protocol = protocol; + m_method = method; m_httpVersion = http_version; m_uri = uri; std::string uri_s(uri); @@ -259,7 +260,7 @@ int Transaction::processURI(const char *uri, const char *protocol, size_t pos = m_uri_decoded.find("?"); size_t pos_raw = uri_s.find("?"); - m_collections.store("REQUEST_LINE", std::string(protocol) + " " + + m_collections.store("REQUEST_LINE", std::string(method) + " " + std::string(uri) + " HTTP/" + std::string(http_version)); if (pos_raw != std::string::npos) { @@ -282,7 +283,7 @@ int Transaction::processURI(const char *uri, const char *protocol, path_info.length() - offset); m_collections.store("REQUEST_BASENAME", basename); } - m_collections.store("REQUEST_METHOD", protocol); + m_collections.store("REQUEST_METHOD", method); m_collections.store("REQUEST_PROTOCOL", "HTTP/" + std::string(http_version)); @@ -1257,7 +1258,7 @@ std::string Transaction::toOldAuditLogFormatIndex(const std::string &filename, ss << tstr << " "; ss << "\""; - ss << this->m_protocol << " "; + ss << this->m_method << " "; ss << this->m_uri << " "; ss << "HTTP/" << m_httpVersion; ss << "\" "; @@ -1304,7 +1305,7 @@ std::string Transaction::toOldAuditLogFormat(int parts, if (parts & AuditLog::BAuditLogPart) { audit_log << "--" << trailer << "-" << "B--" << std::endl; - audit_log << this->m_protocol << " " << this->m_uri << " " << "HTTP/"; + audit_log << this->m_method << " " << this->m_uri << " " << "HTTP/"; audit_log << this->m_httpVersion << std::endl; for (auto h : m_collections.m_transient) { @@ -1410,7 +1411,7 @@ std::string Transaction::toJSON(int parts) { strlen("request")); yajl_gen_map_open(g); - LOGFY_ADD("protocol", m_protocol); + LOGFY_ADD("protocol", m_method); LOGFY_ADD_INT("http_version", m_httpVersion); LOGFY_ADD("uri", this->m_uri); diff --git a/src/variables/duration.cc b/src/variables/duration.cc index 9abb768d..fb964d04 100644 --- a/src/variables/duration.cc +++ b/src/variables/duration.cc @@ -31,7 +31,7 @@ void Duration::evaluateInternal(Transaction *transaction, std::vector *l) { std::string res; - double e = cpu_seconds() - transaction->start; + double e = cpu_seconds() - transaction->m_creationTimeStamp; res = std::to_string(e); diff --git a/src/variables/highest_severity.cc b/src/variables/highest_severity.cc index 9c43a2b8..34cecdf4 100644 --- a/src/variables/highest_severity.cc +++ b/src/variables/highest_severity.cc @@ -29,7 +29,7 @@ namespace Variables { void HighestSeverity::evaluateInternal(Transaction *transaction, std::vector *l) { l->push_back(new transaction::Variable("HIGHEST_SEVERITY", - std::to_string(transaction->highest_severity))); + std::to_string(transaction->m_highestSeverityAction))); }