Add an allowlist for implementation reserved fields for the C++ implementation. This also includes `protoc`.

The allowlist is currently empty.

It uses a simple text format to avoid reentrant dependencies into protobuf
parsing, and allow sharing between languages.

PiperOrigin-RevId: 493075113
pull/11155/head
Protobuf Team Bot 2 years ago committed by Copybara-Service
parent bc441c9892
commit d01a9120ab
  1. 59
      src/google/protobuf/descriptor.cc
  2. 9
      src/google/protobuf/descriptor.h
  3. 43
      src/google/protobuf/descriptor_unittest.cc

@ -5487,12 +5487,6 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto,
} }
} }
namespace {
bool IsAllowedReservedField(const FieldDescriptorProto& field) {
return false;
}
} // namespace
void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
Descriptor* parent, Descriptor* parent,
FieldDescriptor* result, FieldDescriptor* result,
@ -5719,8 +5713,7 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto,
absl::Substitute("Field numbers cannot be greater than $0.", absl::Substitute("Field numbers cannot be greater than $0.",
FieldDescriptor::kMaxNumber)); FieldDescriptor::kMaxNumber));
} else if (result->number() >= FieldDescriptor::kFirstReservedNumber && } else if (result->number() >= FieldDescriptor::kFirstReservedNumber &&
result->number() <= FieldDescriptor::kLastReservedNumber && result->number() <= FieldDescriptor::kLastReservedNumber) {
!IsAllowedReservedField(proto)) {
message_hints_[parent].RequestHintOnFieldNumbers( message_hints_[parent].RequestHintOnFieldNumbers(
proto, DescriptorPool::ErrorCollector::NUMBER); proto, DescriptorPool::ErrorCollector::NUMBER);
AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::NUMBER, AddError(result->full_name(), proto, DescriptorPool::ErrorCollector::NUMBER,
@ -8303,56 +8296,6 @@ void LazyDescriptor::Once(const ServiceDescriptor* service) {
} }
} }
// The format of this file is a sequence of lines like:
// <label>,<type>,<name>,<number>
// Whitespace is ignored.
// Lines that start with `#` are comments and ignored.
std::vector<AllowedReservedField> ParseAllowedReservedField(
absl::string_view input) {
std::vector<AllowedReservedField> out;
for (absl::string_view line :
absl::StrSplit(input, absl::ByAnyChar("\r\n"), absl::SkipEmpty())) {
line = absl::StripAsciiWhitespace(line);
// Skip comments
if (absl::StartsWith(line, "#")) continue;
std::vector<absl::string_view> parts = absl::StrSplit(line, ',');
for (auto& p : parts) p = absl::StripAsciiWhitespace(p);
if (parts.size() != 4) {
GOOGLE_LOG(ERROR) << "Invalid line in ParseAllowedReservedField <" << line
<< ">";
continue;
}
AllowedReservedField field;
if (parts[0] == "optional") {
field.label = FieldDescriptor::LABEL_OPTIONAL;
} else if (parts[0] == "repeated") {
field.label = FieldDescriptor::LABEL_REPEATED;
} else {
GOOGLE_LOG(ERROR) << "Invalid label in ParseAllowedReservedField <" << parts[0]
<< ">";
continue;
}
field.type_name = std::string(parts[1]);
field.name = std::string(parts[2]);
if (!absl::SimpleAtoi(parts[3], &field.number) ||
field.number < FieldDescriptor::kFirstReservedNumber ||
field.number > FieldDescriptor::kLastReservedNumber) {
GOOGLE_LOG(ERROR) << "Invalid number in ParseAllowedReservedField <" << parts[3]
<< ">";
continue;
}
// Only insert after input has been verified.
out.push_back(std::move(field));
}
return out;
}
namespace cpp { namespace cpp {
bool HasPreservingUnknownEnumSemantics(const FieldDescriptor* field) { bool HasPreservingUnknownEnumSemantics(const FieldDescriptor* field) {
return field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3; return field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3;

@ -2493,15 +2493,6 @@ inline FileDescriptor::Syntax FileDescriptor::syntax() const {
namespace internal { namespace internal {
struct AllowedReservedField {
FieldDescriptor::Label label;
int number;
std::string type_name;
std::string name;
};
PROTOBUF_EXPORT std::vector<AllowedReservedField> ParseAllowedReservedField(
absl::string_view input);
// FieldRange(desc) provides an iterable range for the fields of a // FieldRange(desc) provides an iterable range for the fields of a
// descriptor type, appropriate for range-for loops. // descriptor type, appropriate for range-for loops.

@ -67,7 +67,6 @@
#include "google/protobuf/port_def.inc" #include "google/protobuf/port_def.inc"
using ::testing::AnyOf; using ::testing::AnyOf;
using ::testing::ElementsAre;
namespace google { namespace google {
namespace protobuf { namespace protobuf {
@ -864,6 +863,7 @@ TEST_F(DescriptorTest, FieldNamesDedup) {
return names; return names;
}; };
using testing::ElementsAre;
// field_name1 // field_name1
EXPECT_THAT(collect_unique_names(message4_->field(0)), EXPECT_THAT(collect_unique_names(message4_->field(0)),
ElementsAre("fieldName1", "field_name1")); ElementsAre("fieldName1", "field_name1"));
@ -4602,7 +4602,6 @@ TEST_F(ValidationErrorTest, ReservedFieldNumber) {
"foo.proto: Foo: NUMBER: Suggested field numbers for Foo: 1, 2\n"); "foo.proto: Foo: NUMBER: Suggested field numbers for Foo: 1, 2\n");
} }
TEST_F(ValidationErrorTest, ExtensionMissingExtendee) { TEST_F(ValidationErrorTest, ExtensionMissingExtendee) {
BuildFileWithErrors( BuildFileWithErrors(
"name: \"foo.proto\" " "name: \"foo.proto\" "
@ -8490,46 +8489,6 @@ TEST_F(LazilyBuildDependenciesTest, Dependency) {
EXPECT_FALSE(pool_.InternalIsFileLoaded("baz.proto")); EXPECT_FALSE(pool_.InternalIsFileLoaded("baz.proto"));
} }
TEST(AllowedReservedFieldTest, ParsesCorrectInput) {
auto res = internal::ParseAllowedReservedField(R"(
optional, foo.bar , foo_bar, 19000
repeated, int32, the_int, 19123
# comment
optional, optional, optional, 19253
)");
ASSERT_EQ(res.size(), 3);
EXPECT_EQ(res[0].label, FieldDescriptor::LABEL_OPTIONAL);
EXPECT_EQ(res[0].number, 19000);
EXPECT_EQ(res[0].type_name, "foo.bar");
EXPECT_EQ(res[0].name, "foo_bar");
EXPECT_EQ(res[1].label, FieldDescriptor::LABEL_REPEATED);
EXPECT_EQ(res[1].number, 19123);
EXPECT_EQ(res[1].type_name, "int32");
EXPECT_EQ(res[1].name, "the_int");
EXPECT_EQ(res[2].label, FieldDescriptor::LABEL_OPTIONAL);
EXPECT_EQ(res[2].number, 19253);
EXPECT_EQ(res[2].type_name, "optional");
EXPECT_EQ(res[2].name, "optional");
}
TEST(AllowedReservedFieldTest, IncorrectInputDoesNotCrash) {
// Bad label.
EXPECT_THAT(internal::ParseAllowedReservedField("required, a, b, 19000"),
ElementsAre());
// Bad number of fields.
EXPECT_THAT(internal::ParseAllowedReservedField("optional, a, b"),
ElementsAre());
// Bad field number.
EXPECT_THAT(internal::ParseAllowedReservedField("required, a, b, xxx"),
ElementsAre());
// Out of bounds field number.
EXPECT_THAT(internal::ParseAllowedReservedField("required, a, b, 1"),
ElementsAre());
}
// =================================================================== // ===================================================================

Loading…
Cancel
Save