Skip to content

Commit feb2c9d

Browse files
milaGGLmilaGGL
authored
Fix: sort strings in UTF-8 encoded byte order with lazy encoding (#8787)
1 parent 51465ce commit feb2c9d

File tree

7 files changed

+546
-12
lines changed

7 files changed

+546
-12
lines changed

‎.changeset/large-pants-hide.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@firebase/firestore': patch
3+
'firebase': patch
4+
---
5+
6+
Use lazy encoding in UTF-8 encoded byte comparison for strings.

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

Lines changed: 4 additions & 0 deletions
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‎

Lines changed: 3 additions & 8 deletions
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 { compareUtf8Strings, primitiveComparator } 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‎

Lines changed: 7 additions & 3 deletions
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+
compareUtf8Strings,
31+
primitiveComparator
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‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717

1818
import { randomBytes } from '../platform/random_bytes';
19+
import { newTextEncoder } from '../platform/text_serializer';
1920

2021
import { debugAssert } from './assert';
2122

@@ -74,6 +75,67 @@ export interface Equatable<T> {
7475
isEqual(other: T): boolean;
7576
}
7677

78+
/** Compare strings in UTF-8 encoded byte order */
79+
export function compareUtf8Strings(left: string, right: string): number {
80+
let i = 0;
81+
while (i < left.length && i < right.length) {
82+
const leftCodePoint = left.codePointAt(i)!;
83+
const rightCodePoint = right.codePointAt(i)!;
84+
85+
if (leftCodePoint !== rightCodePoint) {
86+
if (leftCodePoint < 128 && rightCodePoint < 128) {
87+
// ASCII comparison
88+
return primitiveComparator(leftCodePoint, rightCodePoint);
89+
} else {
90+
// Lazy instantiate TextEncoder
91+
const encoder = newTextEncoder();
92+
93+
// UTF-8 encode the character at index i for byte comparison.
94+
const leftBytes = encoder.encode(getUtf8SafeSubstring(left, i));
95+
const rightBytes = encoder.encode(getUtf8SafeSubstring(right, i));
96+
97+
const comp = compareByteArrays(leftBytes, rightBytes);
98+
if (comp !== 0) {
99+
return comp;
100+
} else {
101+
// EXTREMELY RARE CASE: Code points differ, but their UTF-8 byte
102+
// representations are identical. This can happen with malformed input
103+
// (invalid surrogate pairs). The backend also actively prevents invalid
104+
// surrogates as INVALID_ARGUMENT errors, so we almost never receive
105+
// invalid strings from backend.
106+
// Fallback to code point comparison for graceful handling.
107+
return primitiveComparator(leftCodePoint, rightCodePoint);
108+
}
109+
}
110+
}
111+
// Increment by 2 for surrogate pairs, 1 otherwise
112+
i += leftCodePoint > 0xffff ? 2 : 1;
113+
}
114+
115+
// Compare lengths if all characters are equal
116+
return primitiveComparator(left.length, right.length);
117+
}
118+
119+
function getUtf8SafeSubstring(str: string, index: number): string {
120+
const firstCodePoint = str.codePointAt(index)!;
121+
if (firstCodePoint > 0xffff) {
122+
// It's a surrogate pair, return the whole pair
123+
return str.substring(index, index + 2);
124+
} else {
125+
// It's a single code point, return it
126+
return str.substring(index, index + 1);
127+
}
128+
}
129+
130+
function compareByteArrays(left: Uint8Array, right: Uint8Array): number {
131+
for (let i = 0; i < left.length && i < right.length; ++i) {
132+
if (left[i] !== right[i]) {
133+
return primitiveComparator(left[i], right[i]);
134+
}
135+
}
136+
return primitiveComparator(left.length, right.length);
137+
}
138+
77139
export interface Iterable<V> {
78140
forEach: (cb: (v: V) => void) => void;
79141
}

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

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

0 commit comments

Comments
 (0)