Skip to content

Commit

Permalink
Check ctype() in DescriptorBuilder for edition 2023 and beyond.
Browse files Browse the repository at this point in the history
It seems possible that old data is stored with serialized descriptors. If we
reject old descriptors due to invalid ctype, the change effectively becomes
breaking changes. We should apply this stricter check for edition 2023 or beyond.

PiperOrigin-RevId: 602516135
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Jan 29, 2024
1 parent fd82df7 commit 307aeac
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7737,7 +7737,8 @@ void DescriptorBuilder::ValidateOptions(const FieldDescriptor* field,

// The following check is temporarily OSS only till we fix all affected
// google3 TAP tests.
if (field->options().has_ctype()) {
if (field->file()->edition() >= Edition::EDITION_2023 &&
field->options().has_ctype()) {
if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) {
AddError(
field->full_name(), proto, DescriptorPool::ErrorCollector::TYPE,
Expand Down
3 changes: 3 additions & 0 deletions src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2957,6 +2957,9 @@ TEST_F(MiscTest, InvalidFieldOptions) {
FileDescriptorProto file_proto;
file_proto.set_name("foo.proto");

file_proto.set_syntax("editions");
file_proto.set_edition(Edition::EDITION_2023);

DescriptorProto* message_proto = AddMessage(&file_proto, "TestMessage");
AddField(message_proto, "foo", 1, FieldDescriptorProto::LABEL_OPTIONAL,
FieldDescriptorProto::TYPE_INT32);
Expand Down

0 comments on commit 307aeac

Please sign in to comment.