From ec940b4ce56c6075943c83e87befc5621f279bc6 Mon Sep 17 00:00:00 2001 From: "takahiro.kurebayashi" Date: Fri, 24 Feb 2023 20:17:37 +0900 Subject: [PATCH 1/4] Adds a new test of StackOverflowError caused by deeply nested ServerTimestamps See: https://github.com/firebase/firebase-android-sdk/issues/4702 --- .../firestore/local/SQLiteLocalStoreTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java index 792c5e8f0ce..1d8592cf0a9 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java @@ -32,12 +32,31 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +import com.google.firebase.Timestamp; import com.google.firebase.firestore.FieldValue; import com.google.firebase.firestore.core.Query; +import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.FieldIndex; +import com.google.firebase.firestore.model.FieldPath; +import com.google.firebase.firestore.model.ObjectValue; +import com.google.firebase.firestore.model.ServerTimestamps; +import com.google.firebase.firestore.model.mutation.FieldMask; +import com.google.firebase.firestore.model.mutation.FieldTransform; +import com.google.firebase.firestore.model.mutation.PatchMutation; +import com.google.firebase.firestore.model.mutation.Precondition; +import com.google.firebase.firestore.model.mutation.ServerTimestampOperation; +import com.google.firestore.v1.Value; + +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -290,4 +309,34 @@ public void testIndexesServerTimestamps() { assertOverlayTypes(keyMap("coll/a", CountingQueryEngine.OverlayType.Set)); assertQueryReturned("coll/a"); } + + @Test + public void testDeeplyNestedServerTimestamps() { + Timestamp timestamp = Timestamp.now(); + Value initialServerTimestamp = ServerTimestamps.valueOf(timestamp, null); + Map fields = new HashMap() {{ + put("timestamp", ServerTimestamps.valueOf(timestamp, initialServerTimestamp)); + }}; + FieldPath path = FieldPath.fromSingleSegment("timestamp"); + FieldMask mask = FieldMask.fromSet(new HashSet() {{ add(path); }}); + FieldTransform fieldTransform = new FieldTransform(path, ServerTimestampOperation.getInstance()); + List fieldTransforms = new ArrayList() {{ add(fieldTransform); }}; + AtomicReference error = new AtomicReference<>(); + Thread thread = new Thread(Thread.currentThread().getThreadGroup(), () -> { + try { + for (int i = 0; i < 1000; ++i) { + writeMutation(new PatchMutation(DocumentKey.fromPathString("some/object/for/test"), ObjectValue.fromMap(fields), mask, Precondition.NONE, fieldTransforms)); + } + } catch (Throwable e) { + error.set(e); + } + }, "test", 1024 * 1024); + try { + thread.start(); + thread.join(); + } catch (InterruptedException e) { + throw new AssertionError(e); + } + assertThat(error.get()).isNull(); + } } From 8b98d892dc5a60ae1c93d7604b24102866c6daef Mon Sep 17 00:00:00 2001 From: "takahiro.kurebayashi" Date: Fri, 24 Feb 2023 20:20:42 +0900 Subject: [PATCH 2/4] Keeps just only oldest previous value. Omit history of server-timestamps to fix StackOverflowError. See: https://github.com/firebase/firebase-android-sdk/issues/4702 --- .../google/firebase/firestore/model/ServerTimestamps.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ServerTimestamps.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ServerTimestamps.java index 7ae19eadcca..ef7b547fd12 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ServerTimestamps.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/ServerTimestamps.java @@ -62,8 +62,9 @@ public static Value valueOf(Timestamp localWriteTime, @Nullable Value previousVa .putFields(TYPE_KEY, encodedType) .putFields(LOCAL_WRITE_TIME_KEY, encodeWriteTime); - if (previousValue != null) { - mapRepresentation.putFields(PREVIOUS_VALUE_KEY, previousValue); + Value actualPreviousValue = previousValue == null ? null : getPreviousValue(previousValue); + if (actualPreviousValue != null) { + mapRepresentation.putFields(PREVIOUS_VALUE_KEY, actualPreviousValue); } return Value.newBuilder().setMapValue(mapRepresentation).build(); From 1fcea791e6412dca005d3ce2a211a4e5e000f14a Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 6 Mar 2023 16:04:29 -0800 Subject: [PATCH 3/4] Fix formatting. --- .../firestore/local/SQLiteLocalStoreTest.java | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java index 1d8592cf0a9..cf544b133bc 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java @@ -46,7 +46,6 @@ import com.google.firebase.firestore.model.mutation.Precondition; import com.google.firebase.firestore.model.mutation.ServerTimestampOperation; import com.google.firestore.v1.Value; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -56,7 +55,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; - import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -314,23 +312,49 @@ public void testIndexesServerTimestamps() { public void testDeeplyNestedServerTimestamps() { Timestamp timestamp = Timestamp.now(); Value initialServerTimestamp = ServerTimestamps.valueOf(timestamp, null); - Map fields = new HashMap() {{ - put("timestamp", ServerTimestamps.valueOf(timestamp, initialServerTimestamp)); - }}; + Map fields = + new HashMap() { + { + put("timestamp", ServerTimestamps.valueOf(timestamp, initialServerTimestamp)); + } + }; FieldPath path = FieldPath.fromSingleSegment("timestamp"); - FieldMask mask = FieldMask.fromSet(new HashSet() {{ add(path); }}); - FieldTransform fieldTransform = new FieldTransform(path, ServerTimestampOperation.getInstance()); - List fieldTransforms = new ArrayList() {{ add(fieldTransform); }}; + FieldMask mask = + FieldMask.fromSet( + new HashSet() { + { + add(path); + } + }); + FieldTransform fieldTransform = + new FieldTransform(path, ServerTimestampOperation.getInstance()); + List fieldTransforms = + new ArrayList() { + { + add(fieldTransform); + } + }; AtomicReference error = new AtomicReference<>(); - Thread thread = new Thread(Thread.currentThread().getThreadGroup(), () -> { - try { - for (int i = 0; i < 1000; ++i) { - writeMutation(new PatchMutation(DocumentKey.fromPathString("some/object/for/test"), ObjectValue.fromMap(fields), mask, Precondition.NONE, fieldTransforms)); - } - } catch (Throwable e) { - error.set(e); - } - }, "test", 1024 * 1024); + Thread thread = + new Thread( + Thread.currentThread().getThreadGroup(), + () -> { + try { + for (int i = 0; i < 1000; ++i) { + writeMutation( + new PatchMutation( + DocumentKey.fromPathString("some/object/for/test"), + ObjectValue.fromMap(fields), + mask, + Precondition.NONE, + fieldTransforms)); + } + } catch (Throwable e) { + error.set(e); + } + }, + "test", + 1024 * 1024); try { thread.start(); thread.join(); From 9e1e82a6478f4084a01da7925f5174d15b99d51a Mon Sep 17 00:00:00 2001 From: Ehsan Nasiri Date: Mon, 6 Mar 2023 16:14:00 -0800 Subject: [PATCH 4/4] Add changelog. --- firebase-firestore/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index e88ab8baa82..b4248aeb5c0 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-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 (#4702). # 24.4.4 * [changed] Relaxed certain query validations performed by the SDK (#4231).