From 29a6a2189aff98bd34417bb2455ac71889796ef0 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 27 Dec 2022 10:47:09 -0800 Subject: [PATCH] Change the logic for reserved field name verification to force a fully qualified type name (ie starts with `.`). This way we guarantee that the spelling the `.proto` file will be the same as the spelling we see after crosslinking types. Otherwise, the spellings might differ and it makes it harder to determine if the name in the allowlist matches the name in the `.proto` file. PiperOrigin-RevId: 497997383 --- src/google/protobuf/descriptor.cc | 8 +++- src/google/protobuf/descriptor_unittest.cc | 47 ++++++++++++++++------ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 8157e3f269..a0cc32baab 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -5860,9 +5860,13 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::NUMBER, absl::Substitute( "Field numbers $0 through $1 are reserved for the protocol " - "buffer library implementation.", + "buffer library implementation$2.", FieldDescriptor::kFirstReservedNumber, - FieldDescriptor::kLastReservedNumber)); + 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 1c0b399fea..89bd484f2c 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -4585,21 +4585,42 @@ TEST_F(ValidationErrorTest, HugeFieldNumber) { TEST_F(ValidationErrorTest, ReservedFieldNumber) { BuildFileWithErrors( - "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_INT32 }" - " field {name:\"baz\" number: 19999 label:LABEL_OPTIONAL " - "type:TYPE_INT32 }" - " field {name:\"moo\" number: 20000 label:LABEL_OPTIONAL " - "type:TYPE_INT32 }" - "}", + 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.\n" + "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");