From 3774ee0acbb5383f00284abf3541d7768d285452 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 25 Jan 2023 08:29:43 -0800 Subject: [PATCH] Stop assuming lazy message sets are initialized. PiperOrigin-RevId: 504569894 --- src/google/protobuf/compiler/cpp/message.cc | 2 +- src/google/protobuf/descriptor.pb.cc | 18 ++++---- src/google/protobuf/extension_set.cc | 46 ++++++++++++--------- src/google/protobuf/extension_set.h | 8 ++-- src/google/protobuf/reflection_ops.cc | 17 ++++++-- src/google/protobuf/unittest_mset.proto | 1 + 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index a65491c71074..4be5fe9a7394 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -4202,7 +4202,7 @@ void MessageGenerator::GenerateIsInitialized(io::Printer* p) { if (descriptor_->extension_range_count() > 0) { format( - "if (!$extensions$.IsInitialized()) {\n" + "if (!$extensions$.IsInitialized(internal_default_instance())) {\n" " return false;\n" "}\n\n"); } diff --git a/src/google/protobuf/descriptor.pb.cc b/src/google/protobuf/descriptor.pb.cc index 7cfe0468ab17..a3c18fecadd0 100644 --- a/src/google/protobuf/descriptor.pb.cc +++ b/src/google/protobuf/descriptor.pb.cc @@ -3863,7 +3863,7 @@ void ExtensionRangeOptions::CopyFrom(const ExtensionRangeOptions& from) { } bool ExtensionRangeOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -7669,7 +7669,7 @@ void FileOptions::CopyFrom(const FileOptions& from) { } bool FileOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -8109,7 +8109,7 @@ void MessageOptions::CopyFrom(const MessageOptions& from) { } bool MessageOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -8689,7 +8689,7 @@ void FieldOptions::CopyFrom(const FieldOptions& from) { } bool FieldOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -8902,7 +8902,7 @@ void OneofOptions::CopyFrom(const OneofOptions& from) { } bool OneofOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -9226,7 +9226,7 @@ void EnumOptions::CopyFrom(const EnumOptions& from) { } bool EnumOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -9483,7 +9483,7 @@ void EnumValueOptions::CopyFrom(const EnumValueOptions& from) { } bool EnumValueOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -9736,7 +9736,7 @@ void ServiceOptions::CopyFrom(const ServiceOptions& from) { } bool ServiceOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } @@ -10037,7 +10037,7 @@ void MethodOptions::CopyFrom(const MethodOptions& from) { } bool MethodOptions::IsInitialized() const { - if (!_impl_._extensions_.IsInitialized()) { + if (!_impl_._extensions_.IsInitialized(internal_default_instance())) { return false; } diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc index 7b6edf5d6bc4..9885e15d0c39 100644 --- a/src/google/protobuf/extension_set.cc +++ b/src/google/protobuf/extension_set.cc @@ -1213,17 +1213,21 @@ void ExtensionSet::UnsafeShallowSwapExtension(ExtensionSet* other, int number) { } } -bool ExtensionSet::IsInitialized() const { +bool ExtensionSet::IsInitialized(const MessageLite* extendee) const { // Extensions are never required. However, we need to check that all // embedded messages are initialized. if (PROTOBUF_PREDICT_FALSE(is_large())) { for (const auto& kv : *map_.large) { - if (!kv.second.IsInitialized()) return false; + if (!kv.second.IsInitialized(this, extendee, kv.first, arena_)) { + return false; + } } return true; } for (const KeyValue* it = flat_begin(); it != flat_end(); ++it) { - if (!it->second.IsInitialized()) return false; + if (!it->second.IsInitialized(this, extendee, it->first, arena_)) { + return false; + } } return true; } @@ -1563,25 +1567,29 @@ void ExtensionSet::Extension::Free() { // Defined in extension_set_heavy.cc. // int ExtensionSet::Extension::SpaceUsedExcludingSelf() const -bool ExtensionSet::Extension::IsInitialized() const { - if (cpp_type(type) == WireFormatLite::CPPTYPE_MESSAGE) { - if (is_repeated) { - for (int i = 0; i < repeated_message_value->size(); i++) { - if (!repeated_message_value->Get(i).IsInitialized()) { - return false; - } - } - } else { - if (!is_cleared) { - if (is_lazy) { - if (!lazymessage_value->IsInitialized()) return false; - } else { - if (!message_value->IsInitialized()) return false; - } +bool ExtensionSet::Extension::IsInitialized(const ExtensionSet* ext_set, + const MessageLite* extendee, + int number, Arena* arena) const { + if (cpp_type(type) != WireFormatLite::CPPTYPE_MESSAGE) return true; + + if (is_repeated) { + for (int i = 0; i < repeated_message_value->size(); i++) { + if (!repeated_message_value->Get(i).IsInitialized()) { + return false; } } + return true; } - return true; + + if (is_cleared) return true; + + if (!is_lazy) return message_value->IsInitialized(); + + const MessageLite* prototype = + ext_set->GetPrototypeForLazyMessage(extendee, number); + ABSL_DCHECK_NE(prototype, nullptr) + << "extendee: " << extendee->GetTypeName() << "; number: " << number; + return lazymessage_value->IsInitialized(prototype, arena); } // Dummy key method to avoid weak vtable. diff --git a/src/google/protobuf/extension_set.h b/src/google/protobuf/extension_set.h index ee381e54fd7c..cb9670f0e2c2 100644 --- a/src/google/protobuf/extension_set.h +++ b/src/google/protobuf/extension_set.h @@ -387,7 +387,7 @@ class PROTOBUF_EXPORT ExtensionSet { void SwapExtension(const MessageLite* extendee, ExtensionSet* other, int number); void UnsafeShallowSwapExtension(ExtensionSet* other, int number); - bool IsInitialized() const; + bool IsInitialized(const MessageLite* extendee) const; // Lite parser const char* ParseField(uint64_t tag, const char* ptr, @@ -558,7 +558,8 @@ class PROTOBUF_EXPORT ExtensionSet { virtual MessageLite* UnsafeArenaReleaseMessage(const MessageLite& prototype, Arena* arena) = 0; - virtual bool IsInitialized() const = 0; + virtual bool IsInitialized(const MessageLite* prototype, + Arena* arena) const = 0; PROTOBUF_DEPRECATED_MSG("Please use ByteSizeLong() instead") virtual int ByteSize() const { return internal::ToIntSize(ByteSizeLong()); } @@ -654,7 +655,8 @@ class PROTOBUF_EXPORT ExtensionSet { int GetSize() const; void Free(); size_t SpaceUsedExcludingSelfLong() const; - bool IsInitialized() const; + bool IsInitialized(const ExtensionSet* ext_set, const MessageLite* extendee, + int number, Arena* arena) const; }; // The Extension struct is small enough to be passed by value, so we use it diff --git a/src/google/protobuf/reflection_ops.cc b/src/google/protobuf/reflection_ops.cc index 59d4c9342afa..a37e8fb1a5a5 100644 --- a/src/google/protobuf/reflection_ops.cc +++ b/src/google/protobuf/reflection_ops.cc @@ -247,9 +247,20 @@ bool ReflectionOps::IsInitialized(const Message& message, bool check_fields, } } } - if (check_descendants && reflection->HasExtensionSet(message) && - !reflection->GetExtensionSet(message).IsInitialized()) { - return false; + if (check_descendants && reflection->HasExtensionSet(message)) { + // Note that "extendee" is only referenced if the extension is lazily parsed + // (e.g. LazyMessageExtensionImpl), which requires a verification function + // to be generated. + // + // Dynamic messages would get null prototype from the generated message + // factory but their verification functions are not generated. Therefore, it + // it will always be eagerly parsed and "extendee" here will not be + // referenced. + const Message* extendee = + MessageFactory::generated_factory()->GetPrototype(descriptor); + if (!reflection->GetExtensionSet(message).IsInitialized(extendee)) { + return false; + } } return true; } diff --git a/src/google/protobuf/unittest_mset.proto b/src/google/protobuf/unittest_mset.proto index fd6aaa568ab9..b95acace58ae 100644 --- a/src/google/protobuf/unittest_mset.proto +++ b/src/google/protobuf/unittest_mset.proto @@ -80,6 +80,7 @@ message TestMessageSetExtension3 { optional TestMessageSetExtension3 message_set_extension = 195273129; } optional NestedTestInt msg = 35; + required int32 required_int = 36; } // This message was used to generate