C++Proto JSON: Align behavior on the type.proto path with the descriptor.proto path

- When printing maps to JSON, if either a key or value wasn't present on the binary wire repr, the type.proto path crashed. It now properly uses the default value for the type.

- Multiple instances on the binary wire format for the same singular value are rejected in the descriptor.proto case, versus in the type.proto case it could silently used the first seen value.

The second case is odd since normal wire format behavior spec is last value silently wins (= neither of the preexisting behaviors here). This change locally aligns the  type.proto path to match the descriptor.proto path.

Fixes #26885

PiperOrigin-RevId: 900807575

Author: protobuf-github-bot

src/google/protobuf/json/internal/unparser_traits.h

@@ -15,13 +15,11 @@
 #include <vector>
 
 #include "google/protobuf/type.pb.h"
-#include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/escaping.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
-#include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/dynamic_message.h"
 #include "google/protobuf/io/coded_stream.h"
@@ -341,54 +339,182 @@ struct UnparseProto3Type : Proto3Type {
     return absl::InternalError("message fields cannot have defaults");
   }
 
-  static absl::StatusOr<float> GetFloat(Field f, const Msg& msg,
-                                        size_t idx = 0) {
+  static absl::StatusOr<float> GetFloat(Field f, const Msg& msg) {
+    auto span = msg.Get<float>(f->proto().number());
+    if (span.empty()) {
+      return GetFloat(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
+  static absl::StatusOr<float> GetFloat(Field f, const Msg& msg, size_t idx) {
     return msg.Get<float>(f->proto().number())[idx];
   }
 
-  static absl::StatusOr<double> GetDouble(Field f, const Msg& msg,
-                                          size_t idx = 0) {
+  static absl::StatusOr<double> GetDouble(Field f, const Msg& msg) {
+    auto span = msg.Get<double>(f->proto().number());
+    if (span.empty()) {
+      return GetDouble(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
+  static absl::StatusOr<double> GetDouble(Field f, const Msg& msg, size_t idx) {
     return msg.Get<double>(f->proto().number())[idx];
   }
 
-  static absl::StatusOr<int32_t> GetInt32(Field f, const Msg& msg,
-                                          size_t idx = 0) {
+  static absl::StatusOr<int32_t> GetInt32(Field f, const Msg& msg) {
+    auto span = msg.Get<int32_t>(f->proto().number());
+    if (span.empty()) {
+      return GetInt32(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
+  static absl::StatusOr<int32_t> GetInt32(Field f, const Msg& msg, size_t idx) {
     return msg.Get<int32_t>(f->proto().number())[idx];
   }
 
+  static absl::StatusOr<uint32_t> GetUInt32(Field f, const Msg& msg) {
+    auto span = msg.Get<uint32_t>(f->proto().number());
+    if (span.empty()) {
+      return GetUInt32(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
   static absl::StatusOr<uint32_t> GetUInt32(Field f, const Msg& msg,
-                                            size_t idx = 0) {
+                                            size_t idx) {
     return msg.Get<uint32_t>(f->proto().number())[idx];
   }
 
-  static absl::StatusOr<int64_t> GetInt64(Field f, const Msg& msg,
-                                          size_t idx = 0) {
+  static absl::StatusOr<int64_t> GetInt64(Field f, const Msg& msg) {
+    auto span = msg.Get<int64_t>(f->proto().number());
+    if (span.empty()) {
+      return GetInt64(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
+  static absl::StatusOr<int64_t> GetInt64(Field f, const Msg& msg, size_t idx) {
     return msg.Get<int64_t>(f->proto().number())[idx];
   }
 
+  static absl::StatusOr<uint64_t> GetUInt64(Field f, const Msg& msg) {
+    auto span = msg.Get<uint64_t>(f->proto().number());
+    if (span.empty()) {
+      return GetUInt64(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
   static absl::StatusOr<uint64_t> GetUInt64(Field f, const Msg& msg,
-                                            size_t idx = 0) {
+                                            size_t idx) {
     return msg.Get<uint64_t>(f->proto().number())[idx];
   }
 
-  static absl::StatusOr<bool> GetBool(Field f, const Msg& msg, size_t idx = 0) {
+  static absl::StatusOr<bool> GetBool(Field f, const Msg& msg) {
+    auto span = msg.Get<Msg::Bool>(f->proto().number());
+    if (span.empty()) {
+      return GetBool(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0] == Msg::kTrue;
+  }
+
+  static absl::StatusOr<bool> GetBool(Field f, const Msg& msg, size_t idx) {
     return msg.Get<Msg::Bool>(f->proto().number())[idx] == Msg::kTrue;
   }
 
+  static absl::StatusOr<int32_t> GetEnumValue(Field f, const Msg& msg) {
+    auto span = msg.Get<int32_t>(f->proto().number());
+    if (span.empty()) {
+      return GetEnumValue(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
   static absl::StatusOr<int32_t> GetEnumValue(Field f, const Msg& msg,
-                                              size_t idx = 0) {
+                                              size_t idx) {
     return msg.Get<int32_t>(f->proto().number())[idx];
   }
 
+  static absl::StatusOr<absl::string_view> GetString(Field f,
+                                                     std::string& scratch,
+                                                     const Msg& msg) {
+    auto span = msg.Get<std::string>(f->proto().number());
+    if (span.empty()) {
+      return GetString(f, scratch);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return span[0];
+  }
+
   static absl::StatusOr<absl::string_view> GetString(Field f,
                                                      std::string& scratch,
                                                      const Msg& msg,
-                                                     size_t idx = 0) {
+                                                     size_t idx) {
     return msg.Get<std::string>(f->proto().number())[idx];
   }
 
+  static absl::StatusOr<const Msg*> GetMessage(Field f, const Msg& msg) {
+    auto span = msg.Get<Msg>(f->proto().number());
+    if (span.empty()) {
+      return GetMessage(f);
+    }
+    if (span.size() > 1) {
+      return absl::InternalError(absl::StrCat(
+          "Attempted to read a non-repeated field with multiple values: ",
+          f->parent().proto().name()));
+    }
+    return &span[0];
+  }
+
   static absl::StatusOr<const Msg*> GetMessage(Field f, const Msg& msg,
-                                               size_t idx = 0) {
+                                               size_t idx) {
     return &msg.Get<Msg>(f->proto().number())[idx];
   }
 

src/google/protobuf/json/json_test.cc

@@ -47,6 +47,7 @@ using ::proto3::TestEnumValue;
 using ::proto3::TestMap;
 using ::proto3::TestMessage;
 using ::proto3::TestOneof;
+using ::proto3::TestStringMapOverlay;
 using ::proto3::TestWrapper;
 using ::testing::ContainsRegex;
 using ::testing::ElementsAre;
@@ -645,6 +646,106 @@ TEST_P(JsonTest, ParseMap) {
   EXPECT_EQ(other->DebugString(), message.DebugString());
 }
 
+TEST_P(JsonTest, BinaryToJsonStreamMapEntryWithNoValue) {
+  TestStringMapOverlay emulated_message;
+  auto* entry = emulated_message.add_string_map();
+  entry->set_key("hello");
+
+  std::string proto_data_emulated = emulated_message.SerializeAsString();
+  io::ArrayInputStream in(proto_data_emulated.data(),
+                          proto_data_emulated.size());
+
+  std::string printed;
+  io::StringOutputStream out(&printed);
+
+  PrintOptions options = {};
+  auto status = BinaryToJsonStream(resolver_.get(),
+                                   "type.googleapis.com/proto3.TestStringMap",
+                                   &in, &out, options);
+  ASSERT_OK(status);
+
+  ASSERT_EQ(printed, R"({"stringMap":{"hello":""}})");
+}
+
+TEST_P(JsonTest, BinaryToJsonStreamMapEntryWithNoKey) {
+  TestStringMapOverlay emulated_message;
+  auto* entry = emulated_message.add_string_map();
+  entry->set_value("1234");
+
+  std::string proto_data_emulated = emulated_message.SerializeAsString();
+  io::ArrayInputStream in(proto_data_emulated.data(),
+                          proto_data_emulated.size());
+
+  std::string printed;
+  io::StringOutputStream out(&printed);
+
+  PrintOptions options = {};
+  auto status = BinaryToJsonStream(resolver_.get(),
+                                   "type.googleapis.com/proto3.TestStringMap",
+                                   &in, &out, options);
+  ASSERT_OK(status);
+
+  ASSERT_EQ(printed, R"({"stringMap":{"":"1234"}})");
+}
+
+TEST_P(JsonTest, BinaryToJsonStreamMapEntryWithNoKeyOrValue) {
+  TestStringMapOverlay emulated_message;
+  (void)emulated_message.add_string_map();
+
+  std::string proto_data_emulated = emulated_message.SerializeAsString();
+  io::ArrayInputStream in(proto_data_emulated.data(),
+                          proto_data_emulated.size());
+
+  std::string printed;
+  io::StringOutputStream out(&printed);
+
+  PrintOptions options = {};
+  auto status = BinaryToJsonStream(resolver_.get(),
+                                   "type.googleapis.com/proto3.TestStringMap",
+                                   &in, &out, options);
+  ASSERT_OK(status);
+
+  ASSERT_EQ(printed, R"({"stringMap":{"":""}})");
+}
+
+TEST_P(JsonTest, BinaryToJsonStreamDuplicateNonRepeatedField) {
+  // The wire tag for field 6 (float_value) is (6 << 3) | 5 = 0x35.
+  // We provide multiple values for the same non-repeated field.
+  std::string binary_data("\x35\x00\x00\x80\x3F\x35\x00\x00\x00\x40", 10);
+  io::ArrayInputStream in(binary_data.data(), binary_data.size());
+
+  std::string printed;
+  io::StringOutputStream out(&printed);
+
+  PrintOptions options = {};
+  options.always_print_fields_with_no_presence = true;
+  auto status = BinaryToJsonStream(resolver_.get(),
+                                   "type.googleapis.com/proto3.TestMessage",
+                                   &in, &out, options);
+  EXPECT_THAT(status, StatusIs(absl::StatusCode::kInvalidArgument));
+  EXPECT_THAT(status.message(),
+              ContainsRegex("repeated entries for singular field number 6"));
+}
+
+TEST_P(JsonTest, BinaryToJsonStreamAlwaysPrintPrimitivesNoValueOnWire) {
+  // Missing field should fall back to default value.
+  std::string binary_data = "";  // Empty message
+  io::ArrayInputStream in(binary_data.data(), binary_data.size());
+
+  std::string printed;
+  io::StringOutputStream out(&printed);
+
+  PrintOptions options;
+  options.always_print_fields_with_no_presence = true;
+  auto status = BinaryToJsonStream(resolver_.get(),
+                                   "type.googleapis.com/proto3.TestMessage",
+                                   &in, &out, options);
+  ASSERT_OK(status);
+
+  // Verify that floatValue is printed with default 0.
+  EXPECT_THAT(printed, ContainsRegex(R"("floatValue":0)"));
+}
+
 TEST_P(JsonTest, RepeatedMapKey) {
   EXPECT_THAT(ToProto<TestMap>(R"json({
     "string_map": {

src/google/protobuf/util/json_format_proto3.proto

@@ -112,6 +112,16 @@ message TestStringMap {
   map<string, string> string_map = 1;
 }
 
+// An overlay message of TestStringMap intended to make it easier to
+// construct some test cases of the binary wire format.
+message TestStringMapOverlay {
+  message MapEntry {
+    string key = 1;
+    string value = 2;
+  }
+  repeated MapEntry string_map = 1;
+}
+
 message TestWrapper {
   google.protobuf.BoolValue bool_value = 1;
   google.protobuf.Int32Value int32_value = 2;