Add validation for string_type and ctype under edition 2024 and over.

PiperOrigin-RevId: 604345202

src/google/protobuf/compiler/BUILD.bazel

@@ -83,6 +83,7 @@ cc_library(
         "//src/google/protobuf:port",
         "//src/google/protobuf:protobuf_lite",
         "//src/google/protobuf/compiler:retention",
+        "//src/google/protobuf/compiler/allowlists",
         "//src/google/protobuf/io",
         "//src/google/protobuf/io:io_win32",
         "@com_google_absl//absl/container:flat_hash_map",

src/google/protobuf/compiler/code_generator.cc

@@ -19,6 +19,7 @@
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
+#include "google/protobuf/compiler/allowlists/allowlists.h"
 #include "google/protobuf/compiler/plugin.pb.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/feature_resolver.h"
@@ -139,6 +140,11 @@ bool IsKnownFeatureProto(absl::string_view filename) {
   return false;
 }
 
+bool CanSkipEditionCheck(absl::string_view filename) {
+  return absl::StartsWith(filename, "google/protobuf/") ||
+         absl::StartsWith(filename, "upb/");
+}
+
 }  // namespace compiler
 }  // namespace protobuf
 }  // namespace google

src/google/protobuf/compiler/code_generator.h

@@ -238,6 +238,9 @@ PROTOC_EXPORT std::string StripProto(absl::string_view filename);
 // Returns true if the proto path corresponds to a known feature file.
 PROTOC_EXPORT bool IsKnownFeatureProto(absl::string_view filename);
 
+// Returns true if the proto path can skip edition check.
+PROTOC_EXPORT bool CanSkipEditionCheck(absl::string_view filename);
+
 }  // namespace compiler
 }  // namespace protobuf
 }  // namespace google

src/google/protobuf/compiler/command_line_interface.cc

