Make retention stripping code tolerant of parsing/serialization errors

PiperOrigin-RevId: 525168179

src/google/protobuf/compiler/retention.cc

@@ -102,8 +102,7 @@ void ConvertToDynamicMessageAndStripOptions(
   const Descriptor* descriptor = pool.FindMessageTypeByName(m.GetTypeName());
   std::vector<int> 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<Message> 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;
+    }
   }
 }
 

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<int>(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