From e72b0e181b236ff23c03a9f9f97f58e6989f25c9 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 22 Aug 2023 13:00:37 -0700 Subject: [PATCH] Publish extension declarations with declaration verifications. PiperOrigin-RevId: 559199526 --- .../compiler/command_line_interface.cc | 6 + .../command_line_interface_unittest.cc | 314 ++++++++ src/google/protobuf/descriptor.cc | 112 ++- src/google/protobuf/descriptor.h | 10 + src/google/protobuf/descriptor_unittest.cc | 684 ++++++++++++++++++ 5 files changed, 1125 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 36d39f5bb4f4..39c7cdc3c4c8 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -1269,6 +1269,12 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } descriptor_pool->EnforceWeakDependencies(true); + + // Enforce extension declarations only when compiling. We want to skip this + // enforcement when protoc is just being invoked to encode or decode protos. + if (mode_ == MODE_COMPILE) { + descriptor_pool->EnforceExtensionDeclarations(true); + } if (!ParseInputFiles(descriptor_pool.get(), disk_source_tree.get(), &parsed_files)) { return 1; diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 951632fff1ab..1d6d288297f6 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -3155,6 +3155,320 @@ TEST_F(CommandLineInterfaceTest, "Option protobuf_unittest.B.i cannot be set on an entity of type `file`."); } + +TEST_F(CommandLineInterfaceTest, ExtensionDeclarationEnforcement) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + full_name: ".foo.o" + type: "int32" + }, + declaration = { + number: 9000, + full_name: ".baz.z" + type: ".foo.Bar" + }]; + } + + extend Foo { + optional int32 o = 5000; + repeated int32 i = 9000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "extension field 9000 is expected to be type \".foo.Bar\", not " + "\"int32\""); + ExpectErrorSubstring( + "extension field 9000 is expected to have field name \".baz.z\", not " + "\".foo.i\""); + ExpectErrorSubstring("extension field 9000 is expected to be optional"); +} + +TEST_F(CommandLineInterfaceTest, ExtensionDeclarationDuplicateNames) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + full_name: ".foo.o" + type: "int32" + }, + declaration = { + number: 9000, + full_name: ".foo.o" + type: ".foo.Bar" + }]; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Extension field name \".foo.o\" is declared multiple times"); +} + +TEST_F(CommandLineInterfaceTest, ExtensionDeclarationDuplicateNumbers) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + full_name: ".foo.o" + type: "int32" + }, + declaration = { + number: 5000, + full_name: ".foo.o" + type: ".foo.Bar" + }]; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Extension declaration number 5000 is declared multiple times"); +} + +TEST_F(CommandLineInterfaceTest, ExtensionDeclarationCannotUseReservedNumber) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + reserved: true + full_name: ".foo.o" + type: "int32" + }]; + } + + extend Foo { + optional int32 o = 5000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Cannot use number 5000 for extension field foo.o, as it is reserved in " + "the extension declarations for message foo.Foo."); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationReservedMissingOneOfNameAndType) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + reserved: true + type: "int32" + }]; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Extension declaration #5000 should have both \"full_name\" and \"type\" " + "set"); +} + +TEST_F(CommandLineInterfaceTest, ExtensionDeclarationMissingBothNameAndType) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 6000 + }]; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Extension declaration #6000 should have both \"full_name\" and \"type\" " + "set"); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationReservedOptionalNameAndType) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + declaration = { + number: 5000, + reserved: true + full_name: ".foo.o" + type: "int32" + }, + declaration = { + number: 9000, + reserved: true + }]; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectNoErrors(); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationRequireDeclarationsForAll) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ declaration = { + number: 5000, + full_name: ".foo.o" + type: "int32" + }]; + } + + extend Foo { + optional int32 o = 5000; + repeated int32 i = 9000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Missing extension declaration for field foo.i with number 9000 in " + "extendee message foo.Foo"); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationVerificationDeclarationUndeclaredError) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [verification = DECLARATION]; + } + extend Foo { + optional string my_field = 5000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Missing extension declaration for field foo.my_field with number 5000 " + "in extendee message foo.Foo"); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationVerificationDeclarationDeclaredCompile) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + verification = DECLARATION, + declaration = { + number: 5000, + full_name: ".foo.my_field", + type: "string" + }]; + } + extend Foo { + optional string my_field = 5000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectNoErrors(); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationUnverifiedWithDeclarationsError) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max [ + verification = UNVERIFIED, + declaration = { + number: 5000, + full_name: "foo.my_field", + type: "string" + }]; + } + extend Foo { + optional string my_field = 5000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectErrorSubstring( + "Cannot mark the extension range as UNVERIFIED when it has extension(s) " + "declared."); +} + +TEST_F(CommandLineInterfaceTest, + ExtensionDeclarationDefaultUnverifiedEmptyRange) { + CreateTempFile("foo.proto", R"schema( + syntax = "proto2"; + package foo; + message Foo { + extensions 4000 to max; + } + extend Foo { + optional string my_field = 5000; + })schema"); + + Run("protocol_compiler --test_out=$tmpdir --proto_path=$tmpdir foo.proto"); + ExpectNoErrors(); +} + +// Returns true if x is a prefix of y. +bool IsPrefix(absl::Span x, absl::Span y) { + return x.size() <= y.size() && x == y.subspan(0, x.size()); +} + +TEST_F(CommandLineInterfaceTest, SourceInfoOptionRetention) { + CreateTempFile("foo.proto", + "syntax = \"proto2\";\n" + "message Foo {\n" + " extensions 1000 to max [\n" + " declaration = {\n" + " number: 1000\n" + " full_name: \".video.cat_video\"\n" + " type: \".video.CatVideo\"\n" + " }];\n" + "}\n"); + + Run("protocol_compiler --descriptor_set_out=$tmpdir/descriptor_set " + "--include_source_info --proto_path=$tmpdir foo.proto"); + + ExpectNoErrors(); + + FileDescriptorSet descriptor_set; + ReadDescriptorSet("descriptor_set", &descriptor_set); + if (HasFatalFailure()) return; + ASSERT_EQ(descriptor_set.file_size(), 1); + EXPECT_EQ(descriptor_set.file(0).name(), "foo.proto"); + + // Everything starting with this path should be have been stripped from the + // source code info. + const int declaration_option_path[] = { + FileDescriptorProto::kMessageTypeFieldNumber, + 0, + DescriptorProto::kExtensionRangeFieldNumber, + 0, + DescriptorProto::ExtensionRange::kOptionsFieldNumber, + ExtensionRangeOptions::kDeclarationFieldNumber}; + + const SourceCodeInfo& source_code_info = + descriptor_set.file(0).source_code_info(); + EXPECT_GT(source_code_info.location_size(), 0); + for (const SourceCodeInfo::Location& location : source_code_info.location()) { + EXPECT_FALSE(IsPrefix(declaration_option_path, location.path())); + } +} + // =================================================================== // Test for --encode and --decode. Note that it would be easier to do this diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 0d4db726cab3..54e6b7fd99f1 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -2069,6 +2069,22 @@ void DescriptorPool::AddUnusedImportTrackFile(absl::string_view file_name, unused_import_track_files_[file_name] = is_error; } +bool DescriptorPool::IsExtendingDescriptor(const FieldDescriptor& field) const { + static const auto& kDescriptorTypes = *new absl::flat_hash_set({ + "google.protobuf.EnumOptions", + "google.protobuf.EnumValueOptions", + "google.protobuf.ExtensionRangeOptions", + "google.protobuf.FieldOptions", + "google.protobuf.FileOptions", + "google.protobuf.MessageOptions", + "google.protobuf.MethodOptions", + "google.protobuf.OneofOptions", + "google.protobuf.ServiceOptions", + "google.protobuf.StreamOptions", + }); + return kDescriptorTypes.contains(field.containing_type()->full_name()); +} + void DescriptorPool::ClearUnusedImportTrackFiles() { unused_import_track_files_.clear(); @@ -4334,6 +4350,12 @@ class DescriptorBuilder { absl::string_view declared_full_name, absl::string_view declared_type_name, bool is_repeated); + // Checks that the extension field type matches the declared type. It also + // handles message types that look like non-message types such as "fixed64" vs + // ".fixed64". + void CheckExtensionDeclarationFieldType(const FieldDescriptor& field, + const FieldDescriptorProto& proto, + absl::string_view type); // A helper class for interpreting options. class OptionInterpreter { @@ -7135,12 +7157,44 @@ void DescriptorBuilder::CrossLinkExtensionRange( } } +void DescriptorBuilder::CheckExtensionDeclarationFieldType( + const FieldDescriptor& field, const FieldDescriptorProto& proto, + absl::string_view type) { + if (had_errors_) return; + std::string actual_type = field.type_name(); + std::string expected_type(type); + if (field.message_type() || field.enum_type()) { + // Field message type descriptor can be in a partial state which will cause + // segmentation fault if it is being accessed. + if (had_errors_) return; + absl::string_view full_name = field.message_type() != nullptr + ? field.message_type()->full_name() + : field.enum_type()->full_name(); + actual_type = absl::StrCat(".", full_name); + } + if (!IsNonMessageType(type) && !absl::StartsWith(type, ".")) { + expected_type = absl::StrCat(".", type); + } + if (expected_type != actual_type) { + AddError(field.full_name(), proto, DescriptorPool::ErrorCollector::EXTENDEE, + [&] { + return absl::Substitute( + "\"$0\" extension field $1 is expected to be type " + "\"$2\", not \"$3\".", + field.containing_type()->full_name(), field.number(), + expected_type, actual_type); + }); + } +} + void DescriptorBuilder::CheckExtensionDeclaration( const FieldDescriptor& field, const FieldDescriptorProto& proto, absl::string_view declared_full_name, absl::string_view declared_type_name, bool is_repeated) { - + if (!declared_type_name.empty()) { + CheckExtensionDeclarationFieldType(field, proto, declared_type_name); + } if (!declared_full_name.empty()) { std::string actual_full_name = absl::StrCat(".", field.full_name()); if (declared_full_name != actual_full_name) { @@ -7812,6 +7866,62 @@ void DescriptorBuilder::ValidateOptions(const FieldDescriptor* field, "json_name cannot have embedded null characters."); } + + // If this is a declared extension, validate that the actual name and type + // match the declaration. + if (field->is_extension()) { + if (pool_->IsExtendingDescriptor(*field)) return; + const Descriptor::ExtensionRange* extension_range = + field->containing_type()->FindExtensionRangeContainingNumber( + field->number()); + + if (extension_range->options_ == nullptr) { + return; + } + + if (pool_->enforce_extension_declarations_) { + for (const auto& declaration : extension_range->options_->declaration()) { + if (declaration.number() != field->number()) continue; + if (declaration.reserved()) { + AddError( + field->full_name(), proto, + DescriptorPool::ErrorCollector::EXTENDEE, [&] { + return absl::Substitute( + "Cannot use number $0 for extension field $1, as it is " + "reserved in the extension declarations for message $2.", + field->number(), field->full_name(), + field->containing_type()->full_name()); + }); + return; + } + CheckExtensionDeclaration(*field, proto, declaration.full_name(), + declaration.type(), declaration.repeated()); + return; + } + + // Either no declarations, or there are but no matches. If there are no + // declarations, we check its verification state. If there are other + // non-matching declarations, we enforce that this extension must also be + // declared. + if (!extension_range->options_->declaration().empty() || + (extension_range->options_->verification() == + ExtensionRangeOptions::DECLARATION)) { + AddError( + field->full_name(), proto, DescriptorPool::ErrorCollector::EXTENDEE, + [&] { + return absl::Substitute( + "Missing extension declaration for field $0 with number $1 " + "in extendee message $2. An extension range must declare for " + "all extension fields if its verification state is " + "DECLARATION or there's any declaration in the range " + "already. Otherwise, consider splitting up the range.", + field->full_name(), field->number(), + field->containing_type()->full_name()); + }); + return; + } + } + } } void DescriptorBuilder::ValidateFieldFeatures( diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index ab6bb3d26318..15e005729832 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -2289,6 +2289,13 @@ class PROTOBUF_EXPORT DescriptorPool { // DescriptorPool will report a import not found error. void EnforceWeakDependencies(bool enforce) { enforce_weak_ = enforce; } + // Toggles enforcement of extension declarations. + // This enforcement is disabled by default because it requires full + // descriptors with source-retention options, which are generally not + // available at runtime. + void EnforceExtensionDeclarations(bool enforce) { + enforce_extension_declarations_ = enforce; + } // Internal stuff -------------------------------------------------- // These methods MUST NOT be called from outside the proto2 library. // These methods may contain hidden pitfalls and may be removed in a @@ -2467,6 +2474,9 @@ class PROTOBUF_EXPORT DescriptorPool { // unused imports are treated as errors (and as warnings when false). absl::flat_hash_map unused_import_track_files_; + // Returns true if the field extends an option message of descriptor.proto. + bool IsExtendingDescriptor(const FieldDescriptor& field) const; + }; diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 96006c4753bf..90b772fd4804 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -58,6 +58,8 @@ #include "absl/log/absl_log.h" #include "absl/log/die_if_null.h" #include "absl/log/scoped_mock_log.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "absl/strings/substitute.h" @@ -87,6 +89,18 @@ using ::testing::ElementsAre; using ::testing::HasSubstr; using ::testing::NotNull; +absl::Status GetStatus(const absl::Status& s) { return s; } +template +absl::Status GetStatus(const absl::StatusOr& s) { + return s.status(); +} +MATCHER_P(StatusIs, status, + absl::StrCat(".status() is ", testing::PrintToString(status))) { + return GetStatus(arg).code() == status; +} +#define EXPECT_OK(x) EXPECT_THAT(x, StatusIs(absl::StatusCode::kOk)) +#define ASSERT_OK(x) ASSERT_THAT(x, StatusIs(absl::StatusCode::kOk)) + namespace google { namespace protobuf { @@ -3997,6 +4011,11 @@ TEST(CustomOptions, DebugString) { class ValidationErrorTest : public testing::Test { protected: + void SetUp() override { + // Enable extension declaration enforcement since most test cases want to + // exercise the full validation. + pool_.EnforceExtensionDeclarations(true); + } // Parse file_text as a FileDescriptorProto in text format and add it // to the DescriptorPool. Expect no errors. const FileDescriptor* BuildFile(const std::string& file_text) { @@ -10000,6 +10019,671 @@ INSTANTIATE_TEST_SUITE_P( +TEST_F(ValidationErrorTest, ExtensionDeclarationsMatchFullNameCompile) { + BuildFile(R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 11 + end: 999 + options: { + declaration: { + number: 100 + full_name: ".ext.test.foo" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "foo" number: 100 type_name: "Bar" } + )pb"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationsMismatchFullName) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 11 + end: 999 + options: { + declaration: { + number: 100 + full_name: ".ext.test.buz" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "foo" number: 100 type_name: "Bar" } + )pb", + "foo.proto: ext.test.foo: EXTENDEE: \"ext.test.Foo\" extension field 100" + " is expected to have field name \".ext.test.buz\", not " + "\".ext.test.foo\".\n"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationsMismatchFullNameAllowed) { + // Make sure that extension declaration names and types are not validated + // outside of protoc. This is important for allowing extensions to be renamed + // safely. + pool_.EnforceExtensionDeclarations(false); + BuildFile( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 11 + end: 999 + options: { + declaration: { + number: 100 + full_name: ".ext.test.buz" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "foo" number: 100 type_name: "Bar" } + )pb"); +} + +TEST_F(ValidationErrorTest, + ExtensionDeclarationsFullNameDoesNotLookLikeIdentifier) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { + number: 10 + full_name: ".ext..test.bar" + type: ".baz" + } + } + } + } + )pb", + "foo.proto: Foo: NAME: \".ext..test.bar\" contains invalid " + "identifiers.\n"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationsDuplicateNames) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 11 + end: 1000 + options: { + declaration: { + number: 123 + full_name: ".foo.Bar.baz", + type: ".Bar" + } + declaration: { + number: 999 + full_name: ".foo.Bar.baz", + type: "int32" + } + } + } + } + )pb", + "foo.proto: .foo.Bar.baz: NAME: Extension field name \".foo.Bar.baz\" is " + "declared multiple times.\n"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationMissingFullNameOrType) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { declaration: { number: 10 full_name: ".foo.Bar.foo" } } + } + extension_range { + start: 11 + end: 12 + options: { declaration: { number: 11 type: ".Baz" } } + } + } + )pb", + "foo.proto: Foo: EXTENDEE: Extension declaration #10 should have both" + " \"full_name\" and \"type\" set.\n" + "foo.proto: Foo: EXTENDEE: Extension declaration #11 should have both" + " \"full_name\" and \"type\" set.\n"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationsNumberNotInRange) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 9999 + options: { + declaration: { number: 9999 full_name: ".abc" type: ".Bar" } + } + } + } + )pb", + "foo.proto: Foo: NUMBER: Extension declaration number 9999 is not in the " + "extension range.\n"); +} + +TEST_F(ValidationErrorTest, ExtensionDeclarationsFullNameMissingLeadingDot) { + BuildFileWithErrors( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 9999 + options: { + declaration: { number: 10 full_name: "bar" type: "fixed64" } + } + } + } + )pb", + "foo.proto: Foo: NAME: \"bar\" must have a leading dot to indicate the " + "fully-qualified scope.\n"); +} + +struct ExtensionDeclarationsTestParams { + std::string test_name; +}; + +// For OSS, we only have declaration to test with. +using ExtensionDeclarationsTest = + testing::TestWithParam; + +// For OSS, this is a function that directly returns the parsed +// FileDescriptorProto. +absl::StatusOr ParameterizeFileProto( + absl::string_view file_text, const ExtensionDeclarationsTestParams& param) { + (void)file_text; // Parameter is used by Google-internal code. + (void)param; // Parameter is used by Google-internal code. + FileDescriptorProto file_proto; + if (!TextFormat::ParseFromString(file_text, &file_proto)) { + return absl::InvalidArgumentError("Failed to parse the input file text."); + } + + return file_proto; +} + +TEST_P(ExtensionDeclarationsTest, DotPrefixTypeCompile) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "bar" number: 10 type_name: "Bar" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + EXPECT_NE(pool.BuildFile(*file_proto), nullptr); +} + +TEST_P(ExtensionDeclarationsTest, EnumTypeCompile) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".ext.test.Bar" + } + } + } + } + enum_type { + name: "Bar" + value: { name: "BUZ" number: 123 } + } + extension { extendee: "Foo" name: "bar" number: 10 type_name: "Bar" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + EXPECT_NE(pool.BuildFile(*file_proto), nullptr); +} + +TEST_P(ExtensionDeclarationsTest, MismatchEnumType) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".ext.test.Bar" + } + } + } + } + enum_type { + name: "Bar" + value: { name: "BUZ1" number: 123 } + } + enum_type { + name: "Abc" + value: { name: "BUZ2" number: 456 } + } + extension { extendee: "Foo" name: "bar" number: 10 type_name: "Abc" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ( + error_collector.text_, + "foo.proto: ext.test.bar: EXTENDEE: \"ext.test.Foo\" extension field 10 " + "is expected to be type \".ext.test.Bar\", not \".ext.test.Abc\".\n"); +} + +TEST_P(ExtensionDeclarationsTest, DotPrefixFullNameCompile) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "bar" number: 10 type_name: "Bar" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + EXPECT_NE(pool.BuildFile(*file_proto), nullptr); +} + +TEST_P(ExtensionDeclarationsTest, MismatchDotPrefixTypeExpectingMessage) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".int32" + } + } + } + } + extension { name: "bar" number: 10 type: TYPE_INT32 extendee: "Foo" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: ext.test.bar: EXTENDEE: \"ext.test.Foo\" extension " + "field 10 is expected to be type \".int32\", not \"int32\".\n"); +} + +TEST_P(ExtensionDeclarationsTest, MismatchDotPrefixTypeExpectingNonMessage) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { number: 10 full_name: ".bar" type: "int32" } + } + } + } + message_type { name: "int32" } + extension { name: "bar" number: 10 type_name: "int32" extendee: "Foo" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: bar: EXTENDEE: \"Foo\" extension field 10 is expected " + "to be type \"int32\", not \".int32\".\n"); +} + +TEST_P(ExtensionDeclarationsTest, MismatchMessageType) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.foo" + type: ".ext.test.Foo" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "foo" number: 10 type_name: "Bar" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ( + error_collector.text_, + "foo.proto: ext.test.foo: EXTENDEE: \"ext.test.Foo\" extension field 10 " + "is expected to be type \".ext.test.Foo\", not \".ext.test.Bar\".\n"); +} + +TEST_P(ExtensionDeclarationsTest, NonMessageTypeCompile) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { number: 10 full_name: ".bar" type: "fixed64" } + } + } + } + extension { name: "bar" number: 10 type: TYPE_FIXED64 extendee: "Foo" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + EXPECT_NE(pool.BuildFile(*file_proto), nullptr); +} + +TEST_P(ExtensionDeclarationsTest, MismatchNonMessageType) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: "int32" + } + } + } + } + extension { name: "bar" number: 10 type: TYPE_FIXED64 extendee: "Foo" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: ext.test.bar: EXTENDEE: \"ext.test.Foo\" extension " + "field 10 is expected to be type \"int32\", not \"fixed64\".\n"); +} + +TEST_P(ExtensionDeclarationsTest, MismatchCardinalityExpectingRepeated) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: "fixed64" + repeated: true + } + } + } + } + extension { name: "bar" number: 10 type: TYPE_FIXED64 extendee: "Foo" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: ext.test.bar: EXTENDEE: \"ext.test.Foo\" extension " + "field 10 is expected to be repeated.\n"); +} + +TEST_P(ExtensionDeclarationsTest, MismatchCardinalityExpectingOptional) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: "fixed64" + } + } + } + } + extension { + name: "bar" + number: 10 + type: TYPE_FIXED64 + extendee: "Foo" + label: LABEL_REPEATED + } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: ext.test.bar: EXTENDEE: \"ext.test.Foo\" extension " + "field 10 is expected to be optional.\n"); +} + +TEST_P(ExtensionDeclarationsTest, TypeDoesNotLookLikeIdentifier) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + message_type { + name: "Foo" + extension_range { + start: 10 + end: 11 + options: { + declaration: { + number: 10 + full_name: ".ext.test.bar" + type: ".b#az" + } + } + } + } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + MockErrorCollector error_collector; + EXPECT_EQ(pool.BuildFileCollectingErrors(*file_proto, &error_collector), + nullptr); + EXPECT_EQ(error_collector.text_, + "foo.proto: Foo: NAME: \".b#az\" contains invalid identifiers.\n"); +} + +TEST_P(ExtensionDeclarationsTest, MultipleDeclarationsInARangeCompile) { + absl::StatusOr file_proto = ParameterizeFileProto( + R"pb( + name: "foo.proto" + package: "ext.test" + message_type { + name: "Foo" + extension_range { + start: 4 + end: 99999 + options: { + declaration: { + number: 10 + full_name: ".ext.test.foo" + type: ".ext.test.Bar" + } + declaration: { + number: 99998 + full_name: ".ext.test.bar" + type: ".ext.test.Bar" + } + declaration: { + number: 12345 + full_name: ".ext.test.baz" + type: ".ext.test.Bar" + } + } + } + } + message_type { name: "Bar" } + extension { extendee: "Foo" name: "foo" number: 10 type_name: "Bar" } + extension { extendee: "Foo" name: "bar" number: 99998 type_name: "Bar" } + extension { extendee: "Foo" name: "baz" number: 12345 type_name: "Bar" } + )pb", + GetParam()); + ASSERT_OK(file_proto); + + DescriptorPool pool; + pool.EnforceExtensionDeclarations(true); + EXPECT_NE(pool.BuildFile(*file_proto), nullptr); +} + +INSTANTIATE_TEST_SUITE_P( + ExtensionDeclarationTests, ExtensionDeclarationsTest, + testing::ValuesIn({ + {"Declaration"}, + }), + [](const testing::TestParamInfo& + info) { return info.param.test_name; }); + + TEST_F(ValidationErrorTest, PackageTooLong) { BuildFileWithErrors( "name: \"foo.proto\" "