Adds reference to filename and line number to lexer errors

owasp-modsecurity · Sep 30, 2015 · 8255ce8 · 8255ce8
1 parent 900af2c
commit 8255ce8
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 12 deletions.
diff --git a/src/actions/phase.cc b/src/actions/phase.cc
@@ -68,6 +68,16 @@ Phase::Phase(std::string action)
     }
 }
 
+
+bool Phase::init(std::string *error) {
+    if (phase >= ModSecurity::Phases::NUMBER_OF_PHASES) {
+        error->assign("Unknown phase: " + std::to_string(phase));
+        return false;
+    }
+    return true;
+}
+
+
 bool Phase::evaluate(Rule *rule, Assay *assay) {
     rule->phase = this->phase;
     return true;

diff --git a/src/actions/phase.h b/src/actions/phase.h
@@ -34,6 +34,7 @@ class Phase : public Action {
  public:
     explicit Phase(std::string action);
 
+    bool init(std::string *error) override;
     bool evaluate(Rule *rule, Assay *assay) override;
     int phase;
     int m_secRulesPhase;

diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy
@@ -224,6 +224,7 @@ using ModSecurity::Variables::Variable;
 %token <std::string> ACTION_REDIRECT
 %token <std::string> ACTION_SKIP_AFTER
 %token <std::string> ACTION_AUDIT_LOG
+%token <std::string> ACTION_PHASE
 %token <std::string> ACTION_SEVERITY
 %token <std::string> ACTION_SETVAR
 %token <std::string> ACTION_EXPIREVAR
@@ -343,7 +344,7 @@ expression:
         Operator *op = Operator::instantiate($5);
         const char *error = NULL;
         if (op->init(&error) == false) {
-            driver.parserError << error;
+            driver.error(@0, error);
             YYERROR;
         }
         Rule *rule = new Rule(
@@ -362,15 +363,18 @@ expression:
         Operator *op = Operator::instantiate("\"@rx " + $5 + "\"");
         const char *error = NULL;
         if (op->init(&error) == false) {
-            driver.parserError << error;
+            driver.error(@0, error);
             YYERROR;
         }
         Rule *rule = new Rule(
             /* op */ op,
             /* variables */ $3,
             /* actions */ $8
             );
-        driver.addSecRule(rule);
+
+        if (driver.addSecRule(rule) == false) {
+            YYERROR;
+        }
       }
     | CONFIG_DIR_SEC_ACTION SPACE QUOTATION_MARK actions QUOTATION_MARK
       {
@@ -405,12 +409,12 @@ expression:
                 a->action_kind == Action::RunTimeBeforeMatchAttemptKind) {
                 None *none = dynamic_cast<None *>(a);
                 if (none != NULL) {
-                    driver.parserError << "The transformation none is not suitable to be part of the SecDefaultActions";
+                    driver.error(@0, "The transformation none is not suitable to be part of the SecDefaultActions");
                     YYERROR;
                 }
                 checkedActions.push_back(a);
             } else {
-                driver.parserError << "The action '" << a->action << "' is not suitable to be part of the SecDefaultActions";
+                driver.error(@0, "The action '" + a->action + "' is not suitable to be part of the SecDefaultActions");
                 YYERROR;
             }
         }
@@ -419,7 +423,11 @@ expression:
         }
 
         if (!driver.defaultActions[definedPhase].empty()) {
-            driver.parserError << "SecDefaultActions can only be placed once per phase and configuration context. Phase " << secRuleDefinedPhase << " was informed already.";
+            std::stringstream ss;
+            ss << "SecDefaultActions can only be placed once per phase and configuration context. Phase ";
+            ss << secRuleDefinedPhase;
+            ss << " was informed already.";
+            driver.error(@0, ss.str());
             YYERROR;
         }
 
@@ -469,8 +477,10 @@ expression:
         if (driver.m_debugLog != NULL) {
           driver.m_debugLog->setDebugLogLevel(atoi($1.c_str()));
         } else {
-            driver.parserError << "Internal error, there is no DebugLog ";
-            driver.parserError << "object associated with the driver class";
+            std::stringstream ss;
+            ss << "Internal error, there is no DebugLog ";
+            ss << "object associated with the driver class";
+            driver.error(@0, ss.str());
             YYERROR;
         }
       }
@@ -479,8 +489,10 @@ expression:
         if (driver.m_debugLog != NULL) {
             driver.m_debugLog->setDebugLogFile($1);
         } else {
-            driver.parserError << "Internal error, there is no DebugLog ";
-            driver.parserError << "object associated with the driver class";
+            std::stringstream ss;
+            ss << "Internal error, there is no DebugLog ";
+            ss << "object associated with the driver class";
+            driver.error(@0, ss.str());
             YYERROR;
         }
       }
@@ -699,6 +711,15 @@ act:
             YYERROR;
         }
       }
+    | ACTION_PHASE
+      {
+        std::string error;
+        $$ = new Phase($1);
+        if ($$->init(&error) == false) {
+            driver.error(@0, error);
+            YYERROR;
+        }
+      }
     | ACTION_INITCOL
       {
         $$ = Action::instantiate($1);

diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll
@@ -272,7 +272,7 @@ CONFIG_DIR_UNICODE_MAP_FILE (?i:SecUnicodeMapFile)
 }
 
 {ACTION}                        { return yy::seclang_parser::make_ACTION(yytext, *driver.loc.back()); }
-{ACTION_PHASE}                  { return yy::seclang_parser::make_ACTION(yytext, *driver.loc.back()); }
+{ACTION_PHASE}                  { return yy::seclang_parser::make_ACTION_PHASE(yytext, *driver.loc.back()); }
 {ACTION_SKIP_AFTER}:{FREE_TEXT} { return yy::seclang_parser::make_ACTION_SKIP_AFTER(strchr(yytext, ':') + 1, *driver.loc.back()); }
 {ACTION_AUDIT_LOG}              { return yy::seclang_parser::make_ACTION_AUDIT_LOG(yytext, *driver.loc.back()); }
 
@@ -398,7 +398,7 @@ CONFIG_DIR_UNICODE_MAP_FILE (?i:SecUnicodeMapFile)
             driver.error (*driver.loc.back(), "", s + std::string(": Not able to open file."));
             throw yy::seclang_parser::syntax_error(*driver.loc.back(), "");
         }
-        driver.ref.push_back(file);
+        driver.ref.push_back(s.c_str());
         driver.loc.push_back(new yy::location());
         yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE ));
 

diff --git a/test/benchmark/basic_rules.conf b/test/benchmark/basic_rules.conf
@@ -1,2 +1,3 @@
 
 include "owasp-modsecurity-crs-orig/modsecurity_crs_10_setup.conf"
+include "owasp-modsecurity-crs-orig/rules/*.conf"