commit 4554699ee5e162806b10a4b630f23cdcee63d683
author Mike Kruskal <mkruskal@google.com> Tue Oct 10 11:42:01 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 10 11:43:49 2023 -0700
tree 3fae8ac8d14a694bf8fac20d32584aade9e564c4
parent 97a239446cbd1f856cb4ae509cfc2725f5c31058

Prohibit setting LEGACY_REQUIRED by default at the file level

PiperOrigin-RevId: 572320452

src/google/protobuf/descriptor.cc

diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
index 8ed805b..87a7b5f 100644
--- a/src/google/protobuf/descriptor.cc
+++ b/src/google/protobuf/descriptor.cc
@@ -4540,6 +4540,8 @@
   // AddError(). Do not look at their options, which have not been interpreted.
   void ValidateOptions(const FileDescriptor* file,
                        const FileDescriptorProto& proto);
+  void ValidateFileFeatures(const FileDescriptor* file,
+                            const FileDescriptorProto& proto);
   void ValidateOptions(const Descriptor* message, const DescriptorProto& proto);
   void ValidateOptions(const OneofDescriptor* oneof,
                        const OneofDescriptorProto& proto);
@@ -7655,6 +7657,8 @@
 
 void DescriptorBuilder::ValidateOptions(const FileDescriptor* file,
                                         const FileDescriptorProto& proto) {
+  ValidateFileFeatures(file, proto);
+
   // Lite files can only be imported by other Lite files.
   if (!IsLite(file)) {
     for (int i = 0; i < file->dependency_count(); i++) {
@@ -7915,6 +7919,14 @@
   return false;
 }
 
+void DescriptorBuilder::ValidateFileFeatures(const FileDescriptor* file,
+                                             const FileDescriptorProto& proto) {
+  if (file->features().field_presence() == FeatureSet::LEGACY_REQUIRED) {
+    AddError(file->name(), proto, DescriptorPool::ErrorCollector::EDITIONS,
+             "Required presence can't be specified by default.");
+  }
+}
+
 void DescriptorBuilder::ValidateFieldFeatures(
     const FieldDescriptor* field, const FieldDescriptorProto& proto) {
   // Rely on our legacy validation for proto2/proto3 files.

src/google/protobuf/descriptor_unittest.cc

diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
index 10aea09..4f75639 100644
--- a/src/google/protobuf/descriptor_unittest.cc
+++ b/src/google/protobuf/descriptor_unittest.cc
@@ -9256,6 +9256,18 @@
       "editions.\n");
 }
 
+TEST_F(FeaturesTest, InvalidRequiredByDefault) {
+  BuildDescriptorMessagesInTestPool();
+  BuildFileWithErrors(
+      R"pb(
+        name: "foo.proto"
+        syntax: "editions"
+        edition: EDITION_2023
+        options { features { field_presence: LEGACY_REQUIRED } }
+      )pb",
+      "foo.proto: foo.proto: EDITIONS: Required presence can't be specified "
+      "by default.\n");
+}
 TEST_F(FeaturesTest, InvalidFieldPacked) {
   BuildDescriptorMessagesInTestPool();
   BuildFileWithErrors(