Skip to content

Commit

Permalink
Fix reflection based parser for map entries with closed enum values.
Browse files Browse the repository at this point in the history
If the value of the parsed enum is unknown, the whole entry has to be moved to
the unknown field set.

PiperOrigin-RevId: 505175979
  • Loading branch information
protobuf-github-bot authored and Copybara-Service committed Jan 27, 2023
1 parent f17a629 commit 55d2123
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
18 changes: 15 additions & 3 deletions src/google/protobuf/map_test.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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) {
Expand Down
20 changes: 19 additions & 1 deletion src/google/protobuf/wire_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down

0 comments on commit 55d2123

Please sign in to comment.