Move the reserved field number check into protoc and remove it from the
runtime.
The runtime should work with all the fields, so that older binaries can load
descriptors made by newer protoc.
PiperOrigin-RevId: 507776470
diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc
index 318b76f..0fcdbe1 100644
--- a/src/google/protobuf/compiler/command_line_interface.cc
+++ b/src/google/protobuf/compiler/command_line_interface.cc
@@ -970,6 +970,69 @@
return false;
}
+template <typename Visitor>
+struct VisitImpl {
+ Visitor visitor;
+ void Visit(const FieldDescriptor* descriptor) { visitor(descriptor); }
+
+ void Visit(const EnumDescriptor* descriptor) { visitor(descriptor); }
+
+ void Visit(const Descriptor* descriptor) {
+ visitor(descriptor);
+
+ for (int i = 0; i < descriptor->enum_type_count(); i++) {
+ Visit(descriptor->enum_type(i));
+ }
+
+ for (int i = 0; i < descriptor->field_count(); i++) {
+ Visit(descriptor->field(i));
+ }
+
+ for (int i = 0; i < descriptor->nested_type_count(); i++) {
+ Visit(descriptor->nested_type(i));
+ }
+
+ for (int i = 0; i < descriptor->extension_count(); i++) {
+ Visit(descriptor->extension(i));
+ }
+ }
+
+ void Visit(const std::vector<const FileDescriptor*>& descriptors) {
+ for (auto* descriptor : descriptors) {
+ visitor(descriptor);
+ for (int i = 0; i < descriptor->message_type_count(); i++) {
+ Visit(descriptor->message_type(i));
+ }
+ for (int i = 0; i < descriptor->enum_type_count(); i++) {
+ Visit(descriptor->enum_type(i));
+ }
+ for (int i = 0; i < descriptor->extension_count(); i++) {
+ Visit(descriptor->extension(i));
+ }
+ }
+ }
+};
+
+// Visit every node in the descriptors calling `visitor(node)`.
+// The visitor does not need to handle all possible node types. Types that are
+// not visitable via `visitor` will be ignored.
+// Disclaimer: this is not fully implemented yet to visit _every_ node.
+// VisitImpl might need to be updated where needs arise.
+template <typename Visitor>
+void VisitDescriptors(const std::vector<const FileDescriptor*>& descriptors,
+ Visitor visitor) {
+ // Provide a fallback to ignore all the nodes that are not interesting to the
+ // input visitor.
+ struct VisitorImpl : Visitor {
+ explicit VisitorImpl(Visitor visitor) : Visitor(visitor) {}
+ using Visitor::operator();
+ // Honeypot to ignore all inputs that Visitor does not take.
+ void operator()(const void*) const {}
+ };
+
+ VisitImpl<VisitorImpl>{VisitorImpl(visitor)}.Visit(descriptors);
+}
+
} // namespace
namespace {
@@ -1063,6 +1126,28 @@
return 1;
}
+ bool validation_error = false; // Defer exiting so we log more warnings.
+
+ VisitDescriptors(parsed_files, [&](const FieldDescriptor* field) {
+ if (field->number() >= FieldDescriptor::kFirstReservedNumber &&
+ field->number() <= FieldDescriptor::kLastReservedNumber) {
+ validation_error = true;
+ static_cast<DescriptorPool::ErrorCollector*>(error_collector.get())
+ ->RecordError(
+ field->file()->name(), field->full_name(), nullptr,
+ DescriptorPool::ErrorCollector::NUMBER,
+ absl::Substitute("Field numbers $0 through $1 are reserved "
+ "for the protocol "
+ "buffer library implementation.",
+ FieldDescriptor::kFirstReservedNumber,
+ FieldDescriptor::kLastReservedNumber));
+ }
+ });
+
+
+ if (validation_error) {
+ return 1;
+ }
// We construct a separate GeneratorContext for each output location. Note
// that two code generators may output to the same location, in which case
diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
index 967b510..c89f473 100644
--- a/src/google/protobuf/compiler/command_line_interface_unittest.cc
+++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -2465,6 +2465,79 @@
"optional fields in proto3");
}
+TEST_F(CommandLineInterfaceTest, ReservedFieldNumbersFail) {
+ CreateTempFile("foo.proto",
+ R"(
+syntax = "proto2";
+message Foo {
+ optional int32 i = 19123;
+}
+)");
+
+ Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto");
+
+ ExpectErrorSubstring(
+ "foo.proto: Field numbers 19000 through 19999 are reserved for the "
+ "protocol buffer library implementation.");
+}
+
+TEST_F(CommandLineInterfaceTest, ReservedFieldNumbersFailAsOneof) {
+ CreateTempFile("foo.proto",
+ R"(
+syntax = "proto2";
+message Foo {
+ oneof one {
+ int32 i = 19123;
+ }
+}
+)");
+
+ Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto");
+
+ ExpectErrorSubstring(
+ "foo.proto: Field numbers 19000 through 19999 are reserved for the "
+ "protocol buffer library implementation.");
+}
+
+TEST_F(CommandLineInterfaceTest, ReservedFieldNumbersFailAsExtension) {
+ CreateTempFile("foo.proto",
+ R"(
+syntax = "proto2";
+message Foo {
+ extensions 4 to max;
+}
+extend Foo {
+ optional int32 i = 19123;
+}
+)");
+
+ Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto");
+
+ ExpectErrorSubstring(
+ "foo.proto: Field numbers 19000 through 19999 are reserved for the "
+ "protocol buffer library implementation.");
+
+ CreateTempFile("foo.proto",
+ R"(
+syntax = "proto2";
+message Foo {
+ extensions 4 to max;
+}
+message Bar {
+ extend Foo {
+ optional int32 i = 19123;
+ }
+}
+)");
+
+ Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto");
+
+ ExpectErrorSubstring(
+ "foo.proto: Field numbers 19000 through 19999 are reserved for the "
+ "protocol buffer library implementation.");
+}
+
+
TEST_F(CommandLineInterfaceTest, Proto3OptionalAllowWithFlag) {
CreateTempFile("google/foo.proto",
"syntax = \"proto3\";\n"
diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc
index a7b33fc..9e3a49d 100644
--- a/src/google/protobuf/descriptor.cc
+++ b/src/google/protobuf/descriptor.cc
@@ -5638,12 +5638,6 @@
}
}
-namespace {
-bool IsAllowedReservedField(const FieldDescriptorProto& field) {
- return false;
-}
-} // namespace
-
void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
Descriptor* parent,
FieldDescriptor* result,
@@ -5871,21 +5865,6 @@
AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::NUMBER,
absl::Substitute("Field numbers cannot be greater than $0.",
FieldDescriptor::kMaxNumber));
- } else if (result->number() >= FieldDescriptor::kFirstReservedNumber &&
- result->number() <= FieldDescriptor::kLastReservedNumber &&
- !IsAllowedReservedField(proto)) {
- message_hints_[parent].RequestHintOnFieldNumbers(
- proto, DescriptorPool::ErrorCollector::NUMBER);
- AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::NUMBER,
- absl::Substitute(
- "Field numbers $0 through $1 are reserved for the protocol "
- "buffer library implementation$2.",
- FieldDescriptor::kFirstReservedNumber,
- FieldDescriptor::kLastReservedNumber,
- absl::StartsWith(proto.type_name(), ".")
- ? ""
- : ", and the type name must be fully qualified with a `.` "
- "prefix"));
}
if (is_extension) {
diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc
index 1828d2c..80eebfe 100644
--- a/src/google/protobuf/descriptor_unittest.cc
+++ b/src/google/protobuf/descriptor_unittest.cc
@@ -4591,50 +4591,6 @@
"foo.proto: Foo: NUMBER: Suggested field numbers for Foo: 1\n");
}
-TEST_F(ValidationErrorTest, ReservedFieldNumber) {
- BuildFileWithErrors(
- R"pb(
- name: "foo.proto"
- message_type {
- name: "Foo"
- field {
- name: "foo"
- number: 18999
- label: LABEL_OPTIONAL
- type: TYPE_INT32
- }
- field {
- name: "bar"
- number: 19000
- label: LABEL_OPTIONAL
- type: TYPE_MESSAGE
- type_name: "Foo"
- }
- field {
- name: "baz"
- number: 19999
- label: LABEL_OPTIONAL
- type: TYPE_MESSAGE
- type_name: ".Foo"
- }
- field {
- name: "moo"
- number: 20000
- label: LABEL_OPTIONAL
- type: TYPE_INT32
- }
- }
- )pb",
-
- "foo.proto: Foo.bar: NUMBER: Field numbers 19000 through 19999 are "
- "reserved for the protocol buffer library implementation, and the type "
- "name must be fully qualified with a `.` prefix.\n"
- "foo.proto: Foo.baz: NUMBER: Field numbers 19000 through 19999 are "
- "reserved for the protocol buffer library implementation.\n"
- "foo.proto: Foo: NUMBER: Suggested field numbers for Foo: 1, 2\n");
-}
-
-
TEST_F(ValidationErrorTest, ExtensionMissingExtendee) {
BuildFileWithErrors(
"name: \"foo.proto\" "
@@ -7379,7 +7335,7 @@
TEST_F(DatabaseBackedPoolTest, UndeclaredDependencyOnUnbuiltType) {
// Check that we find and report undeclared dependencies on types that exist
- // in the descriptor database but that have not not been built yet.
+ // in the descriptor database but that have not been built yet.
MockErrorCollector error_collector;
DescriptorPool pool(&database_, &error_collector);
EXPECT_TRUE(pool.FindMessageTypeByName("Baz") == nullptr);
@@ -8548,7 +8504,7 @@
"google/protobuf/unittest_lazy_dependencies_enum.proto"));
// Verify calling autogenerated function to get a descriptor in the base
- // file will build that file but none of it's imports. This verifies that
+ // file will build that file but none of its imports. This verifies that
// lazily_build_dependencies_ is set on the generated pool, and also that
// the generated function "descriptor()" doesn't somehow subvert the laziness
// by manually loading the dependencies or something.
@@ -8612,7 +8568,7 @@
const FileDescriptor* foo_file = pool_.FindFileByName("foo.proto");
EXPECT_TRUE(foo_file != nullptr);
- // As expected, requesting foo.proto shouldn't build it's dependencies
+ // As expected, requesting foo.proto shouldn't build its dependencies
EXPECT_TRUE(pool_.InternalIsFileLoaded("foo.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("bar.proto"));
EXPECT_FALSE(pool_.InternalIsFileLoaded("baz.proto"));
