From 403e2078b4501359f04b2c0b60bf37b9305a9e0e Mon Sep 17 00:00:00 2001
From: Protobuf Team Bot <protobuf-github-bot@google.com>
Date: Mon, 29 Jan 2024 11:05:07 -0800
Subject: [PATCH] Fail building descriptors if `ctype` is used for fields other
 than string or bytes.

Instead of silently ignoring `[ctype = XXX]` for non string or bytes fields,
this CL starts failing to build descriptors to call out the issue. This may
cause failures to existing proto schemas but fixing them should be straightforward.

PiperOrigin-RevId: 602441330
---
 src/google/protobuf/descriptor.cc          | 23 ++++++++++++++++++++++
 src/google/protobuf/descriptor_unittest.cc | 22 ++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
index 12301a788ee6b..08870ccac099b 100644
--- a/src/google/protobuf/descriptor.cc
+++ b/src/google/protobuf/descriptor.cc
@@ -7735,6 +7735,29 @@ void DescriptorBuilder::ValidateOptions(const FieldDescriptor* field,
 
   ValidateFieldFeatures(field, proto);
 
+  // The following check is temporarily OSS only till we fix all affected
+  // google3 TAP tests.
+  if (field->options().has_ctype()) {
+    if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) {
+      AddError(
+          field->full_name(), proto, DescriptorPool::ErrorCollector::TYPE,
+          absl::StrFormat(
+              "Field %s specifies ctype, but is not a string nor bytes field.",
+              field->full_name())
+              .c_str());
+    }
+    if (field->options().ctype() == FieldOptions::CORD) {
+      if (field->is_extension()) {
+        AddError(field->full_name(), proto,
+                 DescriptorPool::ErrorCollector::TYPE,
+                 absl::StrFormat("Extension %s specifies ctype=CORD which is "
+                                 "not supported for extensions.",
+                                 field->full_name())
+                     .c_str());
+      }
+    }
+  }
+
   // Only message type fields may be lazy.
   if (field->options().lazy() || field->options().unverified_lazy()) {
     if (field->type() != FieldDescriptor::TYPE_MESSAGE) {
diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
index 7bf5000fb952e..f0b6eef050004 100644
--- a/src/google/protobuf/descriptor_unittest.cc
+++ b/src/google/protobuf/descriptor_unittest.cc
@@ -2953,6 +2953,26 @@ TEST_F(MiscTest, DefaultValues) {
   EXPECT_EQ(enum_value_a, message->field(22)->default_value_enum());
 }
 
+TEST_F(MiscTest, InvalidFieldOptions) {
+  FileDescriptorProto file_proto;
+  file_proto.set_name("foo.proto");
+
+  DescriptorProto* message_proto = AddMessage(&file_proto, "TestMessage");
+  AddField(message_proto, "foo", 1, FieldDescriptorProto::LABEL_OPTIONAL,
+           FieldDescriptorProto::TYPE_INT32);
+  FieldDescriptorProto* bar_proto =
+      AddField(message_proto, "bar", 2, FieldDescriptorProto::LABEL_OPTIONAL,
+               FieldDescriptorProto::TYPE_INT32);
+
+  FieldOptions* options = bar_proto->mutable_options();
+  options->set_ctype(FieldOptions::CORD);
+
+  // Expects it to fail as int32 fields cannot have ctype.
+  DescriptorPool pool;
+  const FileDescriptor* file = pool.BuildFile(file_proto);
+  EXPECT_EQ(file, nullptr);
+}
+
 TEST_F(MiscTest, FieldOptions) {
   // Try setting field options.
 
@@ -2964,7 +2984,7 @@ TEST_F(MiscTest, FieldOptions) {
            FieldDescriptorProto::TYPE_INT32);
   FieldDescriptorProto* bar_proto =
       AddField(message_proto, "bar", 2, FieldDescriptorProto::LABEL_OPTIONAL,
-               FieldDescriptorProto::TYPE_INT32);
+               FieldDescriptorProto::TYPE_BYTES);
 
   FieldOptions* options = bar_proto->mutable_options();
   options->set_ctype(FieldOptions::CORD);