Skip to content

Commit

Permalink
Stop assuming lazy message sets are initialized.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 504569894
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Jan 25, 2023
1 parent 8870581 commit 3774ee0
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/cpp/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
18 changes: 9 additions & 9 deletions src/google/protobuf/descriptor.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 27 additions & 19 deletions src/google/protobuf/extension_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 5 additions & 3 deletions src/google/protobuf/extension_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()); }
Expand Down Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions src/google/protobuf/reflection_ops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/unittest_mset.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3774ee0

Please sign in to comment.