Regex: removing some exceptions (#33853)

regex: removing exceptions from cc file

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

contrib/hyperscan/matching/input_matchers/test/matcher_speed_test.cc

@@ -30,11 +30,11 @@ static void BM_CompiledGoogleReMatcher(benchmark::State& state) {
   envoy::type::matcher::v3::RegexMatcher config;
   config.mutable_google_re2();
   config.set_regex(std::string(ClusterRePattern));
-  const auto matcher = Regex::CompiledGoogleReMatcher(config);
+  const auto matcher = Regex::CompiledGoogleReMatcher::create(config).value();
   uint32_t passes = 0;
   for (auto _ : state) { // NOLINT
     for (const std::string& cluster_input : clusterInputs()) {
-      if (matcher.match(cluster_input)) {
+      if (matcher->match(cluster_input)) {
         ++passes;
       }
     }

contrib/hyperscan/regex_engines/source/regex.cc

@@ -8,7 +8,8 @@ namespace Hyperscan {
 HyperscanEngine::HyperscanEngine(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls)
     : dispatcher_(dispatcher), tls_(tls) {}
 
-Envoy::Regex::CompiledMatcherPtr HyperscanEngine::matcher(const std::string& regex) const {
+absl::StatusOr<Envoy::Regex::CompiledMatcherPtr>
+HyperscanEngine::matcher(const std::string& regex) const {
   std::vector<const char*> expressions{regex.c_str()};
   std::vector<unsigned int> flags{HS_FLAG_UTF8};
   std::vector<unsigned int> ids{0};

contrib/hyperscan/regex_engines/source/regex.h

@@ -12,7 +12,7 @@ namespace Hyperscan {
 class HyperscanEngine : public Envoy::Regex::Engine {
 public:
   explicit HyperscanEngine(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls);
-  Envoy::Regex::CompiledMatcherPtr matcher(const std::string& regex) const override;
+  absl::StatusOr<Envoy::Regex::CompiledMatcherPtr> matcher(const std::string& regex) const override;
 
 private:
   Event::Dispatcher& dispatcher_;

contrib/hyperscan/regex_engines/test/config_test.cc

@@ -31,7 +31,7 @@ TEST_F(ConfigTest, IncompatibleArchitecture) {
 TEST_F(ConfigTest, Regex) {
   setup();
 
-  Envoy::Regex::CompiledMatcherPtr matcher = engine_->matcher("^/asdf/.+");
+  Envoy::Regex::CompiledMatcherPtr matcher = engine_->matcher("^/asdf/.+").value();
 
   EXPECT_TRUE(matcher->match("/asdf/1"));
   EXPECT_FALSE(matcher->match("/ASDF/1"));

contrib/hyperscan/regex_engines/test/regex_test.cc

@@ -30,7 +30,7 @@ class EngineTest : public ::testing::Test {
 TEST_F(EngineTest, Matcher) {
   setup();
 
-  EXPECT_NO_THROW(engine_->matcher("^/asdf/.+"));
+  EXPECT_TRUE(engine_->matcher("^/asdf/.+").status().ok());
 }
 
 } // namespace Hyperscan

envoy/common/regex.h

@@ -37,7 +37,7 @@ class Engine {
    * Create a @ref CompiledMatcher with the given regex expression.
    * @param regex the regex expression match string
    */
-  virtual CompiledMatcherPtr matcher(const std::string& regex) const PURE;
+  virtual absl::StatusOr<CompiledMatcherPtr> matcher(const std::string& regex) const PURE;
 };
 
 using EnginePtr = std::shared_ptr<Engine>;

source/common/common/regex.cc

@@ -13,19 +13,49 @@
 namespace Envoy {
 namespace Regex {
 
+absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+CompiledGoogleReMatcher::createAndSizeCheck(const std::string& regex) {
+  absl::Status creation_status = absl::OkStatus();
+  auto ret = std::unique_ptr<CompiledGoogleReMatcher>(
+      new CompiledGoogleReMatcher(regex, creation_status, true));
+  RETURN_IF_NOT_OK(creation_status);
+  return ret;
+}
+
+absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+CompiledGoogleReMatcher::create(const envoy::type::matcher::v3::RegexMatcher& config) {
+  absl::Status creation_status = absl::OkStatus();
+  auto ret = std::unique_ptr<CompiledGoogleReMatcher>(
+      new CompiledGoogleReMatcher(config, creation_status));
+  RETURN_IF_NOT_OK(creation_status);
+  return ret;
+}
+
+absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+CompiledGoogleReMatcher::create(const xds::type::matcher::v3::RegexMatcher& config) {
+  absl::Status creation_status = absl::OkStatus();
+  auto ret = std::unique_ptr<CompiledGoogleReMatcher>(
+      new CompiledGoogleReMatcher(config, creation_status));
+  RETURN_IF_NOT_OK(creation_status);
+  return ret;
+}
+
+REGISTER_FACTORY(GoogleReEngineFactory, EngineFactory);
 CompiledGoogleReMatcher::CompiledGoogleReMatcher(const std::string& regex,
+                                                 absl::Status& creation_status,
                                                  bool do_program_size_check)
     : regex_(regex, re2::RE2::Quiet) {
   if (!regex_.ok()) {
-    throwEnvoyExceptionOrPanic(regex_.error());
+    creation_status = absl::InvalidArgumentError(regex_.error());
+    return;
   }
 
   if (do_program_size_check && Runtime::isRuntimeInitialized()) {
     const uint32_t regex_program_size = static_cast<uint32_t>(regex_.ProgramSize());
     const uint32_t max_program_size_error_level =
         Runtime::getInteger("re2.max_program_size.error_level", 100);
     if (regex_program_size > max_program_size_error_level) {
-      throwEnvoyExceptionOrPanic(
+      creation_status = absl::InvalidArgumentError(
           fmt::format("regex '{}' RE2 program size of {} > max program size of "
                       "{} set for the error level threshold. Increase "
                       "configured max program size if necessary.",
@@ -44,25 +74,26 @@ CompiledGoogleReMatcher::CompiledGoogleReMatcher(const std::string& regex,
 }
 
 CompiledGoogleReMatcher::CompiledGoogleReMatcher(
-    const envoy::type::matcher::v3::RegexMatcher& config)
-    : CompiledGoogleReMatcher(config.regex(), !config.google_re2().has_max_program_size()) {
+    const envoy::type::matcher::v3::RegexMatcher& config, absl::Status& creation_status)
+    : CompiledGoogleReMatcher(config.regex(), creation_status,
+                              !config.google_re2().has_max_program_size()) {
   const uint32_t regex_program_size = static_cast<uint32_t>(regex_.ProgramSize());
 
   // Check if the deprecated field max_program_size is set first, and follow the old logic if so.
   if (config.google_re2().has_max_program_size()) {
     const uint32_t max_program_size =
         PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.google_re2(), max_program_size, 100);
     if (regex_program_size > max_program_size) {
-      throwEnvoyExceptionOrPanic(
+      creation_status = absl::InvalidArgumentError(
           fmt::format("regex '{}' RE2 program size of {} > max program size of "
                       "{}. Increase configured max program size if necessary.",
                       config.regex(), regex_program_size, max_program_size));
     }
   }
 }
 
-CompiledMatcherPtr GoogleReEngine::matcher(const std::string& regex) const {
-  return std::make_unique<CompiledGoogleReMatcher>(regex, true);
+absl::StatusOr<CompiledMatcherPtr> GoogleReEngine::matcher(const std::string& regex) const {
+  return CompiledGoogleReMatcher::createAndSizeCheck(regex);
 }
 
 EnginePtr GoogleReEngineFactory::createEngine(const Protobuf::Message&,
@@ -74,7 +105,5 @@ ProtobufTypes::MessagePtr GoogleReEngineFactory::createEmptyConfigProto() {
   return std::make_unique<envoy::extensions::regex_engines::v3::GoogleRE2>();
 }
 
-REGISTER_FACTORY(GoogleReEngineFactory, EngineFactory);
-
 } // namespace Regex
 } // namespace Envoy

source/common/common/regex.h

@@ -20,12 +20,13 @@ namespace Regex {
 
 class CompiledGoogleReMatcher : public CompiledMatcher {
 public:
-  explicit CompiledGoogleReMatcher(const std::string& regex, bool do_program_size_check);
-
-  explicit CompiledGoogleReMatcher(const xds::type::matcher::v3::RegexMatcher& config)
-      : CompiledGoogleReMatcher(config.regex(), false) {}
-
-  explicit CompiledGoogleReMatcher(const envoy::type::matcher::v3::RegexMatcher& config);
+  // Create, applying re2.max_program_size.error_level and re2.max_program_size.warn_level.
+  static absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+  createAndSizeCheck(const std::string& regex);
+  static absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+  create(const envoy::type::matcher::v3::RegexMatcher& config);
+  static absl::StatusOr<std::unique_ptr<CompiledGoogleReMatcher>>
+  create(const xds::type::matcher::v3::RegexMatcher& config);
 
   // CompiledMatcher
   bool match(absl::string_view value) const override { return re2::RE2::FullMatch(value, regex_); }
@@ -37,13 +38,32 @@ class CompiledGoogleReMatcher : public CompiledMatcher {
     return result;
   }
 
-private:
+protected:
+  explicit CompiledGoogleReMatcher(const std::string& regex) : regex_(regex, re2::RE2::Quiet) {
+    ENVOY_BUG(regex_.ok(), "Invalid regex");
+  }
+  explicit CompiledGoogleReMatcher(const std::string& regex, absl::Status& creation_status,
+                                   bool do_program_size_check);
+  explicit CompiledGoogleReMatcher(const envoy::type::matcher::v3::RegexMatcher& config,
+                                   absl::Status& creation_status);
+  explicit CompiledGoogleReMatcher(const xds::type::matcher::v3::RegexMatcher& config,
+                                   absl::Status& creation_status)
+      : CompiledGoogleReMatcher(config.regex(), creation_status, false) {}
+
   const re2::RE2 regex_;
 };
 
+// Allow creating CompiledGoogleReMatcher without checking for status failures
+// for call sites which really really want to.
+class CompiledGoogleReMatcherNoSafetyChecks : public CompiledGoogleReMatcher {
+public:
+  CompiledGoogleReMatcherNoSafetyChecks(const std::string& regex)
+      : CompiledGoogleReMatcher(regex) {}
+};
+
 class GoogleReEngine : public Engine {
 public:
-  CompiledMatcherPtr matcher(const std::string& regex) const override;
+  absl::StatusOr<CompiledMatcherPtr> matcher(const std::string& regex) const override;
 };
 
 class GoogleReEngineFactory : public EngineFactory {
@@ -69,10 +89,11 @@ class Utility {
   static CompiledMatcherPtr parseRegex(const RegexMatcherType& matcher, Engine& engine) {
     // Fallback deprecated engine type in regex matcher.
     if (matcher.has_google_re2()) {
-      return std::make_unique<CompiledGoogleReMatcher>(matcher);
+      return THROW_OR_RETURN_VALUE(CompiledGoogleReMatcher::create(matcher),
+                                   std::unique_ptr<CompiledGoogleReMatcher>);
     }
 
-    return engine.matcher(matcher.regex());
+    return THROW_OR_RETURN_VALUE(engine.matcher(matcher.regex()), CompiledMatcherPtr);
   }
 };
 

source/common/config/well_known_names.cc

@@ -31,8 +31,8 @@ std::string expandRegex(const std::string& regex) {
 }
 
 const Regex::CompiledGoogleReMatcher& validTagValueRegex() {
-  CONSTRUCT_ON_FIRST_USE(Regex::CompiledGoogleReMatcher, absl::StrCat("^", TAG_VALUE_REGEX, "$"),
-                         false);
+  CONSTRUCT_ON_FIRST_USE(Regex::CompiledGoogleReMatcherNoSafetyChecks,
+                         absl::StrCat("^", TAG_VALUE_REGEX, "$"));
 }
 
 } // namespace

source/server/admin/prometheus_stats.cc

@@ -17,7 +17,7 @@ namespace Server {
 namespace {
 
 const Regex::CompiledGoogleReMatcher& promRegex() {
-  CONSTRUCT_ON_FIRST_USE(Regex::CompiledGoogleReMatcher, "[^a-zA-Z0-9_]", false);
+  CONSTRUCT_ON_FIRST_USE(Regex::CompiledGoogleReMatcherNoSafetyChecks, "[^a-zA-Z0-9_]");
 }
 
 /**

tools/code_format/config.yaml

@@ -137,7 +137,7 @@ paths:
     - source/common/filesystem/kqueue/watcher_impl.cc
     - source/common/filesystem/win32/directory_iterator_impl.cc
     - source/common/common/utility.cc
-    - source/common/common/regex.cc
+    - source/common/common/regex.h
     - source/common/common/matchers.cc
     - source/exe/stripped_main_base.cc
     - source/server/options_impl.cc