Avoid deeply nested timestamps that can cause stack overflow.

Author: ehsannas

Firestore/CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 - [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/core/src/model/server_timestamp_util.cc

@@ -35,7 +35,10 @@ const char kServerTimestampSentinel[] = "server_timestamp";
 Message<google_firestore_v1_Value> EncodeServerTimestamp(
     const Timestamp& local_write_time,
     absl::optional<google_firestore_v1_Value> previous_value) {
-  pb_size_t count = previous_value ? 3 : 2;
+  absl::optional<google_firestore_v1_Value> actual_previous_value =
+      previous_value.has_value() ? GetPreviousValue(*previous_value)
+                                 : absl::nullopt;
+  pb_size_t count = actual_previous_value ? 3 : 2;
 
   Message<google_firestore_v1_Value> result;
   result->which_value_type = google_firestore_v1_Value_map_value_tag;
@@ -54,10 +57,10 @@ Message<google_firestore_v1_Value> EncodeServerTimestamp(
   field->value.timestamp_value.seconds = local_write_time.seconds();
   field->value.timestamp_value.nanos = local_write_time.nanoseconds();
 
-  if (previous_value) {
+  if (actual_previous_value) {
     ++field;
     field->key = nanopb::MakeBytesArray(kPreviousValueKey);
-    field->value = *DeepClone(*previous_value).release();
+    field->value = *DeepClone(*actual_previous_value).release();
   }
 
   return result;

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>
 #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;
@@ -1684,6 +1690,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