Skip to content

Commit 74c7c1f

Browse files
authored
Fix memory leak from ASAN checks (#9750)
1 parent e4d968a commit 74c7c1f

File tree

2 files changed

+24
-128
lines changed

2 files changed

+24
-128
lines changed

Firestore/core/src/model/value_util.cc

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,26 @@ const char* kRawMaxValueFieldValue = "__max__";
5353
pb_bytes_array_s* kMaxValueFieldValue =
5454
nanopb::MakeBytesArray(kRawMaxValueFieldValue);
5555

56+
/** The special map field value entry of a maximum proto value. */
57+
google_firestore_v1_MapValue_FieldsEntry kMaxValueFieldEntry = {
58+
.key = kMaxValueFieldKey,
59+
.value = {
60+
.which_value_type = google_firestore_v1_Value_string_value_tag,
61+
.string_value = const_cast<pb_bytes_array_t*>(kMaxValueFieldValue)}};
62+
63+
/** The special map value of a maximum proto value. */
64+
_google_firestore_v1_MapValue kMaxValueMapValue = {
65+
.fields_count = 1, .fields = &kMaxValueFieldEntry};
66+
67+
/**
68+
* A maximum value that is larger than any other Firestore values. Underlying it
69+
* is a map value with a special map field that SDK user cannot possibly
70+
* construct.
71+
*/
72+
google_firestore_v1_Value kMaxValue = {
73+
.which_value_type = google_firestore_v1_Value_map_value_tag,
74+
.map_value = kMaxValueMapValue};
75+
5676
} // namespace
5777

5878
using nanopb::Message;
@@ -684,16 +704,7 @@ bool IsMinValue(const google_firestore_v1_Value& value) {
684704
}
685705

686706
google_firestore_v1_Value MaxValue() {
687-
google_firestore_v1_Value max_value;
688-
max_value.which_value_type = google_firestore_v1_Value_map_value_tag;
689-
max_value.map_value.fields_count = 1;
690-
max_value.map_value.fields =
691-
nanopb::MakeArray<google_firestore_v1_MapValue_FieldsEntry>(1);
692-
max_value.map_value.fields[0].key = kMaxValueFieldKey;
693-
max_value.map_value.fields[0].value.which_value_type =
694-
google_firestore_v1_Value_string_value_tag;
695-
max_value.map_value.fields[0].value.string_value = kMaxValueFieldValue;
696-
return max_value;
707+
return kMaxValue;
697708
}
698709

