exceptions : normalizing 3 more exceptions (envoyproxy#31088)

moving ProtoValidationException, MissingFieldException OutOfRangeException to EnvoyException

Additional Description:
Risk Level: low
Testing: updated unit tests
Docs Changes: n/a
Release Notes: n/a

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

source/common/protobuf/utility.cc

@@ -114,26 +114,18 @@ uint64_t fractionalPercentDenominatorToInt(
 
 } // namespace ProtobufPercentHelper
 
-MissingFieldException::MissingFieldException(const std::string& message)
-    : EnvoyException(message) {}
-
-ProtoValidationException::ProtoValidationException(const std::string& message)
-    : EnvoyException(message) {
-  ENVOY_LOG_MISC(debug, "Proto validation error; throwing {}", what());
-}
-
 void ProtoExceptionUtil::throwMissingFieldException(const std::string& field_name,
                                                     const Protobuf::Message& message) {
   std::string error =
       fmt::format("Field '{}' is missing in: {}", field_name, message.DebugString());
-  throwExceptionOrPanic(MissingFieldException, error);
+  throwEnvoyExceptionOrPanic(error);
 }
 
 void ProtoExceptionUtil::throwProtoValidationException(const std::string& validation_error,
                                                        const Protobuf::Message& message) {
   std::string error = fmt::format("Proto constraint validation failed ({}): {}", validation_error,
                                   message.DebugString());
-  throwExceptionOrPanic(ProtoValidationException, error);
+  throwEnvoyExceptionOrPanic(error);
 }
 
 size_t MessageUtil::hash(const Protobuf::Message& message) {
@@ -728,7 +720,7 @@ absl::Status validateDurationNoThrow(const ProtobufWkt::Duration& duration,
 void validateDuration(const ProtobufWkt::Duration& duration, int64_t max_seconds_value) {
   const auto result = validateDurationNoThrow(duration, max_seconds_value);
   if (!result.ok()) {
-    throwExceptionOrPanic(DurationUtil::OutOfRangeException, std::string(result.message()));
+    throwEnvoyExceptionOrPanic(std::string(result.message()));
   }
 }
 

source/common/protobuf/utility.h

@@ -24,7 +24,7 @@
   ((message).has_##field_name() ? (message).field_name().value() : (default_value))
 
 // Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, throw
-// a MissingFieldException.
+// a EnvoyException.
 
 #define PROTOBUF_GET_WRAPPED_REQUIRED(message, field_name)                                         \
   ([](const auto& msg) {                                                                           \
@@ -51,8 +51,8 @@
              DurationUtil::durationToMilliseconds((message).field_name()))                         \
        : absl::nullopt)
 
-// Obtain the milliseconds value of a google.protobuf.Duration field if set. Otherwise, throw a
-// MissingFieldException.
+// Obtain the milliseconds value of a google.protobuf.Duration field if set. Otherwise, throw an
+// EnvoyException.
 #define PROTOBUF_GET_MS_REQUIRED(message, field_name)                                              \
   ([](const auto& msg) {                                                                           \
     if (!msg.has_##field_name()) {                                                                 \
@@ -61,8 +61,8 @@
     return DurationUtil::durationToMilliseconds(msg.field_name());                                 \
   }((message)))
 
-// Obtain the seconds value of a google.protobuf.Duration field if set. Otherwise, throw a
-// MissingFieldException.
+// Obtain the seconds value of a google.protobuf.Duration field if set. Otherwise, throw an
+// EnvoyException.
 #define PROTOBUF_GET_SECONDS_REQUIRED(message, field_name)                                         \
   ([](const auto& msg) {                                                                           \
     if (!msg.has_##field_name()) {                                                                 \
@@ -133,11 +133,6 @@ uint64_t fractionalPercentDenominatorToInt(
 
 namespace Envoy {
 
-class MissingFieldException : public EnvoyException {
-public:
-  MissingFieldException(const std::string& message);
-};
-
 class TypeUtil {
 public:
   static absl::string_view typeUrlToDescriptorFullName(absl::string_view type_url);
@@ -203,10 +198,7 @@ class RepeatedPtrUtil {
   }
 };
 
-class ProtoValidationException : public EnvoyException {
-public:
-  ProtoValidationException(const std::string& message);
-};
+using ProtoValidationException = EnvoyException;
 
 /**
  * utility functions to call when throwing exceptions in header files
@@ -274,7 +266,7 @@ class MessageUtil {
    * @param message message to validate.
    * @param validation_visitor the validation visitor to use.
    * @param recurse_into_any whether to recurse into Any messages during unexpected checking.
-   * @throw ProtoValidationException if deprecated fields are used and listed
+   * @throw EnvoyException if deprecated fields are used and listed
    *    in disallowed_features in runtime_features.h
    */
   static void checkForUnexpectedFields(const Protobuf::Message& message,
@@ -294,7 +286,7 @@ class MessageUtil {
    * @param message message to validate.
    * @param validation_visitor the validation visitor to use.
    * @param recurse_into_any whether to recurse into Any messages during unexpected checking.
-   * @throw ProtoValidationException if the message does not satisfy its type constraints.
+   * @throw EnvoyException if the message does not satisfy its type constraints.
    */
   template <class MessageType>
   static void validate(const MessageType& message,
@@ -338,7 +330,7 @@ class MessageUtil {
    * of caller.
    * @param message const Protobuf::Message& to downcast and validate.
    * @return const MessageType& the concrete message type downcasted to on success.
-   * @throw ProtoValidationException if the message does not satisfy its type constraints.
+   * @throw EnvoyException if the message does not satisfy its type constraints.
    */
   template <class MessageType>
   static const MessageType&
@@ -426,7 +418,7 @@ class MessageUtil {
    * @param message source google.protobuf.Any message.
    *
    * @return MessageType the typed message inside the Any.
-   * @throw ProtoValidationException if the message does not satisfy its type constraints.
+   * @throw EnvoyException if the message does not satisfy its type constraints.
    */
   template <class MessageType>
   static inline void anyConvertAndValidate(const ProtobufWkt::Any& message,
@@ -679,18 +671,13 @@ class HashedValue {
 
 class DurationUtil {
 public:
-  class OutOfRangeException : public EnvoyException {
-  public:
-    OutOfRangeException(const std::string& error) : EnvoyException(error) {}
-  };
-
   /**
    * Same as DurationUtil::durationToMilliseconds but with extra validation logic.
    * Same as Protobuf::util::TimeUtil::DurationToSeconds but with extra validation logic.
    * Specifically, we ensure that the duration is positive.
    * @param duration protobuf.
    * @return duration in milliseconds.
-   * @throw OutOfRangeException when duration is out-of-range.
+   * @throw EnvoyException when duration is out-of-range.
    */
   static uint64_t durationToMilliseconds(const ProtobufWkt::Duration& duration);
 
@@ -707,7 +694,7 @@ class DurationUtil {
    * Specifically, we ensure that the duration is positive.
    * @param duration protobuf.
    * @return duration in seconds.
-   * @throw OutOfRangeException when duration is out-of-range.
+   * @throw EnvoyException when duration is out-of-range.
    */
   static uint64_t durationToSeconds(const ProtobufWkt::Duration& duration);
 };

source/common/router/scoped_config_impl.cc

@@ -18,10 +18,6 @@ bool ScopeKey::operator==(const ScopeKey& other) const {
   return this->hash() == other.hash();
 }
 
-void throwProtoValidationExceptionOrPanic(std::string message) {
-  throwExceptionOrPanic(ProtoValidationException, message);
-}
-
 HeaderValueExtractorImpl::HeaderValueExtractorImpl(
     ScopedRoutes::ScopeKeyBuilder::FragmentBuilder&& config)
     : FragmentBuilderBase(std::move(config)),
@@ -33,12 +29,14 @@ HeaderValueExtractorImpl::HeaderValueExtractorImpl(
       ScopedRoutes::ScopeKeyBuilder::FragmentBuilder::HeaderValueExtractor::kIndex) {
     if (header_value_extractor_config_.index() != 0 &&
         header_value_extractor_config_.element_separator().empty()) {
-      throwProtoValidationExceptionOrPanic("Index > 0 for empty string element separator.");
+      ProtoExceptionUtil::throwProtoValidationException(
+          "Index > 0 for empty string element separator.", config_);
     }
   }
   if (header_value_extractor_config_.extract_type_case() ==
       ScopedRoutes::ScopeKeyBuilder::FragmentBuilder::HeaderValueExtractor::EXTRACT_TYPE_NOT_SET) {
-    throwProtoValidationExceptionOrPanic("HeaderValueExtractor extract_type not set.");
+    ProtoExceptionUtil::throwProtoValidationException("HeaderValueExtractor extract_type not set.",
+                                                      config_);
   }
 }
 

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -594,11 +594,8 @@ using FuzzStreamPtr = std::unique_ptr<FuzzStream>;
 DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) {
   try {
     TestUtility::validate(input);
-  } catch (const ProtoValidationException& e) {
-    ENVOY_LOG_MISC(debug, "ProtoValidationException: {}", e.what());
-    return;
-  } catch (const Envoy::ProtobufMessage::DeprecatedProtoFieldException& e) {
-    ENVOY_LOG_MISC(debug, "DeprecatedProtoFieldException: {}", e.what());
+  } catch (const Envoy::EnvoyException& e) {
+    ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what());
     return;
   }
 

test/common/protobuf/utility_test.cc

@@ -1682,29 +1682,29 @@ TEST(DurationUtilTest, OutOfRange) {
   {
     ProtobufWkt::Duration duration;
     duration.set_seconds(-1);
-    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), DurationUtil::OutOfRangeException);
+    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), EnvoyException);
   }
   {
     ProtobufWkt::Duration duration;
     duration.set_nanos(-1);
-    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), DurationUtil::OutOfRangeException);
+    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), EnvoyException);
   }
   {
     ProtobufWkt::Duration duration;
     duration.set_nanos(1000000000);
-    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), DurationUtil::OutOfRangeException);
+    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), EnvoyException);
   }
   {
     ProtobufWkt::Duration duration;
     duration.set_seconds(Protobuf::util::TimeUtil::kDurationMaxSeconds + 1);
-    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), DurationUtil::OutOfRangeException);
+    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), EnvoyException);
   }
   {
     ProtobufWkt::Duration duration;
     constexpr int64_t kMaxInt64Nanoseconds =
         std::numeric_limits<int64_t>::max() / (1000 * 1000 * 1000);
     duration.set_seconds(kMaxInt64Nanoseconds + 1);
-    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), DurationUtil::OutOfRangeException);
+    EXPECT_THROW(DurationUtil::durationToMilliseconds(duration), EnvoyException);
   }
 }
 
