diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 23eb8b6dbd..ce5e0cc166 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1110,19 +1110,6 @@ void RestoreFeaturesToOptions(const FeatureSet* features, ProtoT* proto) { } } -template -bool HasFeatures(const OptionsT& options) { - if (options.has_features()) return true; - - for (const auto& opt : options.uninterpreted_option()) { - if (opt.name_size() > 0 && opt.name(0).name_part() == "features" && - !opt.name(0).is_extension()) { - return true; - } - } - return false; -} - template absl::string_view GetFullName(const DescriptorT& desc) { return desc.full_name(); @@ -8774,6 +8761,19 @@ bool DescriptorBuilder::OptionInterpreter::InterpretSingleOption( // accumulate field numbers to form path to interpreted option dest_path.push_back(field->number()); + // Special handling to prevent feature use in the same file as the + // definition. + // TODO Add proper support for cases where this can work. + if (field->file() == builder_->file_ && + uninterpreted_option_->name(0).name_part() == "features" && + !uninterpreted_option_->name(0).is_extension()) { + return AddNameError([&] { + return absl::StrCat( + "Feature \"", debug_msg_name, + "\" can't be used in the same file it's defined in."); + }); + } + if (i < uninterpreted_option_->name_size() - 1) { if (field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { return AddNameError([&] { diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 9a83c682f3..97065c55cb 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -4059,8 +4059,8 @@ class ValidationErrorTest : public testing::Test { return ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto)); } - const FileDescriptor* ParseAndBuildFile(absl::string_view file_name, - absl::string_view file_text) { + FileDescriptorProto ParseFile(absl::string_view file_name, + absl::string_view file_text) { io::ArrayInputStream input_stream(file_text.data(), file_text.size()); SimpleErrorCollector error_collector; io::Tokenizer tokenizer(&input_stream, &error_collector); @@ -4072,7 +4072,12 @@ class ValidationErrorTest : public testing::Test { << file_text; ABSL_CHECK_EQ("", error_collector.last_error()); proto.set_name(file_name); - return pool_.BuildFile(proto); + return proto; + } + + const FileDescriptor* ParseAndBuildFile(absl::string_view file_name, + absl::string_view file_text) { + return pool_.BuildFile(ParseFile(file_name, file_text)); } @@ -4096,6 +4101,17 @@ class ValidationErrorTest : public testing::Test { BuildFileWithErrors(file_proto, expected_errors); } + // Parse a proto file and build it. Expect errors to be produced which match + // the given error text. + void ParseAndBuildFileWithErrors(absl::string_view file_name, + absl::string_view file_text, + absl::string_view expected_errors) { + MockErrorCollector error_collector; + EXPECT_TRUE(pool_.BuildFileCollectingErrors(ParseFile(file_name, file_text), + &error_collector) == nullptr); + EXPECT_EQ(expected_errors, error_collector.text_); + } + // Parse file_text as a FileDescriptorProto in text format and add it // to the DescriptorPool. Expect errors to be produced which match the // given warning text. @@ -10283,6 +10299,44 @@ TEST_F(FeaturesTest, InvalidOpenEnumNonZeroFirstValue) { "enums.\n"); } +TEST_F(FeaturesTest, InvalidUseFeaturesInSameFile) { + BuildDescriptorMessagesInTestPool(); + ParseAndBuildFileWithErrors("foo.proto", R"schema( + edition = "2023"; + + package test; + import "google/protobuf/descriptor.proto"; + + message Foo { + string bar = 1 [ + features.(test.custom).foo = "xyz", + features.(test.another) = {foo: -321} + ]; + } + + message Custom { + string foo = 1 [features = { [test.custom]: {foo: "abc"} }]; + } + message Another { + Enum foo = 1; + } + + enum Enum { + option features.enum_type = CLOSED; + ZERO = 0; + ONE = 1; + } + + extend google.protobuf.FeatureSet { + Custom custom = 1002 [features.message_encoding=DELIMITED]; + Another another = 1001; + } + )schema", + "foo.proto: test.Foo.bar: OPTION_NAME: Feature " + "\"features.(test.custom)\" can't be used in the " + "same file it's defined in.\n"); +} + TEST_F(FeaturesTest, ClosedEnumNonZeroFirstValue) { BuildDescriptorMessagesInTestPool(); const FileDescriptor* file = BuildFile(