[filter state]: make jsonConvert no throw

source/common/formatter/substitution_formatter.cc

@@ -1335,14 +1335,10 @@ ProtobufWkt::Value FilterStateFormatter::formatValue(const Http::RequestHeaderMa
   }
 
   ProtobufWkt::Value val;
-  // TODO(chaoqin-li1123): make this conversion return an error status instead of throwing.
-  // Access logger conversion from protobufs occurs via json intermediate state, which can throw
-  // when converting that to a structure.
-  TRY_NEEDS_AUDIT { MessageUtil::jsonConvertValue(*proto, val); }
-  catch (EnvoyException& ex) {
-    return unspecifiedValue();
+  if (MessageUtil::jsonConvertValue(*proto, val)) {
+    return val;
   }
-  return val;
+  return unspecifiedValue();
 }
 
 // Given a token, extract the command string between parenthesis if it exists.

source/common/protobuf/utility.cc

@@ -240,6 +240,25 @@ size_t MessageUtil::hash(const Protobuf::Message& message) {
 
 void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message,
                                ProtobufMessage::ValidationVisitor& validation_visitor) {
+  bool has_unknown_field;
+  auto status = loadFromJsonNoThrow(json, message, has_unknown_field);
+  if (status.ok()) {
+    return;
+  }
+  if (has_unknown_field) {
+    // If the parsing failure is caused by the unknown fields.
+    validation_visitor.onUnknownField("type " + message.GetTypeName() + " reason " +
+                                      status.ToString());
+  } else {
+    // If the error has nothing to do with unknown field.
+    throw EnvoyException("Unable to parse JSON as proto (" + status.ToString() + "): " + json);
+  }
+}
+
+Protobuf::util::Status MessageUtil::loadFromJsonNoThrow(const std::string& json,
+                                                        Protobuf::Message& message,
+                                                        bool& has_unknown_fileld) {
+  has_unknown_fileld = false;
   Protobuf::util::JsonParseOptions options;
   options.case_insensitive_enum_parsing = true;
   // Let's first try and get a clean parse when checking for unknown fields;
@@ -248,7 +267,7 @@ void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& messa
   const auto strict_status = Protobuf::util::JsonStringToMessage(json, &message, options);
   if (strict_status.ok()) {
     // Success, no need to do any extra work.
-    return;
+    return strict_status;
   }
   // If we fail, we see if we get a clean parse when allowing unknown fields.
   // This is essentially a workaround
@@ -259,15 +278,11 @@ void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& messa
   const auto relaxed_status = Protobuf::util::JsonStringToMessage(json, &message, options);
   // If we still fail with relaxed unknown field checking, the error has nothing
   // to do with unknown fields.
-  if (!relaxed_status.ok()) {
-    throw EnvoyException("Unable to parse JSON as proto (" + relaxed_status.ToString() +
-                         "): " + json);
+  if (relaxed_status.ok()) {
+    has_unknown_fileld = true;
+    return strict_status;
   }
-  // We know it's an unknown field at this point. If we're at the latest
-  // version, then it's definitely an unknown field, otherwise we try to
-  // load again at a later version.
-  validation_visitor.onUnknownField("type " + message.GetTypeName() + " reason " +
-                                    strict_status.ToString());
+  return relaxed_status;
 }
 
 void MessageUtil::loadFromJson(const std::string& json, ProtobufWkt::Struct& message) {
@@ -526,8 +541,20 @@ void MessageUtil::jsonConvert(const ProtobufWkt::Struct& source,
   jsonConvertInternal(source, validation_visitor, dest);
 }
 
-void MessageUtil::jsonConvertValue(const Protobuf::Message& source, ProtobufWkt::Value& dest) {
-  jsonConvertInternal(source, ProtobufMessage::getNullValidationVisitor(), dest);
+bool MessageUtil::jsonConvertValue(const Protobuf::Message& source, ProtobufWkt::Value& dest) {
+  Protobuf::util::JsonPrintOptions json_options;
+  json_options.preserve_proto_field_names = true;
+  std::string json;
+  auto status = Protobuf::util::MessageToJsonString(source, &json, json_options);
+  if (!status.ok()) {
+    return false;
+  }
+  bool has_unknow_field;
+  status = MessageUtil::loadFromJsonNoThrow(json, dest, has_unknow_field);
+  if (status.ok() || has_unknow_field) {
+    return true;
+  }
+  return false;
 }
 
 ProtobufWkt::Struct MessageUtil::keyValueStruct(const std::string& key, const std::string& value) {

source/common/protobuf/utility.h

@@ -252,6 +252,16 @@ class MessageUtil {
 
   static void loadFromJson(const std::string& json, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);
+  /**
+   * Return ok only when strict conversion(don't ignore unknown field) succeeds.
+   * Return error status for strict conversion and set has_unknown_field to true if relaxed
+   * conversion(ignore unknown field) succeeds.
+   * Return error status for relaxed conversion and set has_unknown_field to false if relaxed
+   * conversion(ignore unknown field) fails.
+   */
+  static Protobuf::util::Status loadFromJsonNoThrow(const std::string& json,
+                                                    Protobuf::Message& message,
+                                                    bool& has_unknown_fileld);
   static void loadFromJson(const std::string& json, ProtobufWkt::Struct& message);
   static void loadFromYaml(const std::string& yaml, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);
@@ -416,7 +426,8 @@ class MessageUtil {
   static void jsonConvert(const ProtobufWkt::Struct& source,
                           ProtobufMessage::ValidationVisitor& validation_visitor,
                           Protobuf::Message& dest);
-  static void jsonConvertValue(const Protobuf::Message& source, ProtobufWkt::Value& dest);
+  // Convert a message to a ProtobufWkt::Value, return false upon failure.
+  static bool jsonConvertValue(const Protobuf::Message& source, ProtobufWkt::Value& dest);
 
   /**
    * Extract YAML as string from a google.protobuf.Message.

test/common/protobuf/utility_test.cc

@@ -1486,14 +1486,14 @@ TEST_F(ProtobufUtilityTest, JsonConvertCamelSnake) {
                        .string_value());
 }
 
-// Test the jsonConvertValue happy path. Failure modes are converted by jsonConvert tests.
+// Test the jsonConvertValue in both success and failure modes.
 TEST_F(ProtobufUtilityTest, JsonConvertValueSuccess) {
   {
     envoy::config::bootstrap::v3::Bootstrap source;
     source.set_flags_path("foo");
     ProtobufWkt::Value tmp;
     envoy::config::bootstrap::v3::Bootstrap dest;
-    MessageUtil::jsonConvertValue(source, tmp);
+    EXPECT_TRUE(MessageUtil::jsonConvertValue(source, tmp));
     TestUtility::jsonConvert(tmp, dest);
     EXPECT_EQ("foo", dest.flags_path());
   }
@@ -1502,12 +1502,19 @@ TEST_F(ProtobufUtilityTest, JsonConvertValueSuccess) {
     ProtobufWkt::StringValue source;
     source.set_value("foo");
     ProtobufWkt::Value dest;
-    MessageUtil::jsonConvertValue(source, dest);
+    EXPECT_TRUE(MessageUtil::jsonConvertValue(source, dest));
 
     ProtobufWkt::Value expected;
     expected.set_string_value("foo");
     EXPECT_THAT(dest, ProtoEq(expected));
   }
+
+  {
+    ProtobufWkt::Duration source;
+    source.set_seconds(-281474976710656);
+    ProtobufWkt::Value dest;
+    EXPECT_FALSE(MessageUtil::jsonConvertValue(source, dest));
+  }
 }
 
 TEST_F(ProtobufUtilityTest, YamlLoadFromStringFail) {