Skip to content

Commit 721e5a7

Browse files
milaGGLmilaGGL
authored
FIX: sort strings in UTF-8 encoded byte order (#8691)
1 parent 9d88e3a commit 721e5a7

File tree

6 files changed

+231
-11
lines changed

6 files changed

+231
-11
lines changed

‎.changeset/spotty-trainers-lay.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@firebase/firestore': patch
3+
'firebase': patch
4+
---
5+
6+
Fixed a server and sdk mismatch in unicode string sorting.

‎packages/firestore/src/local/indexeddb_remote_document_cache.ts

+4
Original file line numberDiff line numberDiff line change
@@ -655,5 +655,9 @@ export function dbKeyComparator(l: DocumentKey, r: DocumentKey): number {
655655
return cmp;
656656
}
657657

658+
// TODO(b/329441702): Document IDs should be sorted by UTF-8 encoded byte
659+
// order, but IndexedDB sorts strings lexicographically. Document ID
660+
// comparison here still relies on primitive comparison to avoid mismatches
661+
// observed in snapshot listeners with Unicode characters in documentIds
658662
return primitiveComparator(left[left.length - 1], right[right.length - 1]);
659663
}

‎packages/firestore/src/model/path.ts

+3-8
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';
1919

2020
import { debugAssert, fail } from '../util/assert';
2121
import { Code, FirestoreError } from '../util/error';
22+
import { primitiveComparator, compareUtf8Strings } from '../util/misc';
2223

2324
export const DOCUMENT_KEY_NAME = '__name__';
2425

@@ -181,7 +182,7 @@ abstract class BasePath<B extends BasePath<B>> {
181182
return comparison;
182183
}
183184
}
184-
return Math.sign(p1.length - p2.length);
185+
return primitiveComparator(p1.length, p2.length);
185186
}
186187

187188
private static compareSegments(lhs: string, rhs: string): number {
@@ -201,13 +202,7 @@ abstract class BasePath<B extends BasePath<B>> {
201202
);
202203
} else {
203204
// both non-numeric
204-
if (lhs < rhs) {
205-
return -1;
206-
}
207-
if (lhs > rhs) {
208-
return 1;
209-
}
210-
return 0;
205+
return compareUtf8Strings(lhs, rhs);
211206
}
212207
}
213208

‎packages/firestore/src/model/values.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ import {
2525
Value
2626
} from '../protos/firestore_proto_api';
2727
import { fail } from '../util/assert';
28-
import { arrayEquals, primitiveComparator } from '../util/misc';
28+
import {
29+
arrayEquals,
30+
primitiveComparator,
31+
compareUtf8Strings
32+
} from '../util/misc';
2933
import { forEach, objectSize } from '../util/obj';
3034
import { isNegativeZero } from '../util/types';
3135

