Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: sort strings in UTF-8 encoded byte order #8691

Merged
merged 14 commits into from
Feb 5, 2025
6 changes: 6 additions & 0 deletions .changeset/spotty-trainers-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@firebase/firestore': patch
'firebase': patch
---

Fixed a server and sdk mismatch in unicode string sorting.
Original file line number Diff line number Diff line change
@@ -655,5 +655,9 @@ export function dbKeyComparator(l: DocumentKey, r: DocumentKey): number {
return cmp;
}

// TODO(b/329441702): Document IDs should be sorted by UTF-8 encoded byte
// order, but IndexedDB sorts strings lexicographically. Document ID
// comparison here still relies on primitive comparison to avoid mismatches
// observed in snapshot listeners with Unicode characters in documentIds
return primitiveComparator(left[left.length - 1], right[right.length - 1]);
}
11 changes: 3 additions & 8 deletions packages/firestore/src/model/path.ts
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ import { Integer } from '@firebase/webchannel-wrapper/bloom-blob';

import { debugAssert, fail } from '../util/assert';
import { Code, FirestoreError } from '../util/error';
import { primitiveComparator, compareUtf8Strings } from '../util/misc';

export const DOCUMENT_KEY_NAME = '__name__';

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

private static compareSegments(lhs: string, rhs: string): number {
@@ -201,13 +202,7 @@ abstract class BasePath<B extends BasePath<B>> {
);
} else {
// both non-numeric
if (lhs < rhs) {
return -1;
}
if (lhs > rhs) {
return 1;
}
return 0;
return compareUtf8Strings(lhs, rhs);
}
}

10 changes: 7 additions & 3 deletions packages/firestore/src/model/values.ts
Original file line number Diff line number Diff line change
@@ -25,7 +25,11 @@ import {
Value
} from '../protos/firestore_proto_api';
import { fail } from '../util/assert';
import { arrayEquals, primitiveComparator } from '../util/misc';
import {
arrayEquals,
primitiveComparator,
compareUtf8Strings
} from '../util/misc';
import { forEach, objectSize } from '../util/obj';
import { isNegativeZero } from '../util/types';

@@ -251,7 +255,7 @@ export function valueCompare(left: Value, right: Value): number {
getLocalWriteTime(right)
);
case TypeOrder.StringValue:
return primitiveComparator(left.stringValue!, right.stringValue!);
return compareUtf8Strings(left.stringValue!, right.stringValue!);
case TypeOrder.BlobValue:
return compareBlobs(left.bytesValue!, right.bytesValue!);
case TypeOrder.RefValue:
@@ -400,7 +404,7 @@ function compareMaps(left: MapValue, right: MapValue): number {
rightKeys.sort();

for (let i = 0; i < leftKeys.length && i < rightKeys.length; ++i) {
const keyCompare = primitiveComparator(leftKeys[i], rightKeys[i]);
const keyCompare = compareUtf8Strings(leftKeys[i], rightKeys[i]);
if (keyCompare !== 0) {
return keyCompare;
}
16 changes: 16 additions & 0 deletions packages/firestore/src/util/misc.ts
Original file line number Diff line number Diff line change
@@ -74,6 +74,22 @@ export interface Equatable<T> {
isEqual(other: T): boolean;
}

/** Compare strings in UTF-8 encoded byte order */
export function compareUtf8Strings(left: string, right: string): number {
// Convert the string to UTF-8 encoded bytes
const encodedLeft = new TextEncoder().encode(left);
const encodedRight = new TextEncoder().encode(right);

for (let i = 0; i < Math.min(encodedLeft.length, encodedRight.length); i++) {
const comparison = primitiveComparator(encodedLeft[i], encodedRight[i]);
if (comparison !== 0) {
return comparison;
}
}

return primitiveComparator(encodedLeft.length, encodedRight.length);
}

export interface Iterable<V> {
forEach: (cb: (v: V) => void) => void;
}
195 changes: 195 additions & 0 deletions packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
@@ -2424,4 +2424,199 @@ apiDescribe('Database', persistence => {
});
});
});

describe('Sort unicode strings', () => {
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];
it('snapshot listener sorts unicode strings the same as server', async () => {
const testDocs = {
'a': { value: 'Łukasiewicz' },
'b': { value: 'Sierpiński' },
'c': { value: '岩澤' },
'd': { value: '🄟' },
'e': { value: 'P' },
'f': { value: '︒' },
'g': { value: '🐵' }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy('value'));

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();

await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
});
});

it('snapshot listener sorts unicode strings in array the same as server', async () => {
const testDocs = {
'a': { value: ['Łukasiewicz'] },
'b': { value: ['Sierpiński'] },
'c': { value: ['岩澤'] },
'd': { value: ['🄟'] },
'e': { value: ['P'] },
'f': { value: ['︒'] },
'g': { value: ['🐵'] }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy('value'));

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();

await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
});
});

it('snapshot listener sorts unicode strings in map the same as server', async () => {
const testDocs = {
'a': { value: { foo: 'Łukasiewicz' } },
'b': { value: { foo: 'Sierpiński' } },
'c': { value: { foo: '岩澤' } },
'd': { value: { foo: '🄟' } },
'e': { value: { foo: 'P' } },
'f': { value: { foo: '︒' } },
'g': { value: { foo: '🐵' } }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy('value'));

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();

await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
});
});

it('snapshot listener sorts unicode strings in map key the same as server', async () => {
const testDocs = {
'a': { value: { 'Łukasiewicz': true } },
'b': { value: { 'Sierpiński': true } },
'c': { value: { '岩澤': true } },
'd': { value: { '🄟': true } },
'e': { value: { 'P': true } },
'f': { value: { '︒': true } },
'g': { value: { '🐵': true } }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy('value'));

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();

await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
});
});

it('snapshot listener sorts unicode strings in document key the same as server', async () => {
const testDocs = {
'Łukasiewicz': { value: true },
'Sierpiński': { value: true },
'岩澤': { value: true },
'🄟': { value: true },
'P': { value: true },
'︒': { value: true },
'🐵': { value: true }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy(documentId()));

const getSnapshot = await getDocsFromServer(orderedQuery);
const expectedDocs = [
'Sierpiński',
'Łukasiewicz',
'岩澤',
'︒',
'P',
'🄟',
'🐵'
];
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();

await checkOnlineAndOfflineResultsMatch(orderedQuery, ...expectedDocs);
});
});

// eslint-disable-next-line no-restricted-properties
(persistence.storage === 'indexeddb' ? it.skip : it)(
'snapshot listener sorts unicode strings in document key the same as server with persistence',
async () => {
const testDocs = {
'Łukasiewicz': { value: true },
'Sierpiński': { value: true },
'岩澤': { value: true },
'🄟': { value: true },
'P': { value: true },
'︒': { value: true },
'🐵': { value: true }
};

return withTestCollection(
persistence,
testDocs,
async collectionRef => {
const orderedQuery = query(collectionRef, orderBy('value'));

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal([
'Sierpiński',
'Łukasiewicz',
'岩澤',
'︒',
'P',
'🄟',
'🐵'
]);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
// TODO: IndexedDB sorts string lexicographically, and misses the document with ID '🄟','🐵'
expect(toIds(watchSnapshot)).to.deep.equal(toIds(getSnapshot));

unsubscribe();
}
);
}
);
});
});