diff options
author | David Neto <dneto@google.com> | 2019-08-24 01:48:08 -0400 |
---|---|---|
committer | David Neto <dneto@google.com> | 2019-08-24 02:07:25 -0400 |
commit | 0eb6499adf7a4debb3cdc87f9c469b45f8d65b5a (patch) | |
tree | 1614b9ec54392c9b49d35d860cdb69c0d87540b5 | |
parent | ecbc165b47c95ddc07037680c2271190cbc3b86d (diff) | |
download | effcee-0eb6499adf7a4debb3cdc87f9c469b45f8d65b5a.tar.gz |
Fail parsing checks if the regexp is bad.
BUG=129514492
-rw-r--r-- | effcee/check.cc | 21 | ||||
-rw-r--r-- | effcee/check_test.cc | 7 |
2 files changed, 21 insertions, 7 deletions
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 @@ bool Check::Matches(StringPiece* input, StringPiece* captured, } 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 @@ Check::Parts PartsForPattern(StringPiece pattern) { } 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 @@ Check::Parts PartsForPattern(StringPiece pattern) { } } - return parts; + return std::make_pair(Result(Result::Status::Ok), std::move(parts)); } } // namespace @@ -234,8 +240,9 @@ std::pair<Result, CheckList> ParseChecks(StringPiece str, 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(); } 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 @@ INSTANTIATE_TEST_SUITE_P(AllCheckTypes, ParseChecksTypeFailTest, "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)); |