@@ -251,7 +255,7 @@ export function valueCompare(left: Value, right: Value): number {
251255
getLocalWriteTime(right)
252256
);
253257
case TypeOrder.StringValue:
254-
return primitiveComparator(left.stringValue!, right.stringValue!);
258+
return compareUtf8Strings(left.stringValue!, right.stringValue!);
255259
case TypeOrder.BlobValue:
256260
return compareBlobs(left.bytesValue!, right.bytesValue!);
257261
case TypeOrder.RefValue:
@@ -400,7 +404,7 @@ function compareMaps(left: MapValue, right: MapValue): number {
400404
rightKeys.sort();
401405

402406
for (let i = 0; i < leftKeys.length && i < rightKeys.length; ++i) {
403-
const keyCompare = primitiveComparator(leftKeys[i], rightKeys[i]);
407+
const keyCompare = compareUtf8Strings(leftKeys[i], rightKeys[i]);
404408
if (keyCompare !== 0) {
405409
return keyCompare;
406410
}

‎packages/firestore/src/util/misc.ts

+16
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,22 @@ export interface Equatable<T> {
7474
isEqual(other: T): boolean;
7575
}
7676

77+
/** Compare strings in UTF-8 encoded byte order */
78+
export function compareUtf8Strings(left: string, right: string): number {
79+
// Convert the string to UTF-8 encoded bytes
80+
const encodedLeft = new TextEncoder().encode(left);
81+
const encodedRight = new TextEncoder().encode(right);
82+
83+
for (let i = 0; i < Math.min(encodedLeft.length, encodedRight.length); i++) {
84+
const comparison = primitiveComparator(encodedLeft[i], encodedRight[i]);
85+
if (comparison !== 0) {
86+
return comparison;
87+
}
88+
}
89+
90+
return primitiveComparator(encodedLeft.length, encodedRight.length);
91+
}
92+
7793
export interface Iterable<V> {
7894
forEach: (cb: (v: V) => void) => void;
7995
}

‎packages/firestore/test/integration/api/database.test.ts

+195
Original file line numberDiff line numberDiff line change
@@ -2424,4 +2424,199 @@ apiDescribe('Database', persistence => {
24242424
});
24252425
});
24262426
});
2427+
2428+
describe('Sort unicode strings', () => {
2429+
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
2430+
it('snapshot listener sorts unicode strings the same as server', async () => {
2431+
const testDocs = {
2432+
'a': { value: 'Łukasiewicz' },
2433+
'b': { value: 'Sierpiński' },
2434+
'c': { value: '岩澤' },
2435+
'd': { value: '🄟' },
2436+
'e': { value: 'Ｐ' },
2437+
'f': { value: '︒' },
2438+
'g': { value: '🐵' }
2439+
};
2440+
2441+
return withTestCollection(persistence, testDocs, async collectionRef => {
2442+
const orderedQuery = query(collectionRef, orderBy('value'));
2443+
2444+
const getSnapshot = await getDocsFromServer(orderedQuery);
2445+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2446+
2447+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2448+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2449+
const watchSnapshot = await storeEvent.awaitEvent();
2450+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2451+
2452+
unsubscribe();
2453+
2454+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
2455+
});
2456+
});
2457+
2458+
it('snapshot listener sorts unicode strings in array the same as server', async () => {
2459+
const testDocs = {
2460+
'a': { value: ['Łukasiewicz'] },
2461+
'b': { value: ['Sierpiński'] },
2462+
'c': { value: ['岩澤'] },
2463+
'd': { value: ['🄟'] },
2464+
'e': { value: ['Ｐ'] },
2465+
'f': { value: ['︒'] },
2466+
'g': { value: ['🐵'] }
2467+
};
2468+
2469+
return withTestCollection(persistence, testDocs, async collectionRef => {
2470+
const orderedQuery = query(collectionRef, orderBy('value'));
2471+
2472+
const getSnapshot = await getDocsFromServer(orderedQuery);
2473+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2474+
2475+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2476+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2477+
const watchSnapshot = await storeEvent.awaitEvent();
2478+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2479+
2480+
unsubscribe();
2481+
2482+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
2483+
});
2484+
});
2485+
2486+
it('snapshot listener sorts unicode strings in map the same as server', async () => {
2487+
const testDocs = {
2488+
'a': { value: { foo: 'Łukasiewicz' } },
2489+
'b': { value: { foo: 'Sierpiński' } },
2490+
'c': { value: { foo: '岩澤' } },
2491+
'd': { value: { foo: '🄟' } },
2492+
'e': { value: { foo: 'Ｐ' } },
2493+
'f': { value: { foo: '︒' } },
2494+
'g': { value: { foo: '🐵' } }
2495+
};
2496+
2497+
return withTestCollection(persistence, testDocs, async collectionRef => {
2498+
const orderedQuery = query(collectionRef, orderBy('value'));
2499+
2500+
const getSnapshot = await getDocsFromServer(orderedQuery);
2501+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2502+
2503+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2504+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2505+
const watchSnapshot = await storeEvent.awaitEvent();
2506+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2507+
2508+
unsubscribe();
2509+
2510+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
2511+
});
2512+
});
2513+
2514+
it('snapshot listener sorts unicode strings in map key the same as server', async () => {
2515+
const testDocs = {
2516+
'a': { value: { 'Łukasiewicz': true } },
2517+
'b': { value: { 'Sierpiński': true } },
2518+
'c': { value: { '岩澤': true } },
2519+
'd': { value: { '🄟': true } },
2520+
'e': { value: { 'Ｐ': true } },
2521+
'f': { value: { '︒': true } },
2522+
'g': { value: { '🐵': true } }
2523+
};
2524+
2525+
return withTestCollection(persistence, testDocs, async collectionRef => {
2526+
const orderedQuery = query(collectionRef, orderBy('value'));
2527+
2528+
const getSnapshot = await getDocsFromServer(orderedQuery);
2529+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2530+
2531+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2532+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2533+
const watchSnapshot = await storeEvent.awaitEvent();
2534+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2535+
2536+
unsubscribe();
2537+
2538+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
2539+
});
2540+
});
2541+
2542+
it('snapshot listener sorts unicode strings in document key the same as server', async () => {
2543+
const testDocs = {
2544+
'Łukasiewicz': { value: true },
2545+
'Sierpiński': { value: true },
2546+
'岩澤': { value: true },
2547+
'🄟': { value: true },
2548+
'Ｐ': { value: true },
2549+
'︒': { value: true },
2550+
'🐵': { value: true }
2551+
};
2552+
2553+
return withTestCollection(persistence, testDocs, async collectionRef => {
2554+
const orderedQuery = query(collectionRef, orderBy(documentId()));
2555+
2556+
const getSnapshot = await getDocsFromServer(orderedQuery);
2557+
const expectedDocs = [
2558+
'Sierpiński',
2559+
'Łukasiewicz',
2560+
'岩澤',
2561+
'︒',
2562+
'Ｐ',
2563+
'🄟',
2564+
'🐵'
2565+
];
2566+
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);
2567+
2568+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2569+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2570+
const watchSnapshot = await storeEvent.awaitEvent();
2571+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2572+
2573+
unsubscribe();
2574+
2575+
await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
2576+
});
2577+
});
2578+
2579+
// eslint-disable-next-line no-restricted-properties
2580+
(persistence.storage === 'indexeddb' ? it.skip : it)(
2581+
'snapshot listener sorts unicode strings in document key the same as server with persistence',
2582+
async () => {
2583+
const testDocs = {
2584+
'Łukasiewicz': { value: true },
2585+
'Sierpiński': { value: true },
2586+
'岩澤': { value: true },
2587+
'🄟': { value: true },
2588+
'Ｐ': { value: true },
2589+
'︒': { value: true },
2590+
'🐵': { value: true }
2591+
};
2592+
2593+
return withTestCollection(
2594+
persistence,
2595+
testDocs,
2596+
async collectionRef => {
2597+
const orderedQuery = query(collectionRef, orderBy('value'));
2598+
2599+
const getSnapshot = await getDocsFromServer(orderedQuery);
2600+
expect(toIds(getSnapshot)).to.deep.equal([
2601+
'Sierpiński',
2602+
'Łukasiewicz',
2603+
'岩澤',
2604+
'︒',
2605+
'Ｐ',
2606+
'🄟',
2607+
'🐵'
2608+
]);
2609+
2610+
const storeEvent = new EventsAccumulator<QuerySnapshot>();
2611+
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
2612+
const watchSnapshot = await storeEvent.awaitEvent();
2613+
// TODO: IndexedDB sorts string lexicographically, and misses the document with ID '🄟','🐵'
2614+
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));
2615+
2616+
unsubscribe();
2617+
}
2618+
);
2619+
}
2620+
);
2621+
});
24272622
});

0 commit comments

Comments
 (0)