diff --git a/.changeset/grumpy-bees-shake.md b/.changeset/grumpy-bees-shake.md new file mode 100644 index 00000000000..bddd707762b --- /dev/null +++ b/.changeset/grumpy-bees-shake.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Fixed stack overflow caused by deeply nested server timestamps. diff --git a/packages/firestore/src/model/server_timestamps.ts b/packages/firestore/src/model/server_timestamps.ts index b2e59c105b9..29fe5570f1d 100644 --- a/packages/firestore/src/model/server_timestamps.ts +++ b/packages/firestore/src/model/server_timestamps.ts @@ -73,6 +73,17 @@ export function serverTimestamp( } }; + // 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 (previousValue && isServerTimestamp(previousValue)) { + previousValue = getPreviousValue(previousValue); + } if (previousValue) { mapValue.fields![PREVIOUS_VALUE_KEY] = previousValue; } diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 204f714a486..431cc473189 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -61,16 +61,22 @@ import { DocumentMap } from '../../../src/model/collections'; import { Document } from '../../../src/model/document'; +import { FieldMask } from '../../../src/model/field_mask'; import { + FieldTransform, Mutation, MutationResult, MutationType, + PatchMutation, Precondition } from '../../../src/model/mutation'; import { MutationBatch, MutationBatchResult } from '../../../src/model/mutation_batch'; +import { ObjectValue } from '../../../src/model/object_value'; +import { serverTimestamp } from '../../../src/model/server_timestamps'; +import { ServerTimestampTransform } from '../../../src/model/transform_operation'; import { BundleMetadata as ProtoBundleMetadata } from '../../../src/protos/firestore_bundle_proto'; import * as api from '../../../src/protos/firestore_proto_api'; import { RemoteEvent } from '../../../src/remote/remote_event'; @@ -94,6 +100,7 @@ import { docUpdateRemoteEvent, existenceFilterEvent, expectEqual, + field, filter, key, localViewChanges, @@ -2268,6 +2275,36 @@ function genericLocalStoreTests( .finish(); }); + it('deeply nested server timestamps do not cause stack overflow', async () => { + const timestamp = Timestamp.now(); + const initialServerTimestamp = serverTimestamp(timestamp, null); + const value: ObjectValue = ObjectValue.empty(); + value.set( + field('timestamp'), + serverTimestamp(timestamp, initialServerTimestamp) + ); + + const mutations: PatchMutation[] = []; + for (let i = 0; i < 100; ++i) { + mutations.push( + new PatchMutation( + key('foo/bar'), + value, + new FieldMask([field('timestamp')]), + Precondition.none(), + [ + new FieldTransform( + field('timestamp'), + new ServerTimestampTransform() + ) + ] + ) + ); + } + await expect(expectLocalStore().afterMutations(mutations).finish()).to.not + .be.eventually.rejected; + }); + it('uses target mapping to execute queries', () => { if (gcIsEager) { return;