@@ -1869,7 +1869,7 @@ TEST_F(DeprecatedFieldsTest, IndividualFieldDeprecatedEmitsCrash) {
       {"envoy.features.fail_on_any_deprecated_feature", "true"},
   });
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'");
   EXPECT_EQ(0, runtime_deprecated_feature_use_.value());
   EXPECT_EQ(0, deprecated_feature_seen_since_process_start_.value());
@@ -1880,7 +1880,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(IndividualFieldDisallowed))
   envoy::test::deprecation_test::Base base;
   base.set_is_deprecated_fatal("foo");
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated_fatal'");
 }
 
@@ -1891,7 +1891,7 @@ TEST_F(DeprecatedFieldsTest,
 
   // Make sure this is set up right.
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated_fatal'");
   // The config will be rejected, so the feature will not be used.
 
@@ -1914,7 +1914,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(IndividualFieldDisallowedWi
 
   // Make sure this is set up right.
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated_fatal'");
   // The config will be rejected, so the feature will not be used.
 
@@ -1942,15 +1942,15 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) {
       {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated", " false"}});
 
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'");
 
   // Verify that even when the enable_all_deprecated_features is enabled the
   // feature is disallowed.
   mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
   EXPECT_THROW_WITH_REGEX(
-      checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+      checkForDeprecation(base), Envoy::EnvoyException,
       "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'");
 }
 
@@ -1964,7 +1964,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(MixOfFatalAndWarnings)) {
   EXPECT_LOG_CONTAINS(
       "warning", "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated'", {
         EXPECT_THROW_WITH_REGEX(
-            checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+            checkForDeprecation(base), Envoy::EnvoyException,
             "Using deprecated option 'envoy.test.deprecation_test.Base.is_deprecated_fatal'");
       });
 }
@@ -2056,16 +2056,14 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault)
       {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_DEFAULT", "false"}});
 
   // Make sure this is set up right.
-  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base),
-                          Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), Envoy::EnvoyException,
                           "Using the default now-deprecated value DEPRECATED_DEFAULT");
 
   // Verify that even when the enable_all_deprecated_features is enabled the
   // enum is disallowed.
   mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
