commit 0eb6499adf7a4debb3cdc87f9c469b45f8d65b5a
author David Neto <dneto@google.com> Sat Aug 24 01:48:08 2019 -0400
committer David Neto <dneto@google.com> Sat Aug 24 02:07:25 2019 -0400
tree 1614b9ec54392c9b49d35d860cdb69c0d87540b5
parent ecbc165b47c95ddc07037680c2271190cbc3b86d

Fail parsing checks if the regexp is bad.

BUG=129514492

effcee/check.cc

diff --git a/effcee/check.cc b/effcee/check.cc
index 2751801..08f7b7a 100644
--- a/effcee/check.cc
+++ b/effcee/check.cc
@@ -130,10 +130,10 @@
 }
 
 namespace {
-// Returns a parts list for the given pattern.  This splits out regular
-// expressions as delimited by {{ and }}, and also variable uses and
-// definitions.
-Check::Parts PartsForPattern(StringPiece pattern) {
+// Returns a Result and a parts list for the given pattern.  This splits out
+// regular expressions as delimited by {{ and }}, and also variable uses and
+// definitions.  This can fail when a regular expression is invalid.
+std::pair<Result, Check::Parts> PartsForPattern(StringPiece pattern) {
   Check::Parts parts;
   StringPiece fixed, regex, var;
 
@@ -160,6 +160,12 @@
       }
       if (!regex.empty()) {
         parts.emplace_back(make_unique<Check::Part>(Type::Regex, regex));
+        if (parts.back()->NumCapturingGroups() < 0) {
+          return std::make_pair(
+              Result(Result::Status::BadRule,
+                     std::string("invalid regex: ") + ToString(regex)),
+              Check::Parts());
+        }
       }
     } else if (var_exists && (!regex_exists || var_start < regex_start)) {
       const auto consumed =
@@ -191,7 +197,7 @@
     }
   }
 
-  return parts;
+  return std::make_pair(Result(Result::Status::Ok), std::move(parts));
 }
 
 }  // namespace
@@ -234,8 +240,9 @@
     StringPiece suffix;
     if (RE2::PartialMatch(line, regexp, &suffix, &matched_param)) {
       const Type type = TypeForSuffix(suffix);
-      auto parts(PartsForPattern(matched_param));
-      check_list.push_back(Check(type, matched_param, std::move(parts)));
+      auto parts = PartsForPattern(matched_param);
+      if (!parts.first) return std::make_pair(parts.first, CheckList());
+      check_list.push_back(Check(type, matched_param, std::move(parts.second)));
     }
     cursor.AdvanceLine();
   }

effcee/check_test.cc

diff --git a/effcee/check_test.cc b/effcee/check_test.cc
index c9d0674..f7089ef 100644
--- a/effcee/check_test.cc
+++ b/effcee/check_test.cc
@@ -246,6 +246,13 @@
                                                                    "FOO"}),
                                  ValuesIn(AllCheckTypesAsPairs())));
 
+TEST(ParseChecks, BadRegexpFails) {
+  const auto parsed = ParseChecks("CHECK: {{\\}}", Options());
+  EXPECT_THAT(parsed.first.status(), Eq(Status::BadRule));
+  EXPECT_THAT(parsed.first.message(), HasSubstr("invalid regex: \\"));
+  EXPECT_THAT(parsed.second, Eq(CheckList({})));
+}
+
 TEST(ParseChecks, CheckSameCantBeFirst) {
   const auto parsed = ParseChecks("CHECK-SAME: now", Options());
   EXPECT_THAT(parsed.first.status(), Eq(Status::BadRule));