From 55d21239e950164f810f3e8c348ce5bf0d7537d6 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Fri, 27 Jan 2023 12:03:05 -0800 Subject: [PATCH] Fix reflection based parser for map entries with closed enum values. If the value of the parsed enum is unknown, the whole entry has to be moved to the unknown field set. PiperOrigin-RevId: 505175979 --- src/google/protobuf/map_test.inc | 18 +++++++++++++++--- src/google/protobuf/wire_format.cc | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc index 89f8033017..8385560d7b 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -2708,9 +2708,7 @@ TEST(GeneratedMapFieldTest, Proto2UnknownEnum) { UNITTEST::TestEnumMap to; EXPECT_TRUE(to.ParseFromString(data)); EXPECT_EQ(0, to.unknown_map_field().size()); - const UnknownFieldSet& unknown_field_set = - to.GetReflection()->GetUnknownFields(to); - EXPECT_EQ(1, unknown_field_set.field_count()); + EXPECT_EQ(1, to.GetReflection()->GetUnknownFields(to).field_count()); EXPECT_EQ(1, to.known_map_field().size()); EXPECT_EQ(UNITTEST::PROTO2_MAP_ENUM_FOO, to.known_map_field().at(0)); @@ -2723,6 +2721,20 @@ TEST(GeneratedMapFieldTest, Proto2UnknownEnum) { EXPECT_EQ(UNITTEST::E_PROTO2_MAP_ENUM_FOO, from.known_map_field().at(0)); EXPECT_EQ(1, from.unknown_map_field().size()); EXPECT_EQ(UNITTEST::E_PROTO2_MAP_ENUM_EXTRA, from.unknown_map_field().at(0)); + + // Test the same behavior with the reflection based parser. + to.Clear(); + const char* ptr; + internal::ParseContext ctx(io::CodedInputStream::GetDefaultRecursionLimit(), + false, &ptr, data); + ptr = WireFormat::_InternalParse(&to, ptr, &ctx); + ASSERT_TRUE(ptr); + ASSERT_TRUE(ctx.EndedAtLimit()); + + EXPECT_EQ(0, to.unknown_map_field().size()); + EXPECT_EQ(1, to.GetReflection()->GetUnknownFields(to).field_count()); + EXPECT_EQ(1, to.known_map_field().size()); + EXPECT_EQ(UNITTEST::PROTO2_MAP_ENUM_FOO, to.known_map_field().at(0)); } TEST(GeneratedMapFieldTest, StandardWireFormat) { diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index f988a090c4..27aa64fda9 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -1030,7 +1030,25 @@ const char* WireFormat::_InternalParseAndMergeField( sub_message = reflection->MutableMessage(msg, field, ctx->data().factory); } - return ctx->ParseMessage(sub_message, ptr); + ptr = ctx->ParseMessage(sub_message, ptr); + + // For map entries, if the value is an unknown enum we have to push it + // into the unknown field set and remove it from the list. + if (ptr != nullptr && field->is_map()) { + auto* value_field = field->message_type()->map_value(); + auto* enum_type = value_field->enum_type(); + if (enum_type != nullptr && + !internal::cpp::HasPreservingUnknownEnumSemantics(value_field) && + enum_type->FindValueByNumber( + sub_message->GetReflection()->GetEnumValue( + *sub_message, value_field)) == nullptr) { + reflection->MutableUnknownFields(msg)->AddLengthDelimited( + field->number(), sub_message->SerializeAsString()); + reflection->RemoveLast(msg, field); + } + } + + return ptr; } }