-  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base),
-                          Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), Envoy::EnvoyException,
                           "Using the default now-deprecated value DEPRECATED_DEFAULT");
 }
 
@@ -2074,8 +2072,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnum)) {
   envoy::test::deprecation_test::Base base;
   base.mutable_enum_container()->set_deprecated_enum(
       envoy::test::deprecation_test::Base::DEPRECATED_FATAL);
-  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base),
-                          Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), Envoy::EnvoyException,
                           "Using deprecated value DEPRECATED_FATAL");
 
   mergeValues(
@@ -2095,8 +2092,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnumGlobalOverride)) {
   envoy::test::deprecation_test::Base base;
   base.mutable_enum_container()->set_deprecated_enum(
       envoy::test::deprecation_test::Base::DEPRECATED_FATAL);
-  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base),
-                          Envoy::ProtobufMessage::DeprecatedProtoFieldException,
+  EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), Envoy::EnvoyException,
                           "Using deprecated value DEPRECATED_FATAL");
 
   mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});

test/extensions/filters/network/common/fuzz/uber_per_readfilter.cc

@@ -139,15 +139,15 @@ void UberFilterFuzzer::checkInvalidInputForFuzzer(const std::string& filter_name
       // Sanity check on connection_keepalive interval and timeout.
       try {
         PROTOBUF_GET_MS_REQUIRED(config.http2_protocol_options().connection_keepalive(), interval);
-      } catch (const DurationUtil::OutOfRangeException& e) {
+      } catch (const EnvoyException& e) {
         throw EnvoyException(
             absl::StrCat("In http2_protocol_options.connection_keepalive interval shall not be "
                          "negative. Exception {}",
                          e.what()));
       }
       try {
         PROTOBUF_GET_MS_REQUIRED(config.http2_protocol_options().connection_keepalive(), timeout);
-      } catch (const DurationUtil::OutOfRangeException& e) {
+      } catch (const EnvoyException& e) {
         throw EnvoyException(
             absl::StrCat("In http2_protocol_options.connection_keepalive timeout shall not be "
                          "negative. Exception {}",

test/extensions/filters/network/local_ratelimit/local_ratelimit_fuzz_test.cc

@@ -35,19 +35,16 @@ DEFINE_PROTO_FUZZER(
 
   try {
     TestUtility::validate(input);
-  } catch (const ProtoValidationException& e) {
-    ENVOY_LOG_MISC(debug, "ProtoValidationException: {}", e.what());
-    return;
-  } catch (const ProtobufMessage::DeprecatedProtoFieldException& e) {
-    ENVOY_LOG_MISC(debug, "DeprecatedProtoFieldException: {}", e.what());
+  } catch (const EnvoyException& e) {
+    ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what());
     return;
   }
   try {
     if (PROTOBUF_GET_MS_REQUIRED(input.config().token_bucket(), fill_interval) < 50) {
       ENVOY_LOG_MISC(debug, "In fill_interval, msecs must be greater than 50ms!");
       return;
     }
-  } catch (const DurationUtil::OutOfRangeException& e) {
+  } catch (const EnvoyException& e) {
     // TODO:
     // protoc-gen-validate has an issue on type "Duration" which may generate interval with seconds
     // > 0 while "nanos" < 0. And negative "nanos" will cause validation inside the filter to fail.

test/per_file_coverage.sh

@@ -15,7 +15,7 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/common/matcher:94.6"
 "source/common/network:94.4" # Flaky, `activateFileEvents`, `startSecureTransport` and `ioctl`, listener_socket do not always report LCOV
 "source/common/network/dns_resolver:91.4"  # A few lines of MacOS code not tested in linux scripts. Tested in MacOS scripts
-"source/common/protobuf:96.5"
+"source/common/protobuf:96.4"
 "source/common/quic:93.6"
 "source/common/secret:95.1"
 "source/common/signal:87.2" # Death tests don't report LCOV

test/tools/schema_validator/validator.cc

@@ -83,8 +83,7 @@ class Visitor : public ProtobufMessage::ValidationVisitor {
   bool skipValidation() override { return false; }
   void onDeprecatedField(absl::string_view description, bool) override {
     if (options_.failOnDeprecated()) {
-      throw ProtobufMessage::DeprecatedProtoFieldException(
-          absl::StrCat("Failing due to deprecated field: ", description));
+      throw EnvoyException(absl::StrCat("Failing due to deprecated field: ", description));
     }
   }
   void onWorkInProgress(absl::string_view description) override {