699710
bool IsMaxValue(const google_firestore_v1_Value& value) {

Firestore/core/test/unit/local/local_serializer_test.cc

Lines changed: 3 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -324,14 +324,13 @@ class LocalSerializerTest : public ::testing::Test {
324324

325325
void ExpectSerializationRoundTrip(const NamedQuery& named_query,
326326
const ::firestore::NamedQuery& proto) {
327-
ByteString bytes = EncodeNamedQuery(&serializer, named_query);
327+
ByteString bytes = EncodeNamedQuery(named_query);
328328
auto actual = ProtobufParse<::firestore::NamedQuery>(bytes);
329329
EXPECT_TRUE(msg_diff.Compare(proto, actual)) << message_differences;
330330
}
331331

332-
ByteString EncodeNamedQuery(local::LocalSerializer* serializer,
333-
const NamedQuery& named_query) {
334-
return MakeByteString(serializer->EncodeNamedQuery(named_query));
332+
ByteString EncodeNamedQuery(const NamedQuery& named_query) {
333+
return MakeByteString(serializer.EncodeNamedQuery(named_query));
335334
}
336335

337336
void ExpectDeserializationRoundTrip(const NamedQuery& named_query,
@@ -370,120 +369,6 @@ class LocalSerializerTest : public ::testing::Test {
370369
MessageDifferencer msg_diff;
371370
};
372371

373-
// TODO(b/174608374): Remove these tests once we perform a schema migration.
374-
TEST_F(LocalSerializerTest, SetMutationAndTransformMutationAreSquashed) {
375-
::firestore::client::WriteBatch batch_proto{};
376-
batch_proto.set_batch_id(42);
377-
*batch_proto.add_writes() = SetProto();
378-
*batch_proto.add_writes() = LegacyTransformProto();
379-
*batch_proto.mutable_local_write_time() = WriteTimeProto();
380-
381-
ByteString bytes = ProtobufSerialize(batch_proto);
382-
StringReader reader(bytes);
383-
auto message = Message<firestore_client_WriteBatch>::TryParse(&reader);
384-
MutationBatch decoded = serializer.DecodeMutationBatch(&reader, *message);
385-
ASSERT_EQ(1, decoded.mutations().size());
386-
ASSERT_EQ(Mutation::Type::Set, decoded.mutations()[0].type());
387-
388-
Message<google_firestore_v1_Write> encoded{
389-
remote_serializer.EncodeMutation(decoded.mutations()[0])};
390-
ExpectSet(*encoded);
391-
ExpectUpdateTransform(*encoded);
392-
}
393-
394-
// TODO(b/174608374): Remove these tests once we perform a schema migration.
395-
TEST_F(LocalSerializerTest, PatchMutationAndTransformMutationAreSquashed) {
396-
::firestore::client::WriteBatch batch_proto{};
397-
batch_proto.set_batch_id(42);
398-
*batch_proto.add_writes() = PatchProto();
399-
*batch_proto.add_writes() = LegacyTransformProto();
400-
*batch_proto.mutable_local_write_time() = WriteTimeProto();
401-
402-
ByteString bytes = ProtobufSerialize(batch_proto);
403-
StringReader reader(bytes);
404-
auto message = Message<firestore_client_WriteBatch>::TryParse(&reader);
405-
MutationBatch decoded = serializer.DecodeMutationBatch(&reader, *message);
406-
ASSERT_EQ(1, decoded.mutations().size());
407-
ASSERT_EQ(Mutation::Type::Patch, decoded.mutations()[0].type());
408-
409-
Message<google_firestore_v1_Write> encoded{
410-
remote_serializer.EncodeMutation(decoded.mutations()[0])};
411-
ExpectPatch(*encoded);
412-
ExpectUpdateTransform(*encoded);
413-
}
414-
415-
// TODO(b/174608374): Remove these tests once we perform a schema migration.
416-
TEST_F(LocalSerializerTest, TransformAndTransformThrowError) {
417-
::firestore::client::WriteBatch batch_proto{};
418-
batch_proto.set_batch_id(42);
419-
*batch_proto.add_writes() = LegacyTransformProto();
420-
*batch_proto.add_writes() = LegacyTransformProto();
421-
*batch_proto.mutable_local_write_time() = WriteTimeProto();
422-
423-
ByteString bytes = ProtobufSerialize(batch_proto);
424-
StringReader reader(bytes);
425-
auto message = Message<firestore_client_WriteBatch>::TryParse(&reader);
426-
EXPECT_ANY_THROW(serializer.DecodeMutationBatch(&reader, *message));
427-
}
428-
429-
// TODO(b/174608374): Remove these tests once we perform a schema migration.
430-
TEST_F(LocalSerializerTest, DeleteAndTransformThrowError) {
431-
::firestore::client::WriteBatch batch_proto{};
432-
batch_proto.set_batch_id(42);
433-
*batch_proto.add_writes() = DeleteProto();
434-
*batch_proto.add_writes() = LegacyTransformProto();
435-
*batch_proto.mutable_local_write_time() = WriteTimeProto();
436-
437-
ByteString bytes = ProtobufSerialize(batch_proto);
438-
StringReader reader(bytes);
439-
auto message = Message<firestore_client_WriteBatch>::TryParse(&reader);
440-
EXPECT_ANY_THROW(serializer.DecodeMutationBatch(&reader, *message));
441-
}
442-
443-
// TODO(b/174608374): Remove these tests once we perform a schema migration.
444-
TEST_F(LocalSerializerTest, MultipleMutationsAreSquashed) {
445-
::firestore::client::WriteBatch batch_proto{};
446-
batch_proto.set_batch_id(42);
447-
*batch_proto.add_writes() = SetProto();
448-
*batch_proto.add_writes() = SetProto();
449-
*batch_proto.add_writes() = LegacyTransformProto();
450-
*batch_proto.add_writes() = DeleteProto();
451-
*batch_proto.add_writes() = PatchProto();
452-
*batch_proto.add_writes() = LegacyTransformProto();
453-
*batch_proto.add_writes() = PatchProto();
454-
*batch_proto.mutable_local_write_time() = WriteTimeProto();
455-
456-
ByteString bytes = ProtobufSerialize(batch_proto);
457-
StringReader reader(bytes);
458-
auto message = Message<firestore_client_WriteBatch>::TryParse(&reader);
459-
MutationBatch decoded = serializer.DecodeMutationBatch(&reader, *message);
460-
ASSERT_EQ(5, decoded.mutations().size());
461-
462-
Message<google_firestore_v1_Write> encoded{
463-
remote_serializer.EncodeMutation(decoded.mutations()[0])};
464-
ExpectSet(*encoded);
465-
ExpectNoUpdateTransform(*encoded);
466-
467-
encoded =
468-
MakeMessage(remote_serializer.EncodeMutation(decoded.mutations()[1]));
469-
ExpectSet(*encoded);
470-
ExpectUpdateTransform(*encoded);
471-
472-
encoded =
473-
MakeMessage(remote_serializer.EncodeMutation(decoded.mutations()[2]));
474-
ExpectDelete(*encoded);
475-
476-
encoded =
477-
MakeMessage(remote_serializer.EncodeMutation(decoded.mutations()[3]));
478-
ExpectPatch(*encoded);
479-
ExpectUpdateTransform(*encoded);
480-
481-
encoded =
482-
MakeMessage(remote_serializer.EncodeMutation(decoded.mutations()[4]));
483-
ExpectPatch(*encoded);
484-
ExpectNoUpdateTransform(*encoded);
485-
}
486-
487372
TEST_F(LocalSerializerTest, EncodesMutationBatch) {
488373
Mutation base =
489374
PatchMutation(Key("docs/1"), WrapObject("a", "b"), FieldMask{Field("a")},

0 commit comments

Comments
 (0)