Skip to content

Internal change #24198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+272 −22
Closed

Internal change #24198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/google/protobuf/descriptor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand All @@ -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) {}
Expand All @@ -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) {}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 &&
Expand Down
23 changes: 12 additions & 11 deletions src/google/protobuf/descriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2676,17 +2676,6 @@ class PROTOBUF_EXPORT DescriptorPool {
class Tables;
std::unique_ptr<Tables> 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<std::string, bool> direct_input_files_;
Expand All @@ -2695,6 +2684,18 @@ class PROTOBUF_EXPORT DescriptorPool {
// just the global and C++ features, but can be overridden for other runtimes.
std::unique_ptr<FeatureSetDefaults> 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;
Expand Down
50 changes: 50 additions & 0 deletions src/google/protobuf/one_platform_descriptor_pool_quirks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
195 changes: 195 additions & 0 deletions src/google/protobuf/one_platform_descriptor_pool_quirks_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 <optional>
#include <string_view>

#include "google/protobuf/descriptor.pb.h"
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#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<google::protobuf::Edition> 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 <typename T>
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 <typename T>
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
Loading