Skip to content

Commit 9404f28

Browse files
paulb777var-const
andauthored
Fix a memory leak introduced in #6418 (#6778) (#6779)
The root of the issue is that when serializing a singular filter, it is being treated as a unary filter before it is definitively established whether it is a unary filter or a field filter. The leak is caused by always serializing the unary filter's field path field for equality and non-equality filters -- if the filter's value turns out not to be NaN or null, the serialization code switches the filter's type to a field filter without clearing the partially-initialized unary filter. `pb_release` would not free the `unary_filter.field.field_path` member variable because it would consider the object not to be a unary filter. Also a small refactoring to make the function easier to digest. Co-authored-by: Konstantin Varlamov <[email protected]>
1 parent 12c567b commit 9404f28

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

Firestore/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
setting. Timestamp fields that read from a `FIRDocumentSnapshot` now always
66
return `FIRTimestamp` objects. Use `FIRTimestamp.dateValue` to convert to
77
`NSDate` if required.
8+
- [fixed] Fixed a memory leak introduced in 1.18.0 that may manifest when
9+
serializing queries containing equality or non-equality comparisons.
810

911
# v1.19.0
1012
- [changed] Internal improvements for future C++ and Unity support. Includes a

Firestore/core/src/remote/serializer.cc

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,30 +1102,34 @@ google_firestore_v1_StructuredQuery_Filter Serializer::EncodeSingularFilter(
11021102
const FieldFilter& filter) const {
11031103
google_firestore_v1_StructuredQuery_Filter result{};
11041104

1105-
if (filter.op() == Filter::Operator::Equal ||
1106-
filter.op() == Filter::Operator::NotEqual) {
1105+
bool is_unary = (filter.op() == Filter::Operator::Equal ||
1106+
filter.op() == Filter::Operator::NotEqual) &&
1107+
(filter.value().is_nan() || filter.value().is_null());
1108+
if (is_unary) {
11071109
result.which_filter_type =
11081110
google_firestore_v1_StructuredQuery_Filter_unary_filter_tag;
11091111
result.unary_filter.which_operand_type =
11101112
google_firestore_v1_StructuredQuery_UnaryFilter_field_tag;
11111113
result.unary_filter.field.field_path = EncodeFieldPath(filter.field());
11121114

1115+
bool is_equality = filter.op() == Filter::Operator::Equal;
11131116
if (filter.value().is_nan()) {
1114-
auto op =
1115-
filter.op() == Filter::Operator::Equal
1117+
result.unary_filter.op =
1118+
is_equality
11161119
? google_firestore_v1_StructuredQuery_UnaryFilter_Operator_IS_NAN
11171120
: google_firestore_v1_StructuredQuery_UnaryFilter_Operator_IS_NOT_NAN; // NOLINT
1118-
result.unary_filter.op = op;
1119-
return result;
11201121

11211122
} else if (filter.value().is_null()) {
1122-
auto op =
1123-
filter.op() == Filter::Operator::Equal
1123+
result.unary_filter.op =
1124+
is_equality
11241125
? google_firestore_v1_StructuredQuery_UnaryFilter_Operator_IS_NULL
11251126
: google_firestore_v1_StructuredQuery_UnaryFilter_Operator_IS_NOT_NULL; // NOLINT
1126-
result.unary_filter.op = op;
1127-
return result;
1127+
1128+
} else {
1129+
HARD_FAIL("Expected a unary filter");
11281130
}
1131+
1132+
return result;
11291133
}
11301134

11311135
result.which_filter_type =

0 commit comments

Comments
 (0)