Refactoring on the operators: negation is now being handled globally

Other minors changes were also made, including adding the prefix `m_'
to all the members of the class.

src/operators/begins_with.cc

@@ -27,7 +27,7 @@ namespace operators {
 bool BeginsWith::evaluate(Transaction *transaction, const std::string &str) {
     bool ret = false;
 
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
 
     if (str.size() < p.size()) {
         ret = false;
@@ -35,10 +35,6 @@ bool BeginsWith::evaluate(Transaction *transaction, const std::string &str) {
          ret = true;
     }
 
-    if (negation) {
-        return !ret;
-    }
-
     return ret;
 }
 

src/operators/contains.cc

@@ -23,17 +23,13 @@ namespace modsecurity {
 namespace operators {
 
 bool Contains::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
     bool contains = input.find(p) != std::string::npos;
 
     if (contains && transaction) {
         transaction->m_matched.push_back(p);
     }
 
-    if (negation) {
-        return !contains;
-    }
-
     return contains;
 }
 

src/operators/contains_word.cc

@@ -38,7 +38,7 @@ bool ContainsWord::acceptableChar(const std::string& a, size_t pos) {
 
 bool ContainsWord::evaluate(Transaction *transaction,
     const std::string& input) {
-    std::string paramTarget = MacroExpansion::expand(param, transaction);
+    std::string paramTarget = MacroExpansion::expand(m_param, transaction);
 
     if (paramTarget.empty()) {
         return true;

src/operators/detect_sqli.cc

@@ -49,10 +49,6 @@ bool DetectSQLi::evaluate(Transaction *transaction, const std::string &input) {
         }
     }
 
-    if (negation) {
-        return issqli == 0;
-    }
-
     return issqli != 0;
 }
 

src/operators/detect_xss.cc

@@ -41,10 +41,6 @@ bool DetectXSS::evaluate(Transaction *transaction, const std::string &input) {
 #endif
     }
 
-    if (negation) {
-        return is_xss == 0;
-    }
-
     return is_xss != 0;
 }
 

src/operators/ends_with.cc

@@ -26,17 +26,13 @@ namespace operators {
 
 bool EndsWith::evaluate(Transaction *transaction, const std::string &input) {
     bool ret = false;
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
 
     if (input.length() >= p.length()) {
         ret = (0 == input.compare(input.length() - p.length(),
             p.length(), p));
     }
 
-    if (negation) {
-        return !ret;
-    }
-
     return ret;
 }
 

src/operators/eq.cc

@@ -28,7 +28,7 @@ bool Eq::evaluate(Transaction *transaction, const std::string &input) {
     int p = 0;
     int i = 0;
     bool eq = false;
-    std::string pt = MacroExpansion::expand(param, transaction);
+    std::string pt = MacroExpansion::expand(m_param, transaction);
 
     try {
         p = std::stoi(pt);
@@ -43,10 +43,6 @@ bool Eq::evaluate(Transaction *transaction, const std::string &input) {
 
     eq = p == i;
 
-    if (negation) {
-        return !eq;
-    }
-
     return eq;
 }
 

src/operators/fuzzy_hash.cc

@@ -34,8 +34,8 @@ bool FuzzyHash::evaluate(Transaction *transaction, const std::string &str) {
 FuzzyHash::FuzzyHash(std::string op, std::string param,
     bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/ge.cc

@@ -24,15 +24,11 @@ namespace modsecurity {
 namespace operators {
 
 bool Ge::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
     std::string i = MacroExpansion::expand(input, transaction);
 
     bool ge = atoll(i.c_str()) >= atoll(p.c_str());
 
-    if (negation) {
-        return !ge;
-    }
-
     return ge;
 }
 

src/operators/geo_lookup.cc

@@ -96,8 +96,8 @@ bool GeoLookup::evaluate(Transaction *trans, const std::string &exp) {
 GeoLookup::GeoLookup(std::string op, std::string param,
 bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/gsblookup.cc

@@ -34,8 +34,8 @@ bool GsbLookup::evaluate(Transaction *transaction, const std::string &str) {
 GsbLookup::GsbLookup(std::string op, std::string param,
     bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/gt.cc

@@ -24,14 +24,10 @@ namespace modsecurity {
 namespace operators {
 
 bool Gt::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
 
     bool gt = atoll(input.c_str()) > atoll(p.c_str());
 
-    if (negation) {
-        return !gt;
-    }
-
     return gt;
 }
 

src/operators/inspect_file.cc

@@ -34,8 +34,8 @@ bool InspectFile::evaluate(Transaction *transaction, const std::string &str) {
 InspectFile::InspectFile(std::string op, std::string param,
     bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/ip_match.cc

@@ -27,7 +27,7 @@ namespace operators {
 
 bool IpMatch::init(const std::string &file, std::string *error) {
     std::string e("");
-    bool res = m_tree.addFromBuffer(param, &e);
+    bool res = m_tree.addFromBuffer(m_param, &e);
 
     if (res == false) {
         error->assign(e);

src/operators/ip_match_from_file.cc

@@ -30,10 +30,10 @@ bool IpMatchFromFile::init(const std::string &file,
     std::string e("");
     bool res = false;
 
-    if (param.compare(0, 8, "https://") == 0) {
+    if (m_param.compare(0, 8, "https://") == 0) {
-        res = m_tree.addFromUrl(param, &e);
+        res = m_tree.addFromUrl(m_param, &e);
     } else {
-        res = m_tree.addFromFile(param, &e);
+        res = m_tree.addFromFile(m_param, &e);
     }
 
     if (res == false) {

src/operators/le.cc

@@ -24,14 +24,10 @@ namespace modsecurity {
 namespace operators {
 
 bool Le::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
 
     bool le = atoll(input.c_str()) <= atoll(p.c_str());
 
-    if (negation) {
-        return !le;
-    }
-
     return le;
 }
 

src/operators/lt.cc

@@ -24,14 +24,10 @@ namespace modsecurity {
 namespace operators {
 
 bool Lt::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
 
     bool lt = atoll(input.c_str()) < atoll(p.c_str());
 
-    if (negation) {
-        return !lt;
-    }
-
     return lt;
 }
 

src/operators/operator.cc

@@ -75,13 +75,25 @@ bool Operator::debug(Transaction *transaction, int x, std::string a) {
 }
 
 
+bool Operator::evaluateInternal(Transaction *transaction,
+    const std::string& a) {
+    bool res = evaluate(transaction, a);
+
+    if (m_negation) {
+        return !res;
+    }
+
+    return res;
+}
+
+
 bool Operator::evaluate(Transaction *transaction, const std::string& a) {
 #ifndef NO_LOGS
     if (transaction) {
-        transaction->debug(2, "Operator: " + this->op + \
+        transaction->debug(2, "Operator: " + this->m_op + \
             " is not implemented or malfunctioning.");
     } else {
-        std::cerr << "Operator: " + this->op + \
+        std::cerr << "Operator: " + this->m_op + \
             " is not implemented or malfunctioning.";
     }
 #endif
@@ -183,4 +195,3 @@ Operator *Operator::instantiate(std::string op_string) {
 
 }  // namespace operators
 }  // namespace modsecurity
-

src/operators/operator.h

@@ -30,27 +30,30 @@ class Operator {
  public:
     /** @ingroup ModSecurity_Operator */
     Operator()
-        : op(""),
+        : m_match_message(""),
-        param(""),
+        m_negation(false),
-        negation(false) { }
+        m_op(""),
+        m_param("") { }
     Operator(std::string op, std::string param, bool negation)
-        : op(op),
+        : m_match_message(""),
-        param(param),
+        m_negation(negation),
-        negation(negation) { }
+        m_op(op),
+        m_param(param) { }
 
     virtual ~Operator() { }
-    std::string op;
+    static Operator *instantiate(std::string opName);
-    std::string param;
-    bool negation;
 
-    virtual bool init(const std::string &file, std::string *error) {
+    virtual bool init(const std::string &arg, std::string *error) {
         return true;
     }
 
+    bool evaluateInternal(Transaction *t, const std::string& a);
     virtual bool evaluate(Transaction *transaction, const std::string &str);
-    static Operator *instantiate(std::string op);
 
+    bool m_negation;
     std::string m_match_message;
+    std::string m_op;
+    std::string m_param;
 
  protected:
     bool debug(Transaction *transaction, int x, std::string a);

src/operators/pm.cc

@@ -109,11 +109,11 @@ bool Pm::init(const std::string &file, std::string *error) {
     std::istringstream *iss;
     const char *err = NULL;
 
-    replaceAll(param, "\\", "\\\\");
+    replaceAll(m_param, "\\", "\\\\");
 
-    char *content = parse_pm_content(param.c_str(), param.length(), &err);
+    char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err);
     if (content == NULL) {
-        iss = new std::istringstream(param);
+        iss = new std::istringstream(m_param);
     } else {
         iss = new std::istringstream(content);
     }

src/operators/pm_from_file.cc

@@ -28,20 +28,20 @@ namespace operators {
 bool PmFromFile::init(const std::string &config, std::string *error) {
     std::istream *iss;
 
-    if (param.compare(0, 8, "https://") == 0) {
+    if (m_param.compare(0, 8, "https://") == 0) {
         Utils::HttpsClient client;
-        bool ret = client.download(param);
+        bool ret = client.download(m_param);
         if (ret == false) {
             error->assign(client.error);
             return false;
         }
         iss = new std::stringstream(client.content);
     } else {
-        std::string resource = find_resource(param, config);
+        std::string resource = find_resource(m_param, config);
         iss = new std::ifstream(resource, std::ios::in);
 
         if (((std::ifstream *)iss)->is_open() == false) {
-            error->assign("Failed to open file: " + param);
+            error->assign("Failed to open file: " + m_param);
             delete iss;
             return false;
         }

src/operators/rsub.cc

@@ -33,8 +33,8 @@ bool Rsub::evaluate(Transaction *transaction, const std::string &str) {
 
 Rsub::Rsub(std::string op, std::string param, bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/str_eq.cc

@@ -22,13 +22,9 @@ namespace modsecurity {
 namespace operators {
 
 bool StrEq::evaluate(Transaction *transaction, const std::string &str) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
     bool eq = !p.compare(str);
 
-    if (negation) {
-        return !eq;
-    }
-
     return eq;
 }
 

src/operators/str_match.cc

@@ -25,13 +25,9 @@ namespace operators {
 
 
 bool StrMatch::evaluate(Transaction *transaction, const std::string &input) {
-    std::string p = MacroExpansion::expand(param, transaction);
+    std::string p = MacroExpansion::expand(m_param, transaction);
     bool ret = input.find(p) != std::string::npos;
 
-    if (negation) {
-        return !ret;
-    }
-
     return ret;
 }
 

src/operators/unconditional_match.cc

@@ -22,10 +22,6 @@ bool UnconditionalMatch::evaluate(Transaction *transaction,
     const std::string &input) {
     bool contains = true;
 
-    if (negation) {
-        return !contains;
-    }
-
     return contains;
 }
 

src/operators/validate_byte_range.cc

@@ -85,22 +85,22 @@ bool ValidateByteRange::getRange(const std::string &rangeRepresentation,
 
 bool ValidateByteRange::init(const std::string &file,
     std::string *error) {
-    size_t pos = param.find_first_of(",");
+    size_t pos = m_param.find_first_of(",");
 
     if (pos == std::string::npos) {
-        getRange(param, error);
+        getRange(m_param, error);
     } else {
-        getRange(std::string(param, 0, pos), error);
+        getRange(std::string(m_param, 0, pos), error);
     }
 
     while (pos != std::string::npos) {
-        size_t next_pos = param.find_first_of(",", pos + 1);
+        size_t next_pos = m_param.find_first_of(",", pos + 1);
 
         if (next_pos == std::string::npos) {
-            getRange(std::string(param, pos + 1, param.length() -
+            getRange(std::string(m_param, pos + 1, m_param.length() -
                 (pos + 1)), error);
         } else {
-            getRange(std::string(param, pos + 1, next_pos), error);
+            getRange(std::string(m_param, pos + 1, next_pos), error);
         }
         pos = next_pos;
     }
@@ -128,10 +128,6 @@ bool ValidateByteRange::evaluate(Transaction *transaction,
     // debug(9, "Found %d byte(s) in %s outside range: %s.",
     //     count, var->name, rule->op_param);
 
-    if (negation) {
-        return !ret;
-    }
-
     return ret;
 }
 

src/operators/validate_dtd.cc

@@ -26,9 +26,9 @@ namespace operators {
 
 
 bool ValidateDTD::init(const std::string &file, std::string *error) {
-    m_resource = find_resource(param, file);
+    m_resource = find_resource(m_param, file);
     if (m_resource == "") {
-        error->assign("XML: File not found: " + param + ".");
+        error->assign("XML: File not found: " + m_param + ".");
         return false;
     }
 

src/operators/validate_hash.cc

@@ -33,8 +33,8 @@ bool ValidateHash::evaluate(Transaction *transaction, const std::string &str) {
 
 ValidateHash::ValidateHash(std::string op, std::string param, bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/validate_schema.cc

@@ -26,9 +26,9 @@ namespace modsecurity {
 namespace operators {
 
 bool ValidateSchema::init(const std::string &file, std::string *error) {
-    m_resource = find_resource(param, file);
+    m_resource = find_resource(m_param, file);
     if (m_resource == "") {
-        error->assign("XML: File not found: " + param + ".");
+        error->assign("XML: File not found: " + m_param + ".");
         return false;
     }
 

src/operators/validate_url_encoding.cc

@@ -115,10 +115,6 @@ bool ValidateUrlEncoding::evaluate(Transaction *transaction,
             break;
     }
 
-    if (negation) {
-        return !res;
-    }
-
     return res;
 }
 

src/operators/verify_cc.cc

@@ -93,7 +93,7 @@ bool VerifyCC::init(const std::string &param2, std::string *error) {
     const char *errptr = NULL;
     int erroffset = 0;
 
-    m_pc = pcre_compile(param.c_str(), PCRE_DOTALL|PCRE_MULTILINE,
+    m_pc = pcre_compile(m_param.c_str(), PCRE_DOTALL|PCRE_MULTILINE,
         &errptr, &erroffset, NULL);
     if (m_pc == NULL) {
         error->assign(errptr);
@@ -143,7 +143,7 @@ bool VerifyCC::evaluate(Transaction *transaction, const std::string &i) {
             if (is_cc) {
                 if (transaction) {
 #ifndef NO_LOGS
-                    transaction->debug(9, "CC# match \"" + param +
+                    transaction->debug(9, "CC# match \"" + m_param +
                         "\" at " + i + ". [offset " +
                         std::to_string(offset) + "]");
 #endif
@@ -159,3 +159,4 @@ bool VerifyCC::evaluate(Transaction *transaction, const std::string &i) {
 
 }  // namespace operators
 }  // namespace modsecurity
+

src/operators/verify_cpf.cc

@@ -33,8 +33,8 @@ bool VerifyCPF::evaluate(Transaction *transaction, const std::string &str) {
 
 VerifyCPF::VerifyCPF(std::string op, std::string param, bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/verify_ssn.cc

@@ -32,8 +32,8 @@ bool VerifySSN::evaluate(Transaction *transaction, const std::string &str) {
 
 VerifySSN::VerifySSN(std::string op, std::string param, bool negation)
     : Operator() {
-    this->op = op;
+    this->m_op = op;
-    this->param = param;
+    this->m_param = param;
 }
 
 }  // namespace operators

src/operators/within.cc

@@ -26,7 +26,7 @@ namespace operators {
 
 bool Within::evaluate(Transaction *transaction, const std::string &str) {
     bool res = false;
-    std::string paramTarget = MacroExpansion::expand(param, transaction);
+    std::string paramTarget = MacroExpansion::expand(m_param, transaction);
 
     if (str.empty()) {
         return true;
@@ -34,11 +34,6 @@ bool Within::evaluate(Transaction *transaction, const std::string &str) {
 
     res = paramTarget.find(str) != std::string::npos;
 
-
-    if (negation) {
-        return !res;
-    }
-
     return res;
 }
 

src/rule.cc

@@ -286,16 +286,16 @@ bool Rule::evaluate(Transaction *trasn) {
     ruleMessage = new modsecurity::RuleMessage(this, m_log_message);
 
 #ifndef NO_LOGS
-    std::string eparam = MacroExpansion::expand(this->op->param, trasn);
+    std::string eparam = MacroExpansion::expand(this->op->m_param, trasn);
 
-    if (this->op->param != eparam) {
+    if (this->op->m_param != eparam) {
-        eparam = "\"" + eparam + "\" Was: \"" + this->op->param + "\"";
+        eparam = "\"" + eparam + "\" Was: \"" + this->op->m_param + "\"";
     } else {
         eparam = "\"" + eparam + "\"";
     }
 
     trasn->debug(4, "(Rule: " + std::to_string(rule_id) \
-        + ") Executing operator \"" + this->op->op \
+        + ") Executing operator \"" + this->op->m_op \
         + "\" with param " \
         + eparam \
         + " against " \
@@ -384,7 +384,7 @@ bool Rule::evaluate(Transaction *trasn) {
                 toHexIfNeeded(value)) + "\" (Variable: " + v->m_key + ")");
 #endif
 
-            ret = this->op->evaluate(trasn, value);
+            ret = this->op->evaluateInternal(trasn, value);
 
 #ifndef NO_LOGS
             clock_t end = clock();
@@ -403,8 +403,8 @@ bool Rule::evaluate(Transaction *trasn) {
 
                 if (this->op->m_match_message.empty() == true) {
                     ruleMessage->m_match = "Matched \"Operator `" +
-                        this->op->op + "' with parameter `" +
+                        this->op->m_op + "' with parameter `" +
-                        limitTo(200, this->op->param) +
+                        limitTo(200, this->op->m_param) +
                         "' against variable `" + v->m_key + "' (Value: `" +
                         limitTo(100, toHexIfNeeded(value)) + "' ) \" at " +
                         v->m_key;

test/optimization/optimization.cc

@@ -84,7 +84,7 @@ int main(int argc, char **argv) {
                 continue;
             }
             if (z->op != NULL) {
-                std::string op = z->op->op;
+                std::string op = z->op->m_op;
                 if (operators.count(op) > 0) {
                     operators[op] = 1 + operators[op];
                 } else {