diff --git a/src/google/protobuf/compiler/retention.cc b/src/google/protobuf/compiler/retention.cc index a8212cddf5..6ac0876dd6 100644 --- a/src/google/protobuf/compiler/retention.cc +++ b/src/google/protobuf/compiler/retention.cc @@ -102,8 +102,7 @@ void ConvertToDynamicMessageAndStripOptions( const Descriptor* descriptor = pool.FindMessageTypeByName(m.GetTypeName()); std::vector path; - if (descriptor == nullptr || - descriptor->file()->pool() == DescriptorPool::generated_pool()) { + if (descriptor == nullptr || &pool == DescriptorPool::generated_pool()) { // If the pool does not contain the descriptor, then this proto file does // not transitively depend on descriptor.proto, in which case we know there // are no custom options to worry about. If we are working with the @@ -115,11 +114,27 @@ void ConvertToDynamicMessageAndStripOptions( std::unique_ptr dynamic_message( factory.GetPrototype(descriptor)->New()); std::string serialized; - ABSL_CHECK(m.SerializeToString(&serialized)); - ABSL_CHECK(dynamic_message->ParseFromString(serialized)); + if (!m.SerializePartialToString(&serialized)) { + ABSL_LOG_EVERY_N_SEC(ERROR, 1) + << "Failed to strip source-retention options"; + return; + } + if (!dynamic_message->ParsePartialFromString(serialized)) { + ABSL_LOG_EVERY_N_SEC(ERROR, 1) + << "Failed to strip source-retention options"; + return; + } StripMessage(*dynamic_message, path, stripped_paths); - ABSL_CHECK(dynamic_message->SerializeToString(&serialized)); - ABSL_CHECK(m.ParseFromString(serialized)); + if (!dynamic_message->SerializePartialToString(&serialized)) { + ABSL_LOG_EVERY_N_SEC(ERROR, 1) + << "Failed to strip source-retention options"; + return; + } + if (!m.ParsePartialFromString(serialized)) { + ABSL_LOG_EVERY_N_SEC(ERROR, 1) + << "Failed to strip source-retention options"; + return; + } } } diff --git a/src/google/protobuf/retention_test.cc b/src/google/protobuf/retention_test.cc index ca80532dbd..2f0075a5bb 100644 --- a/src/google/protobuf/retention_test.cc +++ b/src/google/protobuf/retention_test.cc @@ -243,6 +243,44 @@ TEST(RetentionTest, StripSourceRetentionOptionsWithSourceCodeInfo) { EXPECT_EQ(stripped_file.source_code_info().location_size(), 63); } +TEST(RetentionTest, InvalidDescriptor) { + // This test creates an invalid descriptor and makes sure we can strip its + // options without crashing. + std::string proto_file = + absl::Substitute(R"( + syntax = "proto3"; + + package google.protobuf.internal; + + import "$0"; + + // String option with invalid UTF-8 + option (s) = "\xff"; + + extend google.protobuf.FileOptions { + optional string s = 50000; + })", + FileDescriptorSet::descriptor()->file()->name()); + io::ArrayInputStream input_stream(proto_file.data(), + static_cast(proto_file.size())); + io::ErrorCollector error_collector; + io::Tokenizer tokenizer(&input_stream, &error_collector); + compiler::Parser parser; + FileDescriptorProto file_descriptor_proto; + ASSERT_TRUE(parser.Parse(&tokenizer, &file_descriptor_proto)); + file_descriptor_proto.set_name("retention.proto"); + + DescriptorPool pool; + FileDescriptorProto descriptor_proto_descriptor; + FileDescriptorSet::descriptor()->file()->CopyTo(&descriptor_proto_descriptor); + ASSERT_NE(pool.BuildFile(descriptor_proto_descriptor), nullptr); + const FileDescriptor* file_descriptor = pool.BuildFile(file_descriptor_proto); + ASSERT_NE(file_descriptor, nullptr); + + FileDescriptorProto stripped_file = + compiler::StripSourceRetentionOptions(*file_descriptor); +} + } // namespace } // namespace internal } // namespace protobuf