build: removing JsonOrDie function (envoyproxy#26464)

Signed-off-by: Alyssa Wilk <[email protected]>
pchaseh · Apr 13, 2023 · 09ae448 · 09ae448
1 parent be9a54f
commit 09ae448
Show file tree

Hide file tree

Showing 48 changed files with 108 additions and 109 deletions.
diff --git a/contrib/config/test/kv_store_xds_delegate_integration_test.cc b/contrib/config/test/kv_store_xds_delegate_integration_test.cc
@@ -239,7 +239,7 @@ class KeyValueStoreXdsDelegateIntegrationTest
 
     // Set up a new Envoy, using the previous Envoy's configuration, and create the test server.
     ConfigHelper helper(version_, *api_,
-                        MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap()));
+                        MessageUtil::getJsonStringFromMessageOrError(config_helper_.bootstrap()));
     std::vector<uint32_t> ports;
     std::vector<uint32_t> zero;
     for (auto& upstream : fake_upstreams_) {

diff --git a/contrib/rocketmq_proxy/filters/network/source/active_message.cc b/contrib/rocketmq_proxy/filters/network/source/active_message.cc
@@ -208,7 +208,7 @@ void ActiveMessage::onQueryTopicRoute() {
     TopicRouteData topic_route_data(std::move(queue_data_list), std::move(broker_data_list));
     ProtobufWkt::Struct data_struct;
     topic_route_data.encode(data_struct);
-    std::string json = MessageUtil::getJsonStringFromMessageOrDie(data_struct);
+    std::string json = MessageUtil::getJsonStringFromMessageOrError(data_struct);
     ENVOY_LOG(trace, "Serialize TopicRouteData for {} OK:\n{}", cluster_name, json);
     RemotingCommandPtr response = std::make_unique<RemotingCommand>(
         static_cast<int>(ResponseCode::Success), downstreamRequest()->version(),

diff --git a/contrib/rocketmq_proxy/filters/network/source/codec.cc b/contrib/rocketmq_proxy/filters/network/source/codec.cc
@@ -391,7 +391,7 @@ void Encoder::encode(const RemotingCommandPtr& command, Buffer::Instance& data)
     (*fields)["extFields"] = ext_fields_v;
   }
 
-  std::string json = MessageUtil::getJsonStringFromMessageOrDie(command_struct);
+  std::string json = MessageUtil::getJsonStringFromMessageOrError(command_struct);
 
   int32_t frame_length = 4;
   int32_t header_length = json.size();

diff --git a/contrib/rocketmq_proxy/filters/network/source/conn_manager.cc b/contrib/rocketmq_proxy/filters/network/source/conn_manager.cc
@@ -301,7 +301,7 @@ void ConnectionManager::onGetConsumerListByGroup(RemotingCommandPtr request) {
   RemotingCommandPtr response = std::make_unique<RemotingCommand>(
       enumToSignedInt(ResponseCode::Success), request->version(), request->opaque());
   response->markAsResponse();
-  std::string json = MessageUtil::getJsonStringFromMessageOrDie(body_struct);
+  std::string json = MessageUtil::getJsonStringFromMessageOrError(body_struct);
   response->body().add(json);
   ENVOY_LOG(trace, "GetConsumerListByGroup respond with body: {}", json);
 

diff --git a/contrib/rocketmq_proxy/filters/network/test/topic_route_test.cc b/contrib/rocketmq_proxy/filters/network/test/topic_route_test.cc
@@ -63,7 +63,7 @@ TEST(TopicRouteDataTest, Serialization) {
   }
   ProtobufWkt::Struct doc;
   EXPECT_NO_THROW(topic_route_data.encode(doc));
-  MessageUtil::getJsonStringFromMessageOrDie(doc);
+  MessageUtil::getJsonStringFromMessageOrError(doc);
 }
 
 } // namespace RocketmqProxy

diff --git a/contrib/squash/filters/http/source/squash_filter.cc b/contrib/squash/filters/http/source/squash_filter.cc
@@ -50,7 +50,7 @@ SquashFilterConfig::SquashFilterConfig(
 std::string SquashFilterConfig::getAttachment(const ProtobufWkt::Struct& attachment_template) {
   ProtobufWkt::Struct attachment_json(attachment_template);
   updateTemplateInStruct(attachment_json);
-  return MessageUtil::getJsonStringFromMessageOrDie(attachment_json);
+  return MessageUtil::getJsonStringFromMessageOrError(attachment_json);
 }
 
 void SquashFilterConfig::updateTemplateInStruct(ProtobufWkt::Struct& attachment_template) {

diff --git a/source/common/config/xds_context_params.cc b/source/common/config/xds_context_params.cc
@@ -42,7 +42,12 @@ const NodeContextRenderers& nodeParamCbs() {
 void mergeMetadataJson(Protobuf::Map<std::string, std::string>& params,
                        const ProtobufWkt::Struct& metadata, const std::string& prefix) {
   for (const auto& it : metadata.fields()) {
-    params[prefix + it.first] = MessageUtil::getJsonStringFromMessageOrDie(it.second);
+    ProtobufUtil::StatusOr<std::string> json_or_error =
+        MessageUtil::getJsonStringFromMessage(it.second);
+    ENVOY_BUG(json_or_error.ok(), "Failed to parse json");
+    if (json_or_error.ok()) {
+      params[prefix + it.first] = json_or_error.value();
+    }
   }
 }
 

diff --git a/source/common/formatter/substitution_formatter.cc b/source/common/formatter/substitution_formatter.cc
@@ -151,7 +151,7 @@ std::string JsonFormatterImpl::format(const Http::RequestHeaderMap& request_head
       request_headers, response_headers, response_trailers, stream_info, local_reply_body);
 
   const std::string log_line =
-      MessageUtil::getJsonStringFromMessageOrDie(output_struct, false, true);
+      MessageUtil::getJsonStringFromMessageOrError(output_struct, false, true);
   return absl::StrCat(log_line, "\n");
 }
 
@@ -1897,7 +1897,12 @@ MetadataFormatter::formatMetadata(const envoy::config::core::v3::Metadata& metad
   if (value.kind_case() == ProtobufWkt::Value::kStringValue) {
     str = value.string_value();
   } else {
-    str = MessageUtil::getJsonStringFromMessageOrDie(value, false, true);
+    ProtobufUtil::StatusOr<std::string> json_or_error =
+        MessageUtil::getJsonStringFromMessage(value, false, true);
+    ENVOY_BUG(json_or_error.ok(), "Failed to parse json");
+    if (json_or_error.ok()) {
+      str = json_or_error.value();
+    }
   }
   truncate(str, max_length_);
   return str;

diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
@@ -490,25 +490,6 @@ class MessageUtil {
   getJsonStringFromMessage(const Protobuf::Message& message, bool pretty_print = false,
                            bool always_print_primitive_fields = false);
 
-  /**
-   * Extract JSON as string from a google.protobuf.Message, crashing if the conversion to JSON
-   * fails. This method is safe so long as the message does not contain an Any proto with an
-   * unrecognized type or invalid data.
-   * @param message message of type type.googleapis.com/google.protobuf.Message.
-   * @param pretty_print whether the returned JSON should be formatted.
-   * @param always_print_primitive_fields whether to include primitive fields set to their default
-   * values, e.g. an int32 set to 0 or a bool set to false.
-   * @return std::string of formatted JSON object.
-   */
-  static std::string getJsonStringFromMessageOrDie(const Protobuf::Message& message,
-                                                   bool pretty_print = false,
-                                                   bool always_print_primitive_fields = false) {
-    auto json_or_error =
-        getJsonStringFromMessage(message, pretty_print, always_print_primitive_fields);
-    RELEASE_ASSERT(json_or_error.ok(), json_or_error.status().ToString());
-    return std::move(json_or_error).value();
-  }
-
   /**
    * Extract JSON as string from a google.protobuf.Message, returning some error string if the
    * conversion to JSON fails.

diff --git a/source/common/tracing/custom_tag_impl.cc b/source/common/tracing/custom_tag_impl.cc
@@ -6,6 +6,14 @@
 
 namespace Envoy {
 namespace Tracing {
+namespace {
+
+absl::optional<std::string> jsonOrNullopt(const Protobuf::Message& message) {
+  auto json_or_error = MessageUtil::getJsonStringFromMessage(message);
+  return json_or_error.ok() ? absl::optional<std::string>(json_or_error.value()) : absl::nullopt;
+}
+
+} // namespace
 
 void CustomTagBase::applySpan(Span& span, const CustomTagContext& ctx) const {
   absl::string_view tag_value = value(ctx);
@@ -94,9 +102,9 @@ MetadataCustomTag::metadataToString(const envoy::config::core::v3::Metadata* met
   case ProtobufWkt::Value::kStringValue:
     return value.string_value();
   case ProtobufWkt::Value::kListValue:
-    return MessageUtil::getJsonStringFromMessageOrDie(value.list_value());
+    return jsonOrNullopt(value.list_value());
   case ProtobufWkt::Value::kStructValue:
-    return MessageUtil::getJsonStringFromMessageOrDie(value.struct_value());
+    return jsonOrNullopt(value.struct_value());
   default:
     break;
   }

diff --git a/source/common/upstream/subset_lb.cc b/source/common/upstream/subset_lb.cc
@@ -591,7 +591,7 @@ std::string SubsetLoadBalancer::describeMetadata(const SubsetLoadBalancer::Subse
     }
 
     const ProtobufWkt::Value& value = it.second;
-    buf << it.first << "=" << MessageUtil::getJsonStringFromMessageOrDie(value);
+    buf << it.first << "=" << MessageUtil::getJsonStringFromMessageOrError(value);
   }
 
   return buf.str();

diff --git a/source/extensions/config_subscription/rest/http_subscription_impl.cc b/source/extensions/config_subscription/rest/http_subscription_impl.cc
@@ -73,7 +73,7 @@ void HttpSubscriptionImpl::createRequest(Http::RequestMessage& request) {
   stats_.update_attempt_.inc();
   request.headers().setReferenceMethod(Http::Headers::get().MethodValues.Post);
   request.headers().setPath(path_);
-  request.body().add(MessageUtil::getJsonStringFromMessageOrDie(request_));
+  request.body().add(MessageUtil::getJsonStringFromMessageOrError(request_));
   request.headers().setReferenceContentType(Http::Headers::get().ContentTypeValues.Json);
   request.headers().setContentLength(request.body().length());
 }

diff --git a/source/extensions/tracers/dynamic_ot/config.cc b/source/extensions/tracers/dynamic_ot/config.cc
@@ -20,7 +20,10 @@ Tracing::DriverSharedPtr DynamicOpenTracingTracerFactory::createTracerDriverType
     Server::Configuration::TracerFactoryContext& context) {
   const std::string& library = proto_config.library();
   const ProtobufWkt::Struct& config_struct = proto_config.config();
-  const std::string config = MessageUtil::getJsonStringFromMessageOrDie(config_struct);
+  ProtobufUtil::StatusOr<std::string> json_or_error =
+      MessageUtil::getJsonStringFromMessage(config_struct);
+  ENVOY_BUG(json_or_error.ok(), "Failed to parse json");
+  const std::string config = json_or_error.ok() ? json_or_error.value() : "";
   return std::make_shared<DynamicOpenTracingDriver>(context.serverFactoryContext().scope(), library,
                                                     config);
 }

diff --git a/source/extensions/tracers/xray/daemon_broker.cc b/source/extensions/tracers/xray/daemon_broker.cc
@@ -21,8 +21,8 @@ std::string createHeader(const std::string& format, uint32_t version) {
   source::extensions::tracers::xray::daemon::Header header;
   header.set_format(format);
   header.set_version(version);
-  return MessageUtil::getJsonStringFromMessageOrDie(header, false /* pretty_print  */,
-                                                    false /* always_print_primitive_fields */);
+  return MessageUtil::getJsonStringFromMessageOrError(header, false /* pretty_print  */,
+                                                      false /* always_print_primitive_fields */);
 }
 
 } // namespace

diff --git a/source/extensions/tracers/xray/tracer.cc b/source/extensions/tracers/xray/tracer.cc
@@ -95,7 +95,7 @@ void Span::finishSpan() {
   // `direction` will be either "ingress" or "egress"
   s.mutable_annotations()->insert({std::string(DirectionKey), direction()});
 
-  const std::string json = MessageUtil::getJsonStringFromMessageOrDie(
+  const std::string json = MessageUtil::getJsonStringFromMessageOrError(
       s, false /* pretty_print  */, false /* always_print_primitive_fields */);
 
   broker_.send(json);

diff --git a/source/extensions/tracers/zipkin/span_buffer.cc b/source/extensions/tracers/zipkin/span_buffer.cc
@@ -75,9 +75,13 @@ std::string JsonV2Serializer::serialize(const std::vector<Span>& zipkin_spans) {
             out, absl::StrJoin(
                      toListOfSpans(zipkin_span, replacements), ",",
                      [&replacement_values](std::string* element, const ProtobufWkt::Struct& span) {
-                       const std::string json = MessageUtil::getJsonStringFromMessageOrDie(
-                           span, /* pretty_print */ false,
-                           /* always_print_primitive_fields */ true);
+                       ProtobufUtil::StatusOr<std::string> json_or_error =
+                           MessageUtil::getJsonStringFromMessage(span, false, true);
+                       ENVOY_BUG(json_or_error.ok(), "Failed to parse json");
+                       if (json_or_error.ok()) {
+                         absl::StrAppend(element, absl::StrReplaceAll(json_or_error.value(),
+                                                                      replacement_values));
+                       }
 
                        // The Zipkin API V2 specification mandates to store timestamp value as int64
                        // https://github.com/openzipkin/zipkin-api/blob/228fabe660f1b5d1e28eac9df41f7d1deed4a1c2/zipkin2-api.yaml#L447-L463
@@ -99,7 +103,6 @@ std::string JsonV2Serializer::serialize(const std::vector<Span>& zipkin_spans) {
                        // serializing double in protobuf DoubleToBuffer function, and make it
                        // available to be controlled at caller site.
                        // https://github.com/envoyproxy/envoy/issues/10411).
-                       absl::StrAppend(element, absl::StrReplaceAll(json, replacement_values));
                      }));
       });
   return absl::StrCat("[", serialized_elements, "]");

diff --git a/source/extensions/tracers/zipkin/zipkin_core_types.h b/source/extensions/tracers/zipkin/zipkin_core_types.h
@@ -48,10 +48,10 @@ class ZipkinBase {
    */
   const std::string toJson() const {
     Util::Replacements replacements;
-    return absl::StrReplaceAll(
-        MessageUtil::getJsonStringFromMessageOrDie(toStruct(replacements), /* pretty_print */ false,
-                                                   /* always_print_primitive_fields */ true),
-        replacements);
+    return absl::StrReplaceAll(MessageUtil::getJsonStringFromMessageOrError(
+                                   toStruct(replacements), /* pretty_print */ false,
+                                   /* always_print_primitive_fields */ true),
+                               replacements);
   };
 };
 

diff --git a/source/server/admin/runtime_handler.cc b/source/server/admin/runtime_handler.cc
@@ -72,7 +72,7 @@ Http::Code RuntimeHandler::handlerRuntime(Http::ResponseHeaderMap& response_head
   (*fields)["layers"] = ValueUtil::listValue(layer_names);
   (*fields)["entries"] = ValueUtil::structValue(layer_entries);
 
-  response.add(MessageUtil::getJsonStringFromMessageOrDie(runtime, true, true));
+  response.add(MessageUtil::getJsonStringFromMessageOrError(runtime, true, true));
   return Http::Code::OK;
 }
 

diff --git a/source/server/admin/stats_render.cc b/source/server/admin/stats_render.cc
@@ -164,7 +164,7 @@ void StatsJsonRender::finalize(Buffer::Instance& response) {
       histograms_obj_container_fields["histograms"].set_allocated_list_value(
           histogram_array_.release());
     }
-    auto str = MessageUtil::getJsonStringFromMessageOrDie(
+    auto str = MessageUtil::getJsonStringFromMessageOrError(
         ValueUtil::structValue(histograms_obj_container_), false /* pretty */, true);
 
     // Protobuf json serialization can yield an empty string (printing an

diff --git a/test/common/json/gen_excluded_unicodes.cc b/test/common/json/gen_excluded_unicodes.cc
@@ -77,8 +77,8 @@ void allThreeByteUtf8() {
         utf8[2] = byte3 | Utf8::ContinuePattern;
         auto [unicode, consumed] = Utf8::decode(utf8);
         if (consumed == 3) {
-          std::string proto_sanitized =
-              MessageUtil::getJsonStringFromMessageOrDie(ValueUtil::stringValue(utf8), false, true);
+          std::string proto_sanitized = MessageUtil::getJsonStringFromMessageOrError(
+              ValueUtil::stringValue(utf8), false, true);
           if (isInvalidProtobufSerialization(proto_sanitized)) {
             invalid.collect(unicode);
           }
@@ -106,7 +106,7 @@ void allFourByteUtf8() {
           utf8[3] = byte4 | Utf8::ContinuePattern;
           auto [unicode, consumed] = Utf8::decode(utf8);
           if (consumed == 4) {
-            std::string proto_sanitized = MessageUtil::getJsonStringFromMessageOrDie(
+            std::string proto_sanitized = MessageUtil::getJsonStringFromMessageOrError(
                 ValueUtil::stringValue(utf8), false, true);
             if (isInvalidProtobufSerialization(proto_sanitized)) {
               invalid.collect(unicode);

diff --git a/test/common/json/json_sanitizer_fuzz_test.cc b/test/common/json/json_sanitizer_fuzz_test.cc
@@ -23,7 +23,7 @@ DEFINE_FUZZER(const uint8_t* buf, size_t len) {
     // sanitizer does not crash.
     if (Envoy::Json::TestUtil::isProtoSerializableUtf8(input)) {
       buffer2 =
-          MessageUtil::getJsonStringFromMessageOrDie(ValueUtil::stringValue(input), false, true);
+          MessageUtil::getJsonStringFromMessageOrError(ValueUtil::stringValue(input), false, true);
       absl::string_view proto_sanitized = Envoy::Json::stripDoubleQuotes(buffer2);
       RELEASE_ASSERT(Envoy::Json::TestUtil::utf8Equivalent(sanitized, proto_sanitized, errmsg),
                      errmsg);

diff --git a/test/common/json/json_sanitizer_speed_test.cc b/test/common/json/json_sanitizer_speed_test.cc
@@ -14,8 +14,8 @@ static void BM_ProtoEncoderNoEscape(benchmark::State& state) {
   const std::string str = std::string(pass_through_encoding);
 
   for (auto _ : state) { // NOLINT
-    Envoy::MessageUtil::getJsonStringFromMessageOrDie(Envoy::ValueUtil::stringValue(str), false,
-                                                      true);
+    Envoy::MessageUtil::getJsonStringFromMessageOrError(Envoy::ValueUtil::stringValue(str), false,
+                                                        true);
   }
 }
 BENCHMARK(BM_ProtoEncoderNoEscape);
@@ -35,8 +35,8 @@ static void BM_ProtoEncoderWithEscape(benchmark::State& state) {
   const std::string str = std::string(escaped_encoding);
 
   for (auto _ : state) { // NOLINT
-    Envoy::MessageUtil::getJsonStringFromMessageOrDie(Envoy::ValueUtil::stringValue(str), false,
-                                                      true);
+    Envoy::MessageUtil::getJsonStringFromMessageOrError(Envoy::ValueUtil::stringValue(str), false,
+                                                        true);
   }
 }
 BENCHMARK(BM_ProtoEncoderWithEscape);

diff --git a/test/common/json/json_sanitizer_test.cc b/test/common/json/json_sanitizer_test.cc
@@ -26,7 +26,7 @@ class JsonSanitizerTest : public testing::Test {
   absl::string_view sanitize(absl::string_view str) { return Envoy::Json::sanitize(buffer_, str); }
 
   absl::string_view protoSanitize(absl::string_view str) {
-    proto_serialization_buffer_ = MessageUtil::getJsonStringFromMessageOrDie(
+    proto_serialization_buffer_ = MessageUtil::getJsonStringFromMessageOrError(
         ValueUtil::stringValue(std::string(str)), false, true);
     return stripDoubleQuotes(proto_serialization_buffer_);
   }

diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
@@ -288,7 +288,7 @@ TEST_F(ProtobufUtilityTest, JsonConvertAnyUnknownMessageType) {
 TEST_F(ProtobufUtilityTest, JsonConvertKnownGoodMessage) {
   ProtobufWkt::Any source_any;
   source_any.PackFrom(envoy::config::bootstrap::v3::Bootstrap::default_instance());
-  EXPECT_THAT(MessageUtil::getJsonStringFromMessageOrDie(source_any, true),
+  EXPECT_THAT(MessageUtil::getJsonStringFromMessageOrError(source_any, true),
               testing::HasSubstr("@type"));
 }
 
@@ -300,13 +300,6 @@ TEST_F(ProtobufUtilityTest, JsonConvertOrErrorAnyWithUnknownMessageType) {
               HasSubstr("Failed to convert"));
 }
 
-TEST_F(ProtobufUtilityTest, JsonConvertOrDieAnyWithUnknownMessageType) {
-  ProtobufWkt::Any source_any;
-  source_any.set_type_url("type.googleapis.com/bad.type.url");
-  source_any.set_value("asdf");
-  EXPECT_DEATH(MessageUtil::getJsonStringFromMessageOrDie(source_any), "");
-}
-
 TEST_F(ProtobufUtilityTest, LoadBinaryProtoFromFile) {
   envoy::config::bootstrap::v3::Bootstrap bootstrap;
   bootstrap.mutable_cluster_manager()
@@ -1590,7 +1583,7 @@ TEST_F(ProtobufUtilityTest, JsonConvertCamelSnake) {
   ProtobufWkt::Struct json;
   TestUtility::jsonConvert(bootstrap, json);
   // Verify we can round-trip. This didn't cause the #3665 regression, but useful as a sanity check.
-  TestUtility::loadFromJson(MessageUtil::getJsonStringFromMessageOrDie(json, false), bootstrap);
+  TestUtility::loadFromJson(MessageUtil::getJsonStringFromMessageOrError(json, false), bootstrap);
   // Verify we don't do a camel case conversion.
   EXPECT_EQ("foo", json.fields()
                        .at("cluster_manager")

diff --git a/test/config/utility.cc b/test/config/utility.cc
@@ -1580,7 +1580,7 @@ void ConfigHelper::setLds(absl::string_view version_info) {
   const std::string lds_filename =
       bootstrap().dynamic_resources().lds_config().path_config_source().path();
   std::string file = TestEnvironment::writeStringToFileForTest(
-      "new_lds_file", MessageUtil::getJsonStringFromMessageOrDie(lds));
+      "new_lds_file", MessageUtil::getJsonStringFromMessageOrError(lds));
   TestEnvironment::renameFile(file, lds_filename);
 }
 

diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_integration_test.cc
@@ -334,7 +334,7 @@ TEST_P(AccessLogIntegrationTest, GrpcLoggerSurvivesAfterReloadConfig) {
 
   // Create a new config with HTTP/1.0 proxying. The goal is to trigger a listener update.
   ConfigHelper new_config_helper(
-      version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap()));
+      version_, *api_, MessageUtil::getJsonStringFromMessageOrError(config_helper_.bootstrap()));
   new_config_helper.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
               hcm) {