Skip to content

Commit a57a2b5

Browse files
authored
Fix StackOverflowError by deeply nested server-timestamps. (#7139)
* Fix StackOverflowError by deeply nested server-timestamps. * Create grumpy-bees-shake.md * Make fewer mutations to avoid timing out.
1 parent 5e5c412 commit a57a2b5

File tree

3 files changed

+53
-0
lines changed

3 files changed

+53
-0
lines changed

.changeset/grumpy-bees-shake.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@firebase/firestore": patch
3+
---
4+
5+
Fixed stack overflow caused by deeply nested server timestamps.

packages/firestore/src/model/server_timestamps.ts

+11
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,17 @@ export function serverTimestamp(
7373
}
7474
};
7575

76+
// We should avoid storing deeply nested server timestamp map values
77+
// because we never use the intermediate "previous values".
78+
// For example:
79+
// previous: 42L, add: t1, result: t1 -> 42L
80+
// previous: t1, add: t2, result: t2 -> 42L (NOT t2 -> t1 -> 42L)
81+
// previous: t2, add: t3, result: t3 -> 42L (NOT t3 -> t2 -> t1 -> 42L)
82+
// `getPreviousValue` recursively traverses server timestamps to find the
83+
// least recent Value.
84+
if (previousValue && isServerTimestamp(previousValue)) {
85+
previousValue = getPreviousValue(previousValue);
86+
}
7687
if (previousValue) {
7788
mapValue.fields![PREVIOUS_VALUE_KEY] = previousValue;
7889
}

packages/firestore/test/unit/local/local_store.test.ts

+37
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,22 @@ import {
6161
DocumentMap
6262
} from '../../../src/model/collections';
6363
import { Document } from '../../../src/model/document';
64+
import { FieldMask } from '../../../src/model/field_mask';
6465
import {
66+
FieldTransform,
6567
Mutation,
6668
MutationResult,
6769
MutationType,
70+
PatchMutation,
6871
Precondition
6972
} from '../../../src/model/mutation';
7073
import {
7174
MutationBatch,
7275
MutationBatchResult
7376
} from '../../../src/model/mutation_batch';
77+
import { ObjectValue } from '../../../src/model/object_value';
78+
import { serverTimestamp } from '../../../src/model/server_timestamps';
79+
import { ServerTimestampTransform } from '../../../src/model/transform_operation';
7480
import { BundleMetadata as ProtoBundleMetadata } from '../../../src/protos/firestore_bundle_proto';
7581
import * as api from '../../../src/protos/firestore_proto_api';
7682
import { RemoteEvent } from '../../../src/remote/remote_event';
@@ -94,6 +100,7 @@ import {
94100
docUpdateRemoteEvent,
95101
existenceFilterEvent,
96102
expectEqual,
103+
field,
97104
filter,
98105
key,
99106
localViewChanges,
@@ -2268,6 +2275,36 @@ function genericLocalStoreTests(
22682275
.finish();
22692276
});
22702277

2278+
it('deeply nested server timestamps do not cause stack overflow', async () => {
2279+
const timestamp = Timestamp.now();
2280+
const initialServerTimestamp = serverTimestamp(timestamp, null);
2281+
const value: ObjectValue = ObjectValue.empty();
2282+
value.set(
2283+
field('timestamp'),
2284+
serverTimestamp(timestamp, initialServerTimestamp)
2285+
);
2286+
2287+
const mutations: PatchMutation[] = [];
2288+
for (let i = 0; i < 100; ++i) {
2289+
mutations.push(
2290+
new PatchMutation(
2291+
key('foo/bar'),
2292+
value,
2293+
new FieldMask([field('timestamp')]),
2294+
Precondition.none(),
2295+
[
2296+
new FieldTransform(
2297+
field('timestamp'),
2298+
new ServerTimestampTransform()
2299+
)
2300+
]
2301+
)
2302+
);
2303+
}
2304+
await expect(expectLocalStore().afterMutations(mutations).finish()).to.not
2305+
.be.eventually.rejected;
2306+
});
2307+
22712308
it('uses target mapping to execute queries', () => {
22722309
if (gcIsEager) {
22732310
return;

0 commit comments

Comments
 (0)