Refactoring on RuleMessage class, now accepting http code as parameter

CHANGES

@@ -2,6 +2,8 @@
 v3.0.????? - ?
 ---------------------------
 
+ - Refactoring on RuleMessage class, now accepting http code as parameter.
+   [@zimmerle]
  - Having disruptive msgs as disruptive [instead of warnings] on audit log
    [Issue #1592 - @zimmerle, @nobodysz]
  - Parser: Pipes are no longer welcomed inside regex dict element selection.

headers/modsecurity/rule_message.h

@@ -32,13 +32,19 @@
 
 namespace modsecurity {
 
+
+
 class RuleMessage {
  public:
+    enum LogMessageInfo {
+        ErrorLogTailLogMessageInfo = 2,
+        ClientLogMessageInfo = 4
+    };
+
     explicit RuleMessage(Rule *rule, Transaction *trans) :
         m_accuracy(rule->m_accuracy),
         m_clientIpAddress(trans->m_clientIpAddress),
         m_data(""),
-        m_disruptiveMessage(""),
         m_id(trans->m_id),
         m_isDisruptive(false),
         m_match(""),
@@ -59,35 +65,31 @@ class RuleMessage {
         m_ver(rule->m_ver)
     { }
 
-    std::string errorLog() {
+
-        return RuleMessage::errorLog(this);
-    }
-    std::string disruptiveErrorLog() {
-        return RuleMessage::disruptiveErrorLog(this);
-    }
-    std::string noClientErrorLog() {
-        return RuleMessage::noClientErrorLog(this);
-    }
-    std::string noClientErrorLog(bool disrupt) {
-        return RuleMessage::noClientErrorLog(this, disrupt);
-    }
-    std::string errorLogTail() {
-        return RuleMessage::errorLogTail(this);
-    }
     std::string log() {
-        return RuleMessage::log(this);
+        return RuleMessage::log(this, 0);
     }
-    static std::string disruptiveErrorLog(const RuleMessage *rm);
+    std::string log(int props) {
-    static std::string noClientErrorLog(const RuleMessage *rm);
+        return RuleMessage::log(this, props);
-    static std::string noClientErrorLog(const RuleMessage *rm, bool disrupt);
+    }
-    static std::string errorLogTail(const RuleMessage *rm);
+    std::string errorLog() {
-    static std::string errorLog(const RuleMessage *rm);
+        return RuleMessage::log(this, ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
-    static std::string log(const RuleMessage *rm);
+    }
+
+    static std::string log(const RuleMessage *rm, int props, int code);
+    static std::string log(const RuleMessage *rm, int props) {
+        return RuleMessage::log(rm, props, -1);
+    }
+    static std::string log(const RuleMessage *rm) {
+        return RuleMessage::log(rm, 0);
+    }
+
+    static std::string _details(const RuleMessage *rm);
+    static std::string _errorLogTail(const RuleMessage *rm);
 
     int m_accuracy;
     std::string m_clientIpAddress;
     std::string m_data;
-    std::string m_disruptiveMessage;
     std::string m_id;
     bool m_isDisruptive;
     std::string m_match;

src/actions/disruptive/deny.cc

@@ -33,21 +33,15 @@ bool Deny::evaluate(Rule *rule, Transaction *transaction,
 #ifndef NO_LOGS
     transaction->debug(8, "Running action deny");
 #endif
-    std::string log;
 
     if (transaction->m_it.status == 200) {
         transaction->m_it.status = 403;
     }
 
-    log.append("Access denied with code %d");
-    log.append(" (phase ");
-    log.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
-
-    rm->m_disruptiveMessage.assign(log);
     transaction->m_it.disruptive = true;
     intervention::freeLog(&transaction->m_it);
     transaction->m_it.log = strdup(
-        rm->disruptiveErrorLog().c_str());
+        rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
 
     rm->m_isDisruptive = true;
     return true;

src/actions/disruptive/redirect.cc

@@ -40,23 +40,18 @@ bool Redirect::init(std::string *error) {
 bool Redirect::evaluate(Rule *rule, Transaction *transaction,
    std::shared_ptr<RuleMessage> rm) {
     m_urlExpanded = MacroExpansion::expand(m_url, transaction);
-    std::string log;
 
     /* if it was changed before, lets keep it. */
     if (transaction->m_it.status == 200) {
         transaction->m_it.status = m_status;
     }
-    log.append("Access denied with code %d");
-    log.append(" (phase ");
-    log.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
 
-    rm->m_disruptiveMessage.assign(log);
     intervention::freeUrl(&transaction->m_it);
     transaction->m_it.url = strdup(m_urlExpanded.c_str());
     transaction->m_it.disruptive = true;
     intervention::freeLog(&transaction->m_it);
     transaction->m_it.log = strdup(
-        rm->disruptiveErrorLog().c_str());
+        rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
 
     rm->m_isDisruptive = true;
     return true;

src/rule_message.cc

@@ -23,13 +23,10 @@
 
 namespace modsecurity {
 
-std::string RuleMessage::disruptiveErrorLog(const RuleMessage *rm) {
+
+std::string RuleMessage::_details(const RuleMessage *rm) {
     std::string msg;
 
-    msg.append("[client " + std::string(rm->m_clientIpAddress) + "]");
-    msg.append(" ModSecurity: ");
-    msg.append(rm->m_disruptiveMessage);
-    msg.append(rm->m_match);
     msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
     msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
     msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
@@ -50,91 +47,55 @@ std::string RuleMessage::disruptiveErrorLog(const RuleMessage *rm) {
     msg.append(" [unique_id \"" + rm->m_id + "\"]");
     msg.append(" [ref \"" + rm->m_reference + "\"]");
 
-    return modsecurity::utils::string::toHexIfNeeded(msg);
+    return msg;
 }
 
-std::string RuleMessage::noClientErrorLog(const RuleMessage *rm, bool disruptive) {
-    std::string msg;
-    if (disruptive == false) {
-        return RuleMessage::noClientErrorLog(rm);
-    }
 
-    msg.append("Message: ");
+std::string RuleMessage::_errorLogTail(const RuleMessage *rm) {
-    msg.append(rm->m_disruptiveMessage);
-    msg.append(rm->m_match);
-    msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
-    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 \"" + 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.append(" [ref \"" + rm->m_reference + "\"]");
-
-    return modsecurity::utils::string::toHexIfNeeded(msg);
-}
-
-std::string RuleMessage::noClientErrorLog(const RuleMessage *rm) {
     std::string msg;
 
-    msg.append("ModSecurity: Warning. ");
+    msg.append("[hostname \"" + std::string(rm->m_serverIpAddress) + "\"]");
-    msg.append(rm->m_match);
-    msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
-    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 \"" + 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.append(" [ref \"" + rm->m_reference + "\"]");
-
-    return modsecurity::utils::string::toHexIfNeeded(msg);
-}
-
-std::string RuleMessage::errorLogTail(const RuleMessage *rm) {
-    std::string msg;
-
-    msg.append("[hostname \"" + std::string(rm->m_serverIpAddress) \
-        + "\"]");
     msg.append(" [uri \"" + rm->m_uriNoQueryStringDecoded + "\"]");
     msg.append(" [unique_id \"" + rm->m_id + "\"]");
 
+    return msg;
+}
+
+
+std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
+    std::string msg("");
+
+    if (props & ClientLogMessageInfo) {
+        msg.append("[client " + std::string(rm->m_clientIpAddress) + "] ");
+    }
+
+    if (rm->m_isDisruptive)
+    {
+        msg.append("ModSecurity: Access denied with code ");
+        if (code == -1) {
+            msg.append("%d");
+        }
+        else
+        {
+            msg.append(std::to_string(code));
+        }
+        msg.append(" (phase ");
+        msg.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
+    }
+    else
+    {
+        msg.append("ModSecurity: Warning. ");
+    }
+
+    msg.append(rm->m_match);
+    msg.append(_details(rm));
+
+    if (props & ErrorLogTailLogMessageInfo) {
+        msg.append(" " + _errorLogTail(rm));
+    }
+
     return modsecurity::utils::string::toHexIfNeeded(msg);
 }
 
-std::string RuleMessage::errorLog(const RuleMessage *rm) {
-    std::string msg;
-
-    msg.append("[client " + std::string(rm->m_clientIpAddress) + "] ");
-    msg.append(noClientErrorLog(rm));
-    msg.append(" " + errorLogTail(rm));
-
-    return msg;
-}
-
-std::string RuleMessage::log(const RuleMessage *rm) {
-    std::string msg("");
-    if (rm->m_isDisruptive) {
-        msg.append(disruptiveErrorLog(rm));
-    } else {
-        msg.append(errorLog(rm));
-    }
-
-    return msg;
-}
 
 }  // namespace modsecurity

src/transaction.cc

@@ -1485,11 +1485,7 @@ std::string Transaction::toOldAuditLogFormat(int parts,
     if (parts & audit_log::AuditLog::HAuditLogPart) {
         audit_log << "--" << trailer << "-" << "H--" << std::endl;
         for (auto a : m_rulesMessages) {
-            if (a.m_isDisruptive == true) {
+            audit_log << a.log() << std::endl;
-                audit_log << a.noClientErrorLog(true) << std::endl;
-            } else {
-                audit_log << a.noClientErrorLog() << std::endl;
-            }
         }
         audit_log << std::endl;
         /** TODO: write audit_log H part. */