From a6a238065e61b197b1ac69d61049dff3a7ea4448 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Wed, 1 Mar 2023 08:57:09 -0800 Subject: [PATCH] Fix parsing/serialization of packed proto3 custom options We have an obscure bug where packed-by-default repeated custom options defined in proto3 files are not correctly treated as packed when dynamic extensions are used. This CL introduces a simple fix and adds a test verifying the correct behavior. The bug was not a major problem in most cases, since parsers will accept both the packed and unpacked encodings. It is useful to fix this to ensure that descriptors are serialized in a consistent way, though. PiperOrigin-RevId: 513249881 --- src/google/protobuf/extension_set_heavy.cc | 2 +- src/google/protobuf/extension_set_unittest.cc | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/extension_set_heavy.cc b/src/google/protobuf/extension_set_heavy.cc index cdc4cc34ee..27b5dd6151 100644 --- a/src/google/protobuf/extension_set_heavy.cc +++ b/src/google/protobuf/extension_set_heavy.cc @@ -285,7 +285,7 @@ bool DescriptorPoolExtensionFinder::Find(int number, ExtensionInfo* output) { } else { output->type = extension->type(); output->is_repeated = extension->is_repeated(); - output->is_packed = extension->options().packed(); + output->is_packed = extension->is_packed(); output->descriptor = extension; if (extension->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { output->message_info.prototype = diff --git a/src/google/protobuf/extension_set_unittest.cc b/src/google/protobuf/extension_set_unittest.cc index ccd1c9b8d5..b944404291 100644 --- a/src/google/protobuf/extension_set_unittest.cc +++ b/src/google/protobuf/extension_set_unittest.cc @@ -1359,6 +1359,54 @@ TEST(ExtensionSetTest, DynamicExtensions) { } } +TEST(ExtensionSetTest, Proto3PackedDynamicExtensions) { + // Regression test for b/271121265. This test case verifies that + // packed-by-default repeated custom options in proto3 are correctly + // serialized in packed form when dynamic extensions are used. + + // Create a custom option in proto3 and load this into an overlay + // DescriptorPool with a DynamicMessageFactory. + google::protobuf::FileDescriptorProto file_descriptor_proto; + file_descriptor_proto.set_syntax("proto3"); + file_descriptor_proto.set_name( + "third_party/protobuf/unittest_proto3_packed_extension.proto"); + file_descriptor_proto.set_package("proto3_unittest"); + file_descriptor_proto.add_dependency( + DescriptorProto::descriptor()->file()->name()); + FieldDescriptorProto* extension = file_descriptor_proto.add_extension(); + extension->set_name("repeated_int32_option"); + extension->set_extendee(MessageOptions().GetTypeName()); + extension->set_number(50009); + extension->set_label(FieldDescriptorProto::LABEL_REPEATED); + extension->set_type(FieldDescriptorProto::TYPE_INT32); + extension->set_json_name("repeatedInt32Option"); + google::protobuf::DescriptorPool pool(DescriptorPool::generated_pool()); + ASSERT_NE(pool.BuildFile(file_descriptor_proto), nullptr); + DynamicMessageFactory factory; + factory.SetDelegateToGeneratedFactory(true); + + // Create a serialized MessageOptions proto equivalent to: + // [proto3_unittest.repeated_int32_option]: 1 + UnknownFieldSet unknown_fields; + unknown_fields.AddVarint(50009, 1); + std::string serialized_options; + ASSERT_TRUE(unknown_fields.SerializeToString(&serialized_options)); + + // Parse the MessageOptions using our custom extension registry. + io::ArrayInputStream input_stream(serialized_options.data(), + serialized_options.size()); + io::CodedInputStream coded_stream(&input_stream); + coded_stream.SetExtensionRegistry(&pool, &factory); + MessageOptions message_options; + ASSERT_TRUE(message_options.ParseFromCodedStream(&coded_stream)); + + // Finally, serialize the proto again and verify that the repeated option has + // been correctly serialized in packed form. + std::string reserialized_options; + ASSERT_TRUE(message_options.SerializeToString(&reserialized_options)); + EXPECT_EQ(reserialized_options, "\xca\xb5\x18\x01\x01"); +} + TEST(ExtensionSetTest, BoolExtension) { unittest::TestAllExtensions msg; uint8_t wire_bytes[2] = {13 * 8, 42 /* out of bounds payload for bool */};