@@ -1514,10 +1514,13 @@ bool CommandLineInterface::SetupFeatureResolution(DescriptorPool& pool) {
   // that support editions must agree on the supported edition range.
   std::vector<const FieldDescriptor*> feature_extensions;
   Edition minimum_edition = PROTOBUF_MINIMUM_EDITION;
-  Edition maximum_edition = PROTOBUF_MAXIMUM_EDITION;
+  // Override maximum_edition if experimental_editions is true.
+  Edition maximum_edition =
+      !experimental_editions_ ? PROTOBUF_MAXIMUM_EDITION : Edition::EDITION_MAX;
   for (const auto& output : output_directives_) {
     if (output.generator == nullptr) continue;
-    if ((output.generator->GetSupportedFeatures() &
+    if (!experimental_editions_ &&
+        (output.generator->GetSupportedFeatures() &
          CodeGenerator::FEATURE_SUPPORTS_EDITIONS) != 0) {
       // Only validate min/max edition on generators that advertise editions
       // support.  Generators still under development will always use the
@@ -2576,15 +2579,11 @@ bool CommandLineInterface::EnforceEditionsSupport(
   for (const auto* fd : parsed_files) {
     Edition edition =
         ::google::protobuf::internal::InternalFeatureHelper::GetEdition(*fd);
-    if (edition < Edition::EDITION_2023) {
-      // Legacy proto2/proto3 files don't need any checks.
+    if (edition < Edition::EDITION_2023 || CanSkipEditionCheck(fd->name())) {
+      // Legacy proto2/proto3 or exempted files don't need any checks.
       continue;
     }
 
-    if (absl::StartsWith(fd->name(), "google/protobuf/") ||
-        absl::StartsWith(fd->name(), "upb/")) {
-      continue;
-    }
     if ((supported_features & CodeGenerator::FEATURE_SUPPORTS_EDITIONS) == 0) {
       std::cerr << absl::Substitute(
           "$0: is an editions file, but code generator $1 hasn't been "

src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -1483,8 +1483,7 @@ TEST_F(CommandLineInterfaceTest, InvalidMinimumEditionError) {
 
   mock_generator_->set_minimum_edition(EDITION_1_TEST_ONLY);
 
-  Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
-      "--experimental_editions foo.proto");
+  Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
   ExpectErrorSubstring(
       "generator --test_out specifies a minimum edition 1_TEST_ONLY which is "
       "not the protoc minimum PROTO2");
@@ -1495,8 +1494,7 @@ TEST_F(CommandLineInterfaceTest, InvalidMaximumEditionError) {
 
   mock_generator_->set_maximum_edition(EDITION_99999_TEST_ONLY);
 
-  Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir "
-      "--experimental_editions foo.proto");
+  Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
   ExpectErrorSubstring(
       "generator --test_out specifies a maximum edition 99999_TEST_ONLY which "
       "is not the protoc maximum 2023");

src/google/protobuf/compiler/cpp/generator.cc

@@ -353,6 +353,7 @@ static bool IsEnumMapType(const FieldDescriptor& field) {
 absl::Status CppGenerator::ValidateFeatures(const FileDescriptor* file) const {
   absl::Status status = absl::OkStatus();
   auto edition = GetEdition(*file);
+  absl::string_view filename = file->name();
 
   google::protobuf::internal::VisitDescriptors(*file, [&](const FieldDescriptor& field) {
     const FeatureSet& resolved_features = GetResolvedSourceFeatures(field);
@@ -382,6 +383,28 @@ absl::Status CppGenerator::ValidateFeatures(const FileDescriptor* file) const {
       }
     }
 
+    if (unresolved_features.has_string_type()) {
+      if (!CanSkipEditionCheck(filename) && edition < Edition::EDITION_2024) {
+        status = absl::FailedPreconditionError(absl::StrCat(
+            "Field ", field.full_name(),
+            " specifies string_type which is not currently allowed."));
+      } else if (field.cpp_type() != FieldDescriptor::CPPTYPE_STRING) {
+        status = absl::FailedPreconditionError(absl::StrCat(
+            "Field ", field.full_name(),
+            " specifies string_type, but is not a string nor bytes field."));
+      } else if (unresolved_features.string_type() == pb::CppFeatures::CORD &&
+                 field.is_extension()) {
+        status = absl::FailedPreconditionError(
+            absl::StrCat("Extension ", field.full_name(),
+                         " specifies string_type=CORD which is not supported "
+                         "for extensions."));
+      } else if (field.options().has_ctype()) {
+        status = absl::FailedPreconditionError(absl::StrCat(
+            field.full_name(),
+            " specifies both string_type and ctype which is not supported."));
+      }
+    }
+
     // 'ctype' check has moved to DescriptorBuilder for Edition 2023 and above.
     if (edition < Edition::EDITION_2023 && field.options().has_ctype()) {
       if (field.cpp_type() != FieldDescriptor::CPPTYPE_STRING) {

src/google/protobuf/compiler/cpp/generator_unittest.cc

@@ -148,6 +148,102 @@ TEST_F(CppGeneratorTest, LegacyClosedEnumImplicit) {
       "Field Foo.bar has a closed enum type with implicit presence.");
 }
 
+TEST_F(CppGeneratorTest, NoStringTypeTillEdition2024) {
+  CreateTempFile("foo.proto", R"schema(
+    edition = "2023";
+    import "google/protobuf/cpp_features.proto";
+
+    message Foo {
+      int32 bar = 1;
+      bytes baz = 2 [features.(pb.cpp).string_type = CORD];
+    }
+  )schema");
+
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir "
+      "--experimental_editions foo.proto");
+
+  ExpectErrorSubstring(
+      "Field Foo.baz specifies string_type which is not currently allowed.");
+}
+
+TEST_F(CppGeneratorTest, StringTypeForCord) {
+  CreateTempFile("foo.proto", R"schema(
+    edition = "2024";
+    import "google/protobuf/cpp_features.proto";
+
+    message Foo {
+      int32 bar = 1;
+      bytes baz = 2 [features.(pb.cpp).string_type = CORD];
+    }
+  )schema");
+
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir "
+      "--experimental_editions foo.proto");
+
+  ExpectNoErrors();
+}
+
+TEST_F(CppGeneratorTest, CtypeForCord) {
+  CreateTempFile("foo.proto", R"schema(
+    edition = "2023";
+
+    message Foo {
+      int32 bar = 1;
+      bytes baz = 2 [ctype = CORD];
+    }
+  )schema");
+
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir "
+      "--experimental_editions foo.proto");
+
+  ExpectNoErrors();
+}
+
+TEST_F(CppGeneratorTest, StringTypeForStringFieldsOnly) {
+  CreateTempFile("foo.proto", R"schema(
+    edition = "2024";
+    import "google/protobuf/cpp_features.proto";
+
+    message Foo {
+      int32 bar = 1;
+      int32 baz = 2 [features.(pb.cpp).string_type = CORD];
+    }
+  )schema");
+
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir "
+      "--experimental_editions foo.proto");
+
+  ExpectErrorSubstring(
+      "Field Foo.baz specifies string_type, but is not a string nor bytes "
+      "field.");
+}
+
+TEST_F(CppGeneratorTest, StringTypeCordNotForExtension) {
+  CreateTempFile("foo.proto", R"schema(
+    edition = "2024";
+    import "google/protobuf/cpp_features.proto";
+
+    message Foo {
+      extensions 1 to max;
+    }
+    extend Foo {
+      bytes bar = 1 [features.(pb.cpp).string_type = CORD];
+    }
+  )schema");
+
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir "
+      "--experimental_editions foo.proto");
+
+  ExpectErrorSubstring(
+      "Extension bar specifies string_type=CORD which is not supported for "
+      "extensions.");
+}
+
 TEST_F(CppGeneratorTest, CtypeOnNoneStringFieldTest) {
   CreateTempFile("foo.proto",
                  R"schema(

src/google/protobuf/cpp_edition_defaults.h

@@ -5,7 +5,7 @@
 // the C++ runtime.  This is used for feature resolution under Editions.
 // NOLINTBEGIN
 // clang-format off
-#define PROTOBUF_INTERNAL_CPP_EDITION_DEFAULTS "\n\030\022\023\010\001\020\002\030\002 \003(\0010\002\302>\004\010\001\020\003\030\346\007\n\030\022\023\010\002\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\030\347\007\n\030\022\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\030\350\007 \346\007(\350\007"
+#define PROTOBUF_INTERNAL_CPP_EDITION_DEFAULTS "\n\030\022\023\010\001\020\002\030\002 \003(\0010\002\302>\004\010\001\020\003\030\346\007\n\030\022\023\010\002\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\030\347\007\n\030\022\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\030\350\007\n\030\022\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\001\030\351\007 \346\007(\351\007"
 // clang-format on
 // NOLINTEND
 

src/google/protobuf/descriptor.cc

@@ -7658,25 +7658,34 @@ void DescriptorBuilder::ValidateOptions(const FieldDescriptor* field,
 
   ValidateFieldFeatures(field, proto);
 
-  if (field->file()->edition() >= Edition::EDITION_2023 &&
-      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()) {
+  auto edition = field->file()->edition();
+  auto& field_options = field->options();
+
+  if (field_options.has_ctype()) {
+    if (edition >= Edition::EDITION_2024) {
+      // "ctype" is no longer supported in edition 2024 and beyond.
+      AddError(field->full_name(), proto, DescriptorPool::ErrorCollector::NAME,
+               "ctype option is not allowed under edition 2024 and beyond. Use "
+               "the feature string_type = VIEW|CORD|STRING|... instead.");
+    } else if (edition == Edition::EDITION_2023) {
+      if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) {
         AddError(field->full_name(), proto,
                  DescriptorPool::ErrorCollector::TYPE,
-                 absl::StrFormat("Extension %s specifies ctype=CORD which is "
-                                 "not supported for extensions.",
+                 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());
+        }
+      }
     }
   }
 
@@ -7866,8 +7875,9 @@ void DescriptorBuilder::ValidateFieldFeatures(
              "message_encoding = DELIMITED to control this behavior.");
   }
 
+  auto& field_options = field->options();
   // Validate legacy options that have been migrated to features.
-  if (field->options().has_packed()) {
+  if (field_options.has_packed()) {
     AddError(field->full_name(), proto, DescriptorPool::ErrorCollector::NAME,
              "Field option packed is not allowed under editions.  Use the "
              "repeated_field_encoding feature to control this behavior.");

src/google/protobuf/descriptor_unittest.cc

@@ -7660,6 +7660,32 @@ TEST_F(FeaturesTest, Edition2023Defaults) {
   EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 1);
 }
 
+TEST_F(FeaturesTest, Edition2024Defaults) {
+  FileDescriptorProto file_proto = ParseTextOrDie(R"pb(
+    name: "foo.proto"
+    syntax: "editions"
+    edition: EDITION_2024
+  )pb");
+
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* file = ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto));
+  EXPECT_THAT(file->options(), EqualsProto(""));
+  EXPECT_THAT(GetCoreFeatures(file), EqualsProto(R"pb(
+                field_presence: EXPLICIT
+                enum_type: OPEN
+                repeated_field_encoding: PACKED
+                utf8_validation: VERIFY
+                message_encoding: LENGTH_PREFIXED
+                json_format: ALLOW
+                [pb.cpp] { legacy_closed_enum: false string_type: VIEW }
+              )pb"));
+
+  // Since pb::test is registered in the pool, it should end up with defaults in
+  // our FeatureSet.
+  EXPECT_TRUE(GetFeatures(file).HasExtension(pb::test));
+  EXPECT_EQ(GetFeatures(file).GetExtension(pb::test).int_file_feature(), 1);
+}
+
 TEST_F(FeaturesBaseTest, DefaultEdition2023Defaults) {
   BuildDescriptorMessagesInTestPool();
   BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
@@ -9442,6 +9468,30 @@ TEST_F(FeaturesTest, InvalidFieldPacked) {
       "behavior.\n");
 }
 
+TEST_F(FeaturesTest, NoCtypeFromEdition2024) {
+  BuildDescriptorMessagesInTestPool();
+  BuildFileWithErrors(
+      R"pb(
+        name: "foo.proto"
+        syntax: "editions"
+        edition: EDITION_2024
+        message_type {
+          name: "Foo"
+          field { name: "foo" number: 1 label: LABEL_OPTIONAL type: TYPE_INT32 }
+          field {
+            name: "bar"
+            number: 2
+            label: LABEL_OPTIONAL
+            type: TYPE_STRING
+            options { ctype: CORD }
+          }
+        }
+      )pb",
+      "foo.proto: Foo.bar: NAME: ctype option is not allowed under edition "
+      "2024 and beyond. Use the feature string_type = VIEW|CORD|STRING|... "
+      "instead.\n");
+}
+
 
 TEST_F(FeaturesTest, InvalidFieldImplicitDefault) {
   BuildDescriptorMessagesInTestPool();