Merge branch 'master' into mila/BloomFilter

Author: milaGGL

CONTRIBUTING.md

@@ -132,7 +132,7 @@ To develop Firebase software, **install**:
    To install [clang-format] and [mint] using [Homebrew]:
 
     ```console
-    brew install clang-format@15
+    brew install clang-format@16
     brew install mint
     ```
 

Crashlytics/Crashlytics/Helpers/FIRCLSDefines.h

@@ -23,8 +23,9 @@
 // These macros generate a function to force a symbol for the containing .o, to work around an issue
 // where strip will not strip debug information without a symbol to strip.
 #define DUMMY_FUNCTION_NAME(x) CONCAT(fircls_strip_this_, x)
-#define INJECT_STRIP_SYMBOL(x) \
-  void DUMMY_FUNCTION_NAME(x)(void) {}
+#define INJECT_STRIP_SYMBOL(x)        \
+  void DUMMY_FUNCTION_NAME(x)(void) { \
+  }
 
 // These make some target os types available to previous versions of xcode that do not yet have them
 // in their SDKs

Crashlytics/Shared/FIRCLSMachO/FIRCLSCodeMapping.m

@@ -22,11 +22,11 @@ @interface FIRCLSCodeMapping () {
 
 @implementation FIRCLSCodeMapping
 
-+ (instancetype)mappingWithURL:(NSURL*)URL {
++ (instancetype)mappingWithURL:(NSURL *)URL {
   return [[self alloc] initWithURL:URL];
 }
 
-- (instancetype)initWithURL:(NSURL*)URL {
+- (instancetype)initWithURL:(NSURL *)URL {
   self = [super init];
   if (!self) {
     return nil;

Crashlytics/UnitTests/FIRCLSDwarfExpressionTests.m

@@ -131,7 +131,7 @@ - (void)testDwarfExpressionMachineInit {
   uint8_t fakeData;
   FIRCLSThreadContext registers;
 
-  XCTAssert(FIRCLSDwarfExpressionMachineInit(&machine, (void*)&fakeData, &registers, 42));
+  XCTAssert(FIRCLSDwarfExpressionMachineInit(&machine, (void *)&fakeData, &registers, 42));
   XCTAssert(FIRCLSDwarfExpressionStackIsValid(&machine.stack));
 
   XCTAssertEqual(FIRCLSDwarfExpressionStackPeek(&machine.stack), 42);

FirebaseDatabase/Tests/Helpers/SenTest+FWaiter.h

@@ -19,10 +19,10 @@
 @interface XCTest (FWaiter)
 
 - (NSTimeInterval)waitUntil:(BOOL (^)(void))predicate;
-- (NSTimeInterval)waitUntil:(BOOL (^)(void))predicate description:(NSString*)desc;
+- (NSTimeInterval)waitUntil:(BOOL (^)(void))predicate description:(NSString *)desc;
 - (NSTimeInterval)waitUntil:(BOOL (^)(void))predicate timeout:(NSTimeInterval)seconds;
 - (NSTimeInterval)waitUntil:(BOOL (^)(void))predicate
                     timeout:(NSTimeInterval)seconds
-                description:(NSString*)desc;
+                description:(NSString *)desc;
 
 @end

FirebaseDatabase/Tests/Integration/FData.m

@@ -328,7 +328,8 @@ - (void)testWriteLeafNodeOverwriteAtParentMultipleTimesVerifyExpectedEvents {
                        "already finished." /
                   Users / runner / runners / 2.262.1 / work / firebase -
               ios - sdk / firebase - ios -
-              sdk / Example / Database / Tests / Helpers / FEventTester.m : 123
+              sdk / Example / Database / Tests / Helpers / FEventTester.m:
+123
 ``` FTupleEventTypeString *recvd = [self.actualPathsAndEvents objectAtIndex:i];
 XCTAssertTrue([target isEqualTo:recvd], @"Expected %@ to match %@", target, recvd);
 

FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m

@@ -15,7 +15,7 @@
  */
 
 // Uncomment or set the flag in GCC_PREPROCESSOR_DEFINITIONS to enable integration tests.
-//#define FIR_INSTALLATIONS_INTEGRATION_TESTS_REQUIRED 1
+// #define FIR_INSTALLATIONS_INTEGRATION_TESTS_REQUIRED 1
 
 // macOS requests a user password when accessing the Keychain for the first time,
 // so the tests may fail. Disable integration tests on macOS so far.

FirebaseRemoteConfig/Sources/Public/FirebaseRemoteConfig/FIRRemoteConfig.h

@@ -32,7 +32,7 @@ extern NSString *const _Nonnull FIRRemoteConfigThrottledEndTimeInSecondsKey NS_S
 
 /**
  * Listener registration returned by `addOnConfigUpdateListener`. Calling its method `remove` stops
- * the listener from receiving config updates and unregisters itself.
+ * the associated listener from receiving config updates and unregisters itself.
  *
  * If remove is called and no other listener registrations remain, the connection to the real-time
  * RC backend is closed. Subsequently calling `addOnConfigUpdateListener` will re-open the
@@ -41,8 +41,8 @@ extern NSString *const _Nonnull FIRRemoteConfigThrottledEndTimeInSecondsKey NS_S
 NS_SWIFT_NAME(ConfigUpdateListenerRegistration)
 @interface FIRConfigUpdateListenerRegistration : NSObject
 /**
- * Removes the listener being tracked by this `ConfigUpdateListenerRegistration`. After the initial
- * call, subsequent calls have no effect.
+ * Removes the listener associated with this `ConfigUpdateListenerRegistration`. After the
+ * initial call, subsequent calls have no effect.
  */
 - (void)remove;
 @end
@@ -82,17 +82,17 @@ typedef NS_ERROR_ENUM(FIRRemoteConfigErrorDomain, FIRRemoteConfigError){
     FIRRemoteConfigErrorInternalError = 8003,
 } NS_SWIFT_NAME(RemoteConfigError);
 
-/// Remote Config error domain that handles errors for the real-time service.
+/// Remote Config error domain that handles errors for the real-time config update service.
 extern NSString *const _Nonnull FIRRemoteConfigUpdateErrorDomain NS_SWIFT_NAME(RemoteConfigUpdateErrorDomain);
-/// Firebase Remote Config real-time service error.
+/// Firebase Remote Config real-time config update service error.
 typedef NS_ERROR_ENUM(FIRRemoteConfigUpdateErrorDomain, FIRRemoteConfigUpdateError){
-    /// Unable to make a connection to the backend.
+    /// Unable to make a connection to the Remote Config backend.
     FIRRemoteConfigUpdateErrorStreamError = 8001,
-    /// Unable to fetch the latest config.
+    /// Unable to fetch the latest version of the config.
     FIRRemoteConfigUpdateErrorNotFetched = 8002,
     /// The ConfigUpdate message was unparsable.
     FIRRemoteConfigUpdateErrorMessageInvalid = 8003,
-    /// The real-time Remote Config service is unavailable.
+    /// The Remote Config real-time config update service is unavailable.
     FIRRemoteConfigUpdateErrorUnavailable = 8004,
 } NS_SWIFT_NAME(RemoteConfigUpdateError);
 
@@ -171,7 +171,9 @@ NS_SWIFT_NAME(RemoteConfigSettings)
 @end
 
 #pragma mark - FIRRemoteConfigUpdate
-/// Firebase Remote Config update
+/// Used by Remote Config real-time config update service, this class represents changes between the
+/// newly fetched config and the current one. An instance of this class is passed to
+/// `FIRRemoteConfigUpdateCompletion` when a new config version has been automatically fetched.
 NS_SWIFT_NAME(RemoteConfigUpdate)
 @interface FIRRemoteConfigUpdate : NSObject
 
@@ -325,12 +327,13 @@ NS_SWIFT_NAME(RemoteConfig)
 ///                         nil if the key doesn't exist in the default config.
 - (nullable FIRRemoteConfigValue *)defaultValueForKey:(nullable NSString *)key;
 
-#pragma mark - Realtime
+#pragma mark - Real-time Config Updates
 
 /// Completion handler invoked by `addOnConfigUpdateListener` when there is an update to
 /// the config from the backend.
 ///
-/// @param configUpdate Information on which key's values have changed
+/// @param configUpdate An instance of `FIRRemoteConfigUpdate` that contains information on which
+/// key's values have changed.
 /// @param error  Error message on failure.
 typedef void (^FIRRemoteConfigUpdateCompletion)(FIRRemoteConfigUpdate *_Nullable configUpdate,
                                                 NSError *_Nullable error)
@@ -349,8 +352,7 @@ typedef void (^FIRRemoteConfigUpdateCompletion)(FIRRemoteConfigUpdate *_Nullable
 ///
 /// @param listener              The configured listener that is called for every config update.
 /// @return              Returns a registration representing the listener. The registration contains
-/// a remove method, which can be used to stop receiving for updates for this particular
-/// registration.
+/// a remove method, which can be used to stop receiving updates for the provided listener.
 - (FIRConfigUpdateListenerRegistration *_Nonnull)addOnConfigUpdateListener:
     (FIRRemoteConfigUpdateCompletion _Nonnull)listener
     NS_SWIFT_NAME(addOnConfigUpdateListener(remoteConfigUpdateCompletion:));

FirebaseRemoteConfig/Tests/Sample/RemoteConfigSampleApp/FRCLog.h

@@ -19,6 +19,6 @@
 
 + (instancetype)sharedInstance;
 
-- (void)setLogView:(UITextView*)view;
-- (void)logToConsole:(NSString*)text;
+- (void)setLogView:(UITextView *)view;
+- (void)logToConsole:(NSString *)text;
 @end

FirebaseRemoteConfig/Tests/Unit/RCNConfigAnalyticsTest.m

@@ -17,7 +17,7 @@
 #import <OCMock/OCMock.h>
 #import <XCTest/XCTest.h>
 
-//#import "RCNConfigAnalytics.h"
+// #import "RCNConfigAnalytics.h"
 
 @interface RCNConfigAnalyticsTest : XCTestCase {
   id _mockAnalytics;

FirebaseRemoteConfig/Tests/Unit/RCNRemoteConfig+FIRAppTest.m

@@ -17,10 +17,10 @@
 #import <OCMock/OCMock.h>
 #import <XCTest/XCTest.h>
 
-//#import "FIRRemoteConfig+FIRApp.h"
+// #import "FIRRemoteConfig+FIRApp.h"
 #import "FirebaseCore/Extension/FirebaseCoreInternal.h"
 #import "FirebaseRemoteConfig/Sources/Private/FIRRemoteConfig_Private.h"
-//#import "third_party/firebase/ios/Releases/FirebaseCore/Tests/FIRTestCase.h"
+// #import "third_party/firebase/ios/Releases/FirebaseCore/Tests/FIRTestCase.h"
 
 @interface RCNRemoteConfig_FIRAppTest : FIRTestCase
 

Firestore/CHANGELOG.md

@@ -1,5 +1,6 @@
 # 10.7.0
 - [feature] Add support for disjunctions in queries (`OR` queries).
+- [fixed] Fixed stack overflow caused by deeply nested server timestamps.
 
 # 10.6.0
 - [fixed] Fix a potential high memory usage issue.

Firestore/Source/API/FSTUserDataWriter.h

@@ -35,6 +35,6 @@ namespace api = firebase::firestore::api;
 - (instancetype)initWithFirestore:(std::shared_ptr<api::Firestore>)firestore
           serverTimestampBehavior:(FIRServerTimestampBehavior)serverTimestampBehavior;
 
-- (id)convertedValue:(const firebase::firestore::google_firestore_v1_Value&)value;
+- (id)convertedValue:(const firebase::firestore::google_firestore_v1_Value &)value;
 
 @end

Firestore/core/src/model/server_timestamp_util.cc

@@ -35,6 +35,17 @@ const char kServerTimestampSentinel[] = "server_timestamp";
 Message<google_firestore_v1_Value> EncodeServerTimestamp(
     const Timestamp& local_write_time,
     absl::optional<google_firestore_v1_Value> previous_value) {
+  // We should avoid storing deeply nested server timestamp map values
+  // because we never use the intermediate "previous values".
+  // For example:
+  // previous: 42L, add: t1, result: t1 -> 42L
+  // previous: t1,  add: t2, result: t2 -> 42L (NOT t2 -> t1 -> 42L)
+  // previous: t2,  add: t3, result: t3 -> 42L (NOT t3 -> t2 -> t1 -> 42L)
+  // `getPreviousValue` recursively traverses server timestamps to find the
+  // least recent Value.
+  if (previous_value.has_value() && IsServerTimestamp(*previous_value)) {
+    previous_value = GetPreviousValue(*previous_value);
+  }
   pb_size_t count = previous_value ? 3 : 2;
 
   Message<google_firestore_v1_Value> result;

Firestore/core/src/remote/serializer.cc

@@ -43,13 +43,15 @@
 #include "Firestore/core/src/model/resource_path.h"
 #include "Firestore/core/src/model/server_timestamp_util.h"
 #include "Firestore/core/src/model/set_mutation.h"
+#include "Firestore/core/src/model/snapshot_version.h"
 #include "Firestore/core/src/model/value_util.h"
 #include "Firestore/core/src/model/verify_mutation.h"
 #include "Firestore/core/src/nanopb/byte_string.h"
 #include "Firestore/core/src/nanopb/nanopb_util.h"
 #include "Firestore/core/src/nanopb/reader.h"
 #include "Firestore/core/src/nanopb/writer.h"
 #include "Firestore/core/src/timestamp_internal.h"
+#include "Firestore/core/src/util/comparison.h"
 #include "Firestore/core/src/util/hard_assert.h"
 #include "Firestore/core/src/util/status.h"
 #include "Firestore/core/src/util/statusor.h"
@@ -112,6 +114,7 @@ using nanopb::SetRepeatedField;
 using nanopb::SharedMessage;
 using nanopb::Writer;
 using remote::WatchChange;
+using util::ComparisonResult;
 using util::ReadContext;
 using util::Status;
 using util::StatusOr;
@@ -638,8 +641,19 @@ google_firestore_v1_Target Serializer::EncodeTarget(
         nanopb::CopyBytesArray(target_data.resume_token().get());
 
     if (target_data.expected_count().has_value()) {
+      int32_t expected_count = target_data.expected_count().value();
       result.has_expected_count = true;
+      result.expected_count.value = expected_count;
+    }
+  } else if (target_data.snapshot_version().CompareTo(
+                 SnapshotVersion::None()) == ComparisonResult::Descending) {
+    result.which_resume_type = google_firestore_v1_Target_read_time_tag;
+    result.resume_type.read_time =
+        EncodeVersion(target_data.snapshot_version());
+
+    if (target_data.expected_count().has_value()) {
       int32_t expected_count = target_data.expected_count().value();
+      result.has_expected_count = true;
       result.expected_count.value = expected_count;
     }
   }

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

@@ -16,6 +16,7 @@
 
 #include "Firestore/core/test/unit/local/local_store_test.h"
 
+#include <thread>  // NOLINT(build/c++11)
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -34,10 +35,14 @@
 #include "Firestore/core/src/model/document.h"
 #include "Firestore/core/src/model/document_key.h"
 #include "Firestore/core/src/model/field_index.h"
+#include "Firestore/core/src/model/field_mask.h"
+#include "Firestore/core/src/model/field_path.h"
+#include "Firestore/core/src/model/field_transform.h"
 #include "Firestore/core/src/model/mutable_document.h"
 #include "Firestore/core/src/model/mutation.h"
 #include "Firestore/core/src/model/mutation_batch_result.h"
 #include "Firestore/core/src/model/patch_mutation.h"
+#include "Firestore/core/src/model/server_timestamp_util.h"
 #include "Firestore/core/src/model/set_mutation.h"
 #include "Firestore/core/src/model/transform_operation.h"
 #include "Firestore/core/src/remote/existence_filter.h"
@@ -92,6 +97,7 @@ using testutil::Key;
 using testutil::Map;
 using testutil::OverlayTypeMap;
 using testutil::Query;
+using testutil::ServerTimestamp;
 using testutil::UnknownDoc;
 using testutil::UpdateRemoteEvent;
 using testutil::UpdateRemoteEventWithLimboTargets;
@@ -1685,6 +1691,24 @@ TEST_P(LocalStoreTest, PatchMutationLeadsToPatchOverlay) {
       OverlayTypeMap({{Key("foo/baz"), model::Mutation::Type::Patch}}));
 }
 
+TEST_P(LocalStoreTest, DeeplyNestedTimestampDoesNotCauseStackOverflow) {
+  Timestamp timestamp = Timestamp::Now();
+  Message<_google_firestore_v1_Value> initialServerTimestamp =
+      model::EncodeServerTimestamp(timestamp, absl::nullopt);
+  model::FieldPath path = model::FieldPath::FromDotSeparatedString("timestamp");
+  auto makeDeeplyNestedTimestamp = [&]() {
+    for (int i = 0; i < 1000; ++i) {
+      WriteMutation(testutil::MergeMutation(
+          "foo/bar",
+          Map("timestamp",
+              model::EncodeServerTimestamp(timestamp, *initialServerTimestamp)),
+          {path}, {ServerTimestamp("timestamp")}));
+    }
+  };
+  std::thread t(makeDeeplyNestedTimestamp);
+  EXPECT_NO_FATAL_FAILURE(t.join());
+}
+
 }  // namespace local
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/test/unit/remote/serializer_test.cc

@@ -1125,6 +1125,42 @@ TEST_F(SerializerTest, EncodesFirstLevelKeyQueries) {
   ExpectRoundTrip(model, proto);
 }
 
+TEST_F(SerializerTest, EncodesTargetDataWithExpectedResumeType) {
+  TargetData target = CreateTargetData("docs/1");
+
+  {
+    SCOPED_TRACE("EncodesTargetDataWithoutResumeType");
+    v1::Target proto;
+    proto.mutable_documents()->add_documents(ResourceName("docs/1"));
+    proto.set_target_id(1);
+    ExpectRoundTrip(target, proto);
+  }
+
+  {
+    SCOPED_TRACE("EncodesTargetDataWithResumeToken");
+    v1::Target proto;
+    proto.mutable_documents()->add_documents(ResourceName("docs/1"));
+    proto.set_target_id(1);
+    proto.set_resume_token("resume_token");
+    ExpectRoundTrip(target.WithResumeToken(nanopb::ByteString{"resume_token"},
+                                           model::SnapshotVersion::None()),
+                    proto);
+  }
+
+  {
+    SCOPED_TRACE("EncodesTargetDataWithResumeByReadTime");
+    v1::Target proto;
+    proto.mutable_documents()->add_documents(ResourceName("docs/1"));
+    proto.set_target_id(1);
+    proto.mutable_read_time()->set_seconds(1000);
+    proto.mutable_read_time()->set_nanos(42);
+    ExpectRoundTrip(
+        target.WithResumeToken(nanopb::ByteString{""},
+                               model::SnapshotVersion(Timestamp(1000, 42))),
+        proto);
+  }
+}
+
 TEST_F(SerializerTest, EncodesFirstLevelAncestorQueries) {
   TargetData model = CreateTargetData("messages");
 

Gemfile.lock

@@ -3,7 +3,7 @@ GEM
   specs:
     CFPropertyList (3.0.6)
       rexml
-    activesupport (7.0.4.2)
+    activesupport (7.0.4.3)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
       minitest (>= 5.1)
@@ -119,7 +119,7 @@ GEM
       rexml
     kramdown-parser-gfm (1.1.0)
       kramdown (~> 2.0)
-    minitest (5.17.0)
+    minitest (5.18.0)
     molinillo (0.8.0)
     multipart-post (2.1.1)
     nanaimo (0.3.0)

README.md

@@ -140,7 +140,7 @@ GitHub Actions will verify that any code changes are done in a style compliant
 way. Install `clang-format` and `mint`:
 
 ```console
-brew install clang-format@15
+brew install clang-format@16
 brew install mint
 ```
 

scripts/setup_check.sh

@@ -35,7 +35,7 @@ fi
 
 # install clang-format
 brew update
-brew install clang-format@15
+brew install clang-format@16
 
 # mint installs tools from Mintfile on demand.
 brew install mint