diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 193bda01695d6..85beffdfb077b 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -2376,11 +2376,11 @@ DescriptorPool::DescriptorPool() default_error_collector_(nullptr), underlay_(nullptr), tables_(new Tables), + enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), enforce_dependencies_(true), lazily_build_dependencies_(false), allow_unknown_(false), enforce_weak_(false), - enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), disallow_enforce_utf8_(false), deprecated_legacy_json_field_conflicts_(false), enforce_naming_style_(false) {} @@ -2392,11 +2392,11 @@ DescriptorPool::DescriptorPool(DescriptorDatabase* fallback_database, default_error_collector_(error_collector), underlay_(nullptr), tables_(new Tables), + enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), enforce_dependencies_(true), lazily_build_dependencies_(false), allow_unknown_(false), enforce_weak_(false), - enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), disallow_enforce_utf8_(false), deprecated_legacy_json_field_conflicts_(false), enforce_naming_style_(false) {} @@ -2407,11 +2407,11 @@ DescriptorPool::DescriptorPool(const DescriptorPool* underlay) default_error_collector_(nullptr), underlay_(underlay), tables_(new Tables), + enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), enforce_dependencies_(true), lazily_build_dependencies_(false), allow_unknown_(false), enforce_weak_(false), - enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement), disallow_enforce_utf8_(false), deprecated_legacy_json_field_conflicts_(false), enforce_naming_style_(false) {} @@ -7608,12 +7608,14 @@ void DescriptorBuilder::BuildEnumValue(const EnumValueDescriptorProto& proto, AllocateOptions(proto, result, EnumValueDescriptorProto::kOptionsFieldNumber, "google.protobuf.EnumValueOptions", alloc); - // Again, enum values are weird because we makes them appear as siblings - // of the enum type instead of children of it. So, we use - // parent->containing_type() as the value's parent. - bool added_to_outer_scope = - AddSymbol(result->full_name(), parent->containing_type(), result->name(), - proto, Symbol::EnumValue(result, 0)); + bool added_to_outer_scope; + + // Again, enum values are weird because we makes them appear as siblings + // of the enum type instead of children of it. So, we use + // parent->containing_type() as the value's parent. + added_to_outer_scope = + AddSymbol(result->full_name(), parent->containing_type(), + result->name(), proto, Symbol::EnumValue(result, 0)); // However, we also want to be able to search for values within a single // enum type, so we add it as a child of the enum type itself, too. @@ -8086,8 +8088,10 @@ void DescriptorBuilder::CrossLinkField(FieldDescriptor* field, // because that locks the pool's mutex, which we have already locked // at this point. const EnumValueDescriptor* default_value = - LookupSymbolNoPlaceholder(proto.default_value(), - field->enum_type()->full_name()) + field->enum_type() + ->file() + ->tables_ + ->FindNestedSymbol(field->enum_type(), proto.default_value()) .enum_value_descriptor(); if (default_value != nullptr && diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 2adb558a97e5e..dff1a97536df0 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -2676,17 +2676,6 @@ class PROTOBUF_EXPORT DescriptorPool { class Tables; std::unique_ptr tables_; - bool enforce_dependencies_; - bool lazily_build_dependencies_; - bool allow_unknown_; - bool enforce_weak_; - ExtDeclEnforcementLevel enforce_extension_declarations_; - bool disallow_enforce_utf8_; - bool deprecated_legacy_json_field_conflicts_; - bool enforce_naming_style_; - bool enforce_symbol_visibility_ = false; - mutable bool build_started_ = false; - // Set of files to track for additional validation. The bool value when true // means unused imports are treated as errors (and as warnings when false). absl::flat_hash_map direct_input_files_; @@ -2695,6 +2684,18 @@ class PROTOBUF_EXPORT DescriptorPool { // just the global and C++ features, but can be overridden for other runtimes. std::unique_ptr feature_set_defaults_spec_; + ExtDeclEnforcementLevel enforce_extension_declarations_; + + bool enforce_dependencies_ : 1; + bool lazily_build_dependencies_ : 1; + bool allow_unknown_ : 1; + bool enforce_weak_ : 1; + bool disallow_enforce_utf8_ : 1; + bool deprecated_legacy_json_field_conflicts_ : 1; + bool enforce_naming_style_ : 1; + bool enforce_symbol_visibility_ : 1 = false; + mutable bool build_started_ : 1 = false; + // Returns true if the field extends an option message of descriptor.proto. bool IsReadyForCheckingDescriptorExtDecl( absl::string_view message_name) const; diff --git a/src/google/protobuf/one_platform_descriptor_pool_quirks.h b/src/google/protobuf/one_platform_descriptor_pool_quirks.h new file mode 100644 index 0000000000000..f46408f1c3490 --- /dev/null +++ b/src/google/protobuf/one_platform_descriptor_pool_quirks.h @@ -0,0 +1,50 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2025 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#ifndef GOOGLE_PROTOBUF_ONE_PLATFORM_DESCRIPTOR_POOL_QUIRKS_H__ +#define GOOGLE_PROTOBUF_ONE_PLATFORM_DESCRIPTOR_POOL_QUIRKS_H__ + +#include "absl/status/status.h" +#include "google/protobuf/descriptor.h" + +// Must be included last. +#include "google/protobuf/port_def.inc" + +namespace google { +namespace protobuf { + +class DescriptorPool; + +class OnePlatformDescriptorPoolQuirks final { + public: + // Enables OnePlatform quirks for the provided descriptor pool. The usage of + // this class and method requires approval from CEL and protobuf leads. It is + // intended to be a short-term workaround. + // + // 1. Treats all enums as "scoped", that is enum values are only required to + // be unique amongst the enum itself and not among the siblings of the enum as + // required by protobuf. A side effect is that + // `{DescriptorPool,FileDescriptor,Descriptor}::FindEnumValueByName` will + // always return `nullptr`. Instead you must exclusively use + // `EnumDescriptor::FindValueByName`. + static absl::Status Enable(google::protobuf::DescriptorPool& pool) { + if (pool.build_started_) { + return absl::FailedPreconditionError( + "OnePlatformDescriptorPoolQuirks::Enable must be called before " + "building any descriptors"); + } + pool.one_platform_quirks_ = true; + return absl::OkStatus(); + } +}; + +} // namespace protobuf +} // namespace google + +#include "google/protobuf/port_undef.inc" + +#endif diff --git a/src/google/protobuf/one_platform_descriptor_pool_quirks_test.cc b/src/google/protobuf/one_platform_descriptor_pool_quirks_test.cc new file mode 100644 index 0000000000000..7ed555b2f36cd --- /dev/null +++ b/src/google/protobuf/one_platform_descriptor_pool_quirks_test.cc @@ -0,0 +1,195 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2025 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +#include "google/protobuf/one_platform_descriptor_pool_quirks.h" + +#include +#include + +#include "google/protobuf/descriptor.pb.h" +#include +#include +#include "absl/status/status_matchers.h" +#include "absl/strings/str_cat.h" +#include "google/protobuf/descriptor.h" + +namespace google { +namespace protobuf { +namespace { + +using ::absl_testing::IsOk; +using ::testing::IsNull; +using ::testing::NotNull; + +inline constexpr std::string_view kScopedEnumsFileDescriptorName = + "one_platform_descriptor_pool_quirks.proto"; +inline constexpr std::string_view kScopedEnumsFileDescriptorPackage = + "one_platform_descriptor_pool_quirks"; + +google::protobuf::FileDescriptorProto GetScopedEnumsFileDescriptor( + std::string_view syntax, std::optional edition) { + google::protobuf::FileDescriptorProto file_descriptor_proto; + file_descriptor_proto.set_name(kScopedEnumsFileDescriptorName); + file_descriptor_proto.set_package(kScopedEnumsFileDescriptorPackage); + file_descriptor_proto.set_syntax(syntax); + if (edition.has_value()) { + file_descriptor_proto.set_edition(*edition); + } + auto* enum_descriptor1 = file_descriptor_proto.add_enum_type(); + enum_descriptor1->set_name("Enum1"); + { + auto* enum_value_descriptor1 = enum_descriptor1->add_value(); + enum_value_descriptor1->set_name("FOO"); + enum_value_descriptor1->set_number(0); + auto* enum_value_descriptor2 = enum_descriptor1->add_value(); + enum_value_descriptor2->set_name("UNIQUE1_FOO"); + enum_value_descriptor2->set_number(1); + } + auto* enum_descriptor2 = file_descriptor_proto.add_enum_type(); + enum_descriptor2->set_name("Enum2"); + { + auto* enum_value_descriptor1 = enum_descriptor2->add_value(); + enum_value_descriptor1->set_name("UNIQUE2_FOO"); + enum_value_descriptor1->set_number(0); + auto* enum_value_descriptor2 = enum_descriptor2->add_value(); + enum_value_descriptor2->set_name("FOO"); + enum_value_descriptor2->set_number(1); + } + auto* message_descriptor = file_descriptor_proto.add_message_type(); + message_descriptor->set_name("Message"); + *message_descriptor->add_enum_type() = *enum_descriptor1; + *message_descriptor->add_enum_type() = *enum_descriptor2; + return file_descriptor_proto; +} + +template +void CheckFindEnumValueByNameAbsent(const T* parent) { + EXPECT_THAT(parent->FindEnumValueByName("FOO"), IsNull()); + EXPECT_THAT(parent->FindEnumValueByName("UNIQUE1_FOO"), IsNull()); + EXPECT_THAT(parent->FindEnumValueByName("UNIQUE2_FOO"), IsNull()); +} + +void CheckFindEnumValueByNameAbsent(const google::protobuf::DescriptorPool* parent) { + EXPECT_THAT(parent->FindEnumValueByName( + absl::StrCat(kScopedEnumsFileDescriptorPackage, ".FOO")), + IsNull()); + EXPECT_THAT(parent->FindEnumValueByName(absl::StrCat( + kScopedEnumsFileDescriptorPackage, ".UNIQUE1_FOO")), + IsNull()); + EXPECT_THAT(parent->FindEnumValueByName(absl::StrCat( + kScopedEnumsFileDescriptorPackage, ".UNIQUE2_FOO")), + IsNull()); + EXPECT_THAT(parent->FindEnumValueByName(absl::StrCat( + kScopedEnumsFileDescriptorPackage, ".Message.FOO")), + IsNull()); + EXPECT_THAT(parent->FindEnumValueByName(absl::StrCat( + kScopedEnumsFileDescriptorPackage, ".Message.UNIQUE1_FOO")), + IsNull()); + EXPECT_THAT(parent->FindEnumValueByName(absl::StrCat( + kScopedEnumsFileDescriptorPackage, ".Message.UNIQUE2_FOO")), + IsNull()); +} + +template +void CheckFindEnumValueByNamePresent(const T* parent) { + const auto* enum1_descriptor = parent->FindEnumTypeByName("Enum1"); + ASSERT_THAT(enum1_descriptor, NotNull()); + const auto* enum2_descriptor = parent->FindEnumTypeByName("Enum2"); + ASSERT_THAT(enum2_descriptor, NotNull()); + + const google::protobuf::EnumValueDescriptor* enum_value_descriptor; + + enum_value_descriptor = enum1_descriptor->FindValueByName("FOO"); + ASSERT_THAT(enum_value_descriptor, NotNull()); + EXPECT_EQ(enum_value_descriptor->name(), "FOO"); + EXPECT_EQ(enum_value_descriptor->number(), 0); + + enum_value_descriptor = enum1_descriptor->FindValueByName("UNIQUE1_FOO"); + ASSERT_THAT(enum_value_descriptor, NotNull()); + EXPECT_EQ(enum_value_descriptor->name(), "UNIQUE1_FOO"); + EXPECT_EQ(enum_value_descriptor->number(), 1); + + enum_value_descriptor = enum2_descriptor->FindValueByName("UNIQUE2_FOO"); + ASSERT_THAT(enum_value_descriptor, NotNull()); + EXPECT_EQ(enum_value_descriptor->name(), "UNIQUE2_FOO"); + EXPECT_EQ(enum_value_descriptor->number(), 0); + + enum_value_descriptor = enum2_descriptor->FindValueByName("FOO"); + ASSERT_THAT(enum_value_descriptor, NotNull()); + EXPECT_EQ(enum_value_descriptor->name(), "FOO"); + EXPECT_EQ(enum_value_descriptor->number(), 1); +} + +TEST(OnePlatformDescriptorPoolQuirks, ScopedEnumsProto2) { + google::protobuf::DescriptorPool descriptor_pool; + ASSERT_THAT(OnePlatformDescriptorPoolQuirks::Enable(descriptor_pool), IsOk()); + ASSERT_THAT(descriptor_pool.BuildFile( + GetScopedEnumsFileDescriptor("proto2", std::nullopt)), + NotNull()); + const auto* file_descriptor = + descriptor_pool.FindFileByName(kScopedEnumsFileDescriptorName); + ASSERT_THAT(file_descriptor, NotNull()); + const auto* message_descriptor = descriptor_pool.FindMessageTypeByName( + absl::StrCat(kScopedEnumsFileDescriptorPackage, ".Message")); + ASSERT_THAT(message_descriptor, NotNull()); + + // OnePlatformDescriptorPoolQuirks disables lookup for enum values by name + // everywhere except for EnumDescriptor::FindValueByName. + CheckFindEnumValueByNameAbsent(&descriptor_pool); + CheckFindEnumValueByNameAbsent(file_descriptor); + CheckFindEnumValueByNameAbsent(message_descriptor); + CheckFindEnumValueByNamePresent(file_descriptor); + CheckFindEnumValueByNamePresent(message_descriptor); +} + +TEST(OnePlatformDescriptorPoolQuirks, ScopedEnumsProto3) { + google::protobuf::DescriptorPool descriptor_pool; + ASSERT_THAT(OnePlatformDescriptorPoolQuirks::Enable(descriptor_pool), IsOk()); + ASSERT_THAT(descriptor_pool.BuildFile( + GetScopedEnumsFileDescriptor("proto3", std::nullopt)), + NotNull()); + const auto* file_descriptor = + descriptor_pool.FindFileByName(kScopedEnumsFileDescriptorName); + ASSERT_THAT(file_descriptor, NotNull()); + const auto* message_descriptor = descriptor_pool.FindMessageTypeByName( + absl::StrCat(kScopedEnumsFileDescriptorPackage, ".Message")); + ASSERT_THAT(message_descriptor, NotNull()); + + // OnePlatformDescriptorPoolQuirks disables lookup for enum values by name + // everywhere except for EnumDescriptor::FindValueByName. + CheckFindEnumValueByNameAbsent(&descriptor_pool); + CheckFindEnumValueByNameAbsent(file_descriptor); + CheckFindEnumValueByNameAbsent(message_descriptor); + CheckFindEnumValueByNamePresent(file_descriptor); + CheckFindEnumValueByNamePresent(message_descriptor); +} + +TEST(OnePlatformDescriptorPoolQuirks, ScopedEnumsProtoEditions) { + google::protobuf::DescriptorPool descriptor_pool; + ASSERT_THAT(OnePlatformDescriptorPoolQuirks::Enable(descriptor_pool), IsOk()); + ASSERT_THAT(descriptor_pool.BuildFile(GetScopedEnumsFileDescriptor( + "editions", google::protobuf::EDITION_2023)), + NotNull()); + const auto* file_descriptor = + descriptor_pool.FindFileByName(kScopedEnumsFileDescriptorName); + ASSERT_THAT(file_descriptor, NotNull()); + const auto* message_descriptor = descriptor_pool.FindMessageTypeByName( + absl::StrCat(kScopedEnumsFileDescriptorPackage, ".Message")); + ASSERT_THAT(message_descriptor, NotNull()); + + // OnePlatformDescriptorPoolQuirks disables lookup for enum values by name + // everywhere except for EnumDescriptor::FindValueByName. + CheckFindEnumValueByNameAbsent(&descriptor_pool); + CheckFindEnumValueByNameAbsent(file_descriptor); + CheckFindEnumValueByNameAbsent(message_descriptor); + CheckFindEnumValueByNamePresent(file_descriptor); + CheckFindEnumValueByNamePresent(message_descriptor); +} + +} // namespace +} // namespace protobuf +} // namespace google