handle invalid unchanged-names input with BloomFilter::Create

Firestore/Example/Tests/SpecTests/FSTSpecTests.mm

@@ -119,6 +119,7 @@
 using firebase::firestore::util::MakeStringPtr;
 using firebase::firestore::util::Path;
 using firebase::firestore::util::Status;
+using firebase::firestore::util::StatusOr;
 using firebase::firestore::util::TimerId;
 using firebase::firestore::util::ToString;
 using firebase::firestore::util::WrapCompare;
@@ -329,20 +330,28 @@ - (SnapshotVersion)parseVersion:(NSNumber *_Nullable)version {
   return Version(version.longLongValue);
 }
 
-- (BloomFilter)parseBloomFilter:(NSDictionary *_Nullable)bloomFilterProto {
+- (absl::optional<BloomFilter>)parseBloomFilter:(NSDictionary *_Nullable)bloomFilterProto {
+  if (bloomFilterProto == nil) {
+    return absl::nullopt;
+  }
   NSDictionary *bitsData = bloomFilterProto[@"bits"];
 
-  NSString *bitmapData = bitsData[@"bitmap"];
-  // Decode base64 json string: bitmap
-  std::string bitmap;
-  absl::Base64Unescape([bitmapData UTF8String], &bitmap);
-  std::vector<uint8_t> bitmapVector(bitmap.begin(), bitmap.end());
+  // Decode base64 string into uint8_t vector. If not specified, will default bitmap to empty
+  // vector.
+  NSString *bitmapEncoded = bitsData[@"bitmap"];
+  std::string bitmapDecoded;
+  absl::Base64Unescape([bitmapEncoded UTF8String], &bitmapDecoded);
+  std::vector<uint8_t> bitmap(bitmapDecoded.begin(), bitmapDecoded.end());
 
+  // If not specified, will default padding and hashCount to 0.
   int32_t padding = [bitsData[@"padding"] intValue];
   int32_t hashCount = [bloomFilterProto[@"hashCount"] intValue];
+  StatusOr<BloomFilter> maybeBloomFilter = BloomFilter::Create(bitmap, padding, hashCount);
+  if (maybeBloomFilter.ok()) {
+    return maybeBloomFilter.ValueOrDie();
+  }
 
-  BloomFilter filter(bitmapVector, padding, hashCount);
-  return filter;
+  return absl::nullopt;
 }
 
 - (DocumentViewChange)parseChange:(NSDictionary *)jsonDoc ofType:(DocumentViewChange::Type)type {
@@ -488,9 +497,9 @@ - (void)doWatchFilter:(NSDictionary *)watchFilter {
   HARD_ASSERT(targets.count == 1, "ExistenceFilters currently support exactly one target only.");
 
   NSArray<NSNumber *> *keys = watchFilter[@"keys"];
-  int keyCount = keys ? keys.count : 0;
+  int keyCount = keys ? (int)keys.count : 0;
 
-  BloomFilter bloomFilter = [self parseBloomFilter:watchFilter[@"bloomFilter"]];
+  absl::optional<BloomFilter> bloomFilter = [self parseBloomFilter:watchFilter[@"bloomFilter"]];
 
   ExistenceFilter filter{keyCount, bloomFilter};
   ExistenceFilterWatchChange change{filter, targets[0].intValue};

Firestore/Protos/nanopb/google/firestore/v1/bloom_filter.nanopb.cc

@@ -44,7 +44,7 @@ const pb_field_t google_firestore_v1_BitSequence_fields[3] = {
 };
 
 const pb_field_t google_firestore_v1_BloomFilter_fields[3] = {
-    PB_FIELD(  1, MESSAGE , SINGULAR, STATIC  , FIRST, google_firestore_v1_BloomFilter, bits, bits, &google_firestore_v1_BitSequence_fields),
+    PB_FIELD(  1, MESSAGE , OPTIONAL, STATIC  , FIRST, google_firestore_v1_BloomFilter, bits, bits, &google_firestore_v1_BitSequence_fields),
     PB_FIELD(  2, INT32   , SINGULAR, STATIC  , OTHER, google_firestore_v1_BloomFilter, hash_count, bits, 0),
     PB_LAST_FIELD
 };
@@ -96,7 +96,9 @@ std::string google_firestore_v1_BloomFilter::ToString(int indent) const {
     std::string tostring_header = PrintHeader(indent, "BloomFilter", this);
     std::string tostring_result;
 
-    tostring_result += PrintMessageField("bits ", bits, indent + 1, false);
+    if (has_bits) {
+        tostring_result += PrintMessageField("bits ", bits, indent + 1, true);
+    }
     tostring_result += PrintPrimitiveField("hash_count: ",
         hash_count, indent + 1, false);
 

Firestore/Protos/nanopb/google/firestore/v1/bloom_filter.nanopb.h

@@ -42,6 +42,7 @@ typedef struct _google_firestore_v1_BitSequence {
 } google_firestore_v1_BitSequence;
 
 typedef struct _google_firestore_v1_BloomFilter {
+    bool has_bits;
     google_firestore_v1_BitSequence bits;
     int32_t hash_count;
 
@@ -53,9 +54,9 @@ typedef struct _google_firestore_v1_BloomFilter {
 
 /* Initializer values for message structs */
 #define google_firestore_v1_BitSequence_init_default {NULL, 0}
-#define google_firestore_v1_BloomFilter_init_default {google_firestore_v1_BitSequence_init_default, 0}
+#define google_firestore_v1_BloomFilter_init_default {false, google_firestore_v1_BitSequence_init_default, 0}
 #define google_firestore_v1_BitSequence_init_zero {NULL, 0}
-#define google_firestore_v1_BloomFilter_init_zero {google_firestore_v1_BitSequence_init_zero, 0}
+#define google_firestore_v1_BloomFilter_init_zero {false, google_firestore_v1_BitSequence_init_zero, 0}
 
 /* Field tags (for use in manual encoding/decoding) */
 #define google_firestore_v1_BitSequence_bitmap_tag 1

Firestore/Protos/protos/google/firestore/v1/bloom_filter.options

@@ -0,0 +1,21 @@
+# Copyright 2023 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# In proto3 mode, Nanopb doesn't allow distinguishing between unset fields and
+# fields having default values, even for non-primitive types. Using the
+# workaround suggested in
+# https://github.com/nanopb/nanopb/issues/255#issuecomment-291842903
+
+# `bits` might not be set if the BloomFilter is empty.
+google.firestore.v1.BloomFilter.bits proto3:false

Firestore/core/src/remote/serializer.cc

@@ -1426,18 +1426,31 @@ std::unique_ptr<WatchChange> Serializer::DecodeDocumentRemove(
 
 std::unique_ptr<WatchChange> Serializer::DecodeExistenceFilterWatchChange(
     ReadContext*, const google_firestore_v1_ExistenceFilter& filter) const {
+  ExistenceFilter existence_filter = DecodeExistenceFilter(filter);
+  return absl::make_unique<ExistenceFilterWatchChange>(
+      std::move(existence_filter), filter.target_id);
+}
+
+ExistenceFilter Serializer::DecodeExistenceFilter(
+    const google_firestore_v1_ExistenceFilter& filter) const {
+  // Create bloom filter if there is an unchanged_names present in the filter
+  // and inputs are valid, otherwise keep it null.
   absl::optional<BloomFilter> bloom_filter = absl::nullopt;
-  if (filter.has_unchanged_names) {
+  if (filter.has_unchanged_names && filter.unchanged_names.has_bits) {
+    // TODO(Mila): Ensure bloom filter with invalid inputs are handled correctly
+    // in next PR.
     ByteString bitmap_string(filter.unchanged_names.bits.bitmap);
     std::vector<uint8_t> bitmap = MakeVector(bitmap_string);
-
-    bloom_filter = BloomFilter(bitmap, filter.unchanged_names.bits.padding,
-                               filter.unchanged_names.hash_count);
+    int32_t padding = filter.unchanged_names.bits.padding;
+    int32_t hash_count = filter.unchanged_names.hash_count;
+    StatusOr<BloomFilter> maybe_bloom_filter =
+        BloomFilter::Create(bitmap, padding, hash_count);
+    if (maybe_bloom_filter.ok()) {
+      bloom_filter = maybe_bloom_filter.ValueOrDie();
+    }
   }
 
-  ExistenceFilter existence_filter{filter.count, bloom_filter};
-  return absl::make_unique<ExistenceFilterWatchChange>(
-      std::move(existence_filter), filter.target_id);
+  return {filter.count, bloom_filter};
 }
 
 bool Serializer::IsLocalResourceName(const ResourcePath& path) const {

Firestore/core/src/remote/serializer.h

@@ -341,6 +341,9 @@ class Serializer {
       util::ReadContext* context,
       const google_firestore_v1_ExistenceFilter& filter) const;
 
+  ExistenceFilter DecodeExistenceFilter(
+      const google_firestore_v1_ExistenceFilter& filter) const;
+
   model::DatabaseId database_id_;
   // TODO(varconst): Android caches the result of calling `EncodeDatabaseName`
   // as well, consider implementing that.