fix: list every file when the account exceeds maxObjectsInGet #1069

This commit is contained in:
Linus Rath
2026-09-22 00:17:44 +02:00
parent 106bc07f04
commit 9893b0a8ad
3 changed files with 233 additions and 21 deletions
+8 -4
View File
@@ -2212,14 +2212,18 @@ async function buildMinimalOdf(type: string): Promise<Buffer | null> {
function handleFileNodeGet(args: MethodArgs, callId: string): MethodResult {
const ids = args.ids as string[] | null | undefined;
const list = ids == null ? [...fileNodes] : fileNodes.filter((n) => ids.includes(n.id));
// Like Stalwart, ids:null stops at maxObjectsInGet without saying so (#1069).
const list = ids == null ? fileNodes.slice(0, MAX_OBJECTS_IN_GET) : fileNodes.filter((n) => ids.includes(n.id));
const notFound = ids == null ? [] : ids.filter((id) => !fileNodes.some((n) => n.id === id));
return ['FileNode/get', { accountId: ACCOUNT_ID, state: nextState(), list, notFound }, callId];
}
function handleFileNodeQuery(_args: MethodArgs, callId: string): MethodResult {
const ids = fileNodes.filter((n) => n.blobId !== null).map((n) => n.id);
return ['FileNode/query', { accountId: ACCOUNT_ID, queryState: nextState(), ids, total: ids.length, position: 0 }, callId];
function handleFileNodeQuery(args: MethodArgs, callId: string): MethodResult {
const matching = fileNodes.filter((n) => n.blobId !== null).map((n) => n.id);
const position = typeof args.position === 'number' ? args.position : 0;
const limit = typeof args.limit === 'number' ? args.limit : matching.length;
const ids = matching.slice(position, position + limit);
return ['FileNode/query', { accountId: ACCOUNT_ID, queryState: nextState(), ids, total: matching.length, position }, callId];
}
function handleFileNodeSet(args: MethodArgs, callId: string): MethodResult {
+131
View File
@@ -26,6 +26,137 @@ function mockFetch(response: object) {
const webdavFolder = { id: 'f1', parentId: null, name: 'Spares%20Catalog%20%D8%B9%D8%B1%D8%A8%D9%8A', type: '', blobId: null, size: 0, created: '2026-01-01T00:00:00Z', modified: '2026-01-01T00:00:00Z' };
const jmapFile = { id: 'f2', parentId: 'f1', name: '100% done.txt', type: 'text/plain', blobId: 'b2', size: 3, created: '2026-01-01T00:00:00Z', modified: '2026-01-01T00:00:00Z' };
type MethodCall = [string, Record<string, unknown>, string];
interface FakeNode { id: string; parentId: string | null; name: string; blobId: string | null }
const file = (id: string, parentId: string | null = null): FakeNode => ({ id, parentId, name: `${id}.txt`, blobId: `blob-${id}` });
const folder = (id: string, parentId: string | null = null): FakeNode => ({ id, parentId, name: id, blobId: null });
/**
* Answers like Stalwart: `FileNode/get { ids: null }` stops at maxObjectsInGet
* without saying so, an over-long id list is refused, and FileNode/query clamps
* `limit` to the server's own maximum.
*/
function fakeFilesServer(nodes: FakeNode[], opts: {
maxObjectsInGet: number;
queryMaxResults?: number;
// Stalwart before 0.16.6 returns leaf files only.
queryOmitsFolders?: boolean;
queryFails?: boolean;
}) {
const sent: MethodCall[][] = [];
vi.spyOn(globalThis, 'fetch').mockImplementation((async (_url: string, init: RequestInit) => {
const { methodCalls } = JSON.parse(init.body as string) as { methodCalls: MethodCall[] };
sent.push(methodCalls);
const methodResponses = methodCalls.map(([method, args, callId]) => {
if (method === 'FileNode/get') {
const ids = args.ids as string[] | null;
if (ids === null) return [method, { list: nodes.slice(0, opts.maxObjectsInGet), notFound: [] }, callId];
if (ids.length > opts.maxObjectsInGet) return ['error', { type: 'requestTooLarge' }, callId];
return [method, { list: nodes.filter(n => ids.includes(n.id)), notFound: ids.filter(id => !nodes.some(n => n.id === id)) }, callId];
}
if (method === 'FileNode/query') {
if (opts.queryFails) return ['error', { type: 'unknownMethod' }, callId];
const matching = opts.queryOmitsFolders ? nodes.filter(n => n.blobId !== null) : nodes;
const position = (args.position as number) ?? 0;
const limit = Math.min((args.limit as number) ?? Infinity, opts.queryMaxResults ?? Infinity);
return [method, { ids: matching.slice(position, position + limit).map(n => n.id), position, total: matching.length }, callId];
}
return ['error', { type: 'unknownMethod' }, callId];
});
const body = JSON.stringify({ methodResponses });
return { ok: true, status: 200, text: () => Promise.resolve(body) } as Response;
}) as never);
return sent;
}
function createLimitedClient(core: Record<string, number>): JMAPClient {
const client = createClient();
Object.assign(client, {
capabilities: { 'urn:ietf:params:jmap:core': core, 'urn:ietf:params:jmap:filenode': {} },
});
return client;
}
const sortedIds = (nodes: { id: string }[]) => nodes.map(n => n.id).sort();
describe('JMAPClient FileNode listing past maxObjectsInGet (#1069)', () => {
beforeEach(() => {
vi.restoreAllMocks();
vi.spyOn(console, 'warn').mockImplementation(() => {});
});
it('stays a single request while the account fits in one /get', async () => {
const client = createLimitedClient({ maxObjectsInGet: 5 });
const sent = fakeFilesServer([folder('d1'), file('a', 'd1'), file('b')], { maxObjectsInGet: 5 });
const nodes = await client.listAllFileNodes();
expect(sortedIds(nodes)).toEqual(['a', 'b', 'd1']);
expect(sent).toHaveLength(1);
expect(sent[0][0][1].ids).toBeNull();
});
it('pages the query and fetches the nodes the first /get cut off', async () => {
const all = [folder('d1'), ...Array.from({ length: 10 }, (_, i) => file(`f${i}`, 'd1')), folder('d2')];
const client = createLimitedClient({ maxObjectsInGet: 3, maxCallsInRequest: 2 });
// The server hands out fewer ids per page than the client asks for.
const sent = fakeFilesServer(all, { maxObjectsInGet: 3, queryMaxResults: 5 });
const nodes = await client.listAllFileNodes();
expect(sortedIds(nodes)).toEqual(sortedIds(all));
const calls = sent.flat();
expect(calls.filter(([m]) => m === 'FileNode/query').map(([, a]) => a.position)).toEqual([0, 5, 10]);
// Only the nine missing nodes are fetched again, never more than the
// server allows per /get or per request.
const idGets = calls.filter(([m, a]) => m === 'FileNode/get' && a.ids !== null);
expect(idGets.flatMap(([, a]) => a.ids as string[]).sort()).toEqual(sortedIds(all.slice(3)));
for (const [, args] of idGets) expect((args.ids as string[]).length).toBeLessThanOrEqual(3);
for (const request of sent) expect(request.length).toBeLessThanOrEqual(2);
});
it('recovers folders when the query returns files only (Stalwart < 0.16.6)', async () => {
// Both folders sit past the first /get, and only `deep` has files in it.
const all = [file('f0'), file('f1'), file('f2', 'deep'), folder('top'), folder('deep', 'top')];
const client = createLimitedClient({ maxObjectsInGet: 2 });
fakeFilesServer(all, { maxObjectsInGet: 2, queryOmitsFolders: true });
const nodes = await client.listAllFileNodes();
expect(sortedIds(nodes)).toEqual(sortedIds(all));
});
it('asks for an unreadable parent once and moves on', async () => {
// A node shared out of a folder the user cannot read.
const all = [file('f0'), file('f1'), file('f2', 'hidden')];
const client = createLimitedClient({ maxObjectsInGet: 2 });
const sent = fakeFilesServer(all, { maxObjectsInGet: 2 });
const nodes = await client.listAllFileNodes();
expect(sortedIds(nodes)).toEqual(['f0', 'f1', 'f2']);
const askedForHidden = sent.flat().filter(([m, a]) => m === 'FileNode/get' && (a.ids as string[] | null)?.includes('hidden'));
expect(askedForHidden).toHaveLength(1);
});
it('keeps the first page when the query is refused', async () => {
const all = [file('f0'), file('f1'), file('f2')];
const client = createLimitedClient({ maxObjectsInGet: 2 });
fakeFilesServer(all, { maxObjectsInGet: 2, queryFails: true });
const nodes = await client.listAllFileNodes();
expect(sortedIds(nodes)).toEqual(['f0', 'f1']);
});
it('lists shared accounts past the limit too', async () => {
const all = [file('f0'), file('f1'), file('f2'), file('f3')];
const client = createLimitedClient({ maxObjectsInGet: 2 });
fakeFilesServer(all, { maxObjectsInGet: 2 });
const nodes = await client.listAllFileNodesAcrossAccounts();
expect(sortedIds(nodes)).toEqual(['f0', 'f1', 'f2', 'f3']);
});
});
describe('JMAPClient FileNode name decoding (#869)', () => {
beforeEach(() => {
vi.restoreAllMocks();
+94 -17
View File
@@ -507,6 +507,11 @@ export function isConcurrentRequestRefusal(status: number, body: string): boolea
/** Base back-off per attempt when the server refuses a request for being one too many in parallel. */
const CONCURRENT_REQUEST_RETRY_DELAYS_MS = [200, 400, 800];
/** Ids asked for per FileNode/query page; Stalwart clamps it to queryMaxResults (5000 by default). */
const FILE_NODE_QUERY_PAGE = 5000;
/** Safety bound on how many FileNode ids one listing pages through. */
const FILE_NODE_MAX_IDS = 200_000;
function computeHasMore(position: number, emailCount: number, total: number, limit: number): boolean {
if (total > 0) return (position + emailCount) < total;
return emailCount === limit;
@@ -7144,8 +7149,8 @@ export class JMAPClient implements IJMAPClient {
}
async listFileNodes(parentId: string | null): Promise<FileNode[]> {
// FileNode/query omits folder nodes in Stalwart (see listAllFileNodes), so
// we enumerate the whole account and filter children client-side.
// FileNode/query omits folder nodes on older Stalwart (see listAllFileNodes),
// so we enumerate the whole account and filter children client-side.
const all = await this.listAllFileNodes();
return all.filter(n => (n.parentId ?? null) === parentId);
}
@@ -7154,17 +7159,34 @@ export class JMAPClient implements IJMAPClient {
* Fetch every FileNode in the account, files AND folders, to build the folder
* hierarchy client-side from parentId links.
*
* IMPORTANT: this uses `FileNode/get` with `ids: null` (return-all), NOT
* `FileNode/query`. Stalwart's FileNode/query only returns leaf files - it
* omits container (folder) nodes entirely - so a query-based listing made
* every folder invisible (the whole account looked like a single root file).
* IMPORTANT: this starts from `FileNode/get` with `ids: null` (return-all),
* NOT `FileNode/query`. Before Stalwart 0.16.6, FileNode/query only returns
* leaf files - it omits container (folder) nodes entirely - so a query-based
* listing made every folder invisible (the whole account looked like a
* single root file).
*/
async listAllFileNodes(): Promise<FileNode[]> {
const accountId = this.getFilesAccountId();
const nodes = await this.fetchAllFileNodes(this.getFilesAccountId());
return nodes.map(withDecodedName);
}
/**
* Every raw FileNode of one account, past the server's /get ceiling.
*
* `FileNode/get { ids: null }` stops at maxObjectsInGet (500 by default) and
* Stalwart gives no sign that the list was cut, so larger accounts silently
* lost files (#1069). A full first page therefore counts as truncated: the
* remaining ids come from paging FileNode/query and are fetched in /get-sized
* batches. Stalwart before 0.16.6 leaves folders out of FileNode/query, so
* parents that are still unknown afterwards are fetched by id.
*/
private async fetchAllFileNodes(accountId: string): Promise<FileNode[]> {
const using = this.fileUsing();
const properties = JMAPClient.FILE_NODE_PROPERTIES;
const response = await this.request(
[["FileNode/get", { accountId, ids: null, properties: JMAPClient.FILE_NODE_PROPERTIES }, "fng0"]],
this.fileUsing(),
[["FileNode/get", { accountId, ids: null, properties }, "fng0"]],
using,
);
const getResult = response.methodResponses?.find(r => r[0] === "FileNode/get" || (r[0] === "error" && r[2] === "fng0"));
@@ -7172,7 +7194,68 @@ export class JMAPClient implements IJMAPClient {
console.error('[Files] FileNode/get error:', getResult?.[1]);
throw new Error(getResult?.[1]?.description || "FileNode list failed");
}
return ((getResult[1].list || []) as FileNode[]).map(withDecodedName);
const firstPage = (getResult[1].list || []) as FileNode[];
const maxObjects = this.getMaxObjectsInGet();
if (firstPage.length < maxObjects) return firstPage;
const known = new Map(firstPage.map(node => [node.id, node]));
// Several /get calls share one request, so a large account costs a few
// round trips rather than one per batch - the tree is re-read on every
// navigation.
const fetchByIds = async (ids: string[]) => {
const calls = batched(ids, maxObjects).map((batch, i): JMAPMethodCall =>
["FileNode/get", { accountId, ids: batch, properties }, `fng${i + 1}`]);
for (const group of batched(calls, this.getMaxCallsInRequest())) {
const batchResponse = await this.request(group, using);
for (const result of batchResponse.methodResponses || []) {
if (result[0] !== "FileNode/get") {
throw new Error(result[1]?.description || "FileNode/get failed");
}
for (const node of (result[1].list || []) as FileNode[]) known.set(node.id, node);
}
}
};
try {
const missing = new Set<string>();
for (let position = 0; position < FILE_NODE_MAX_IDS;) {
const queryResponse = await this.request(
[["FileNode/query", { accountId, filter: {}, position, limit: FILE_NODE_QUERY_PAGE, calculateTotal: true }, "fnq0"]],
using,
);
const queryResult = queryResponse.methodResponses?.[0];
if (!queryResult || queryResult[0] !== "FileNode/query") {
throw new Error(queryResult?.[1]?.description || "FileNode/query failed");
}
// The server may clamp `limit`, so only an empty page or a reached
// total ends the listing - never a page shorter than the one asked for.
const pageIds = (queryResult[1].ids || []) as string[];
if (pageIds.length === 0) break;
for (const id of pageIds) {
if (!known.has(id)) missing.add(id);
}
position += pageIds.length;
const total = queryResult[1].total;
if (typeof total === "number" && position >= total) break;
}
await fetchByIds([...missing]);
const asked = new Set<string>();
for (;;) {
const parents = new Set<string>();
for (const node of known.values()) {
if (node.parentId && !known.has(node.parentId) && !asked.has(node.parentId)) parents.add(node.parentId);
}
if (parents.size === 0) break;
for (const id of parents) asked.add(id);
await fetchByIds([...parents]);
}
} catch (error) {
// A partial tree beats none: keep what was read, as before #1069.
console.warn(`[Files] Listing for account ${accountId} is incomplete past ${maxObjects} nodes:`, error);
}
return [...known.values()];
}
/**
@@ -7191,13 +7274,7 @@ export class JMAPClient implements IJMAPClient {
const isPrimary = accountId === primaryId;
const account = this.accounts[accountId];
try {
const response = await this.request(
[["FileNode/get", { accountId, ids: null, properties: JMAPClient.FILE_NODE_PROPERTIES }, "fng0"]],
this.fileUsing(),
);
const getResult = response.methodResponses?.find(r => r[0] === "FileNode/get");
if (!getResult || getResult[0] === "error") continue;
const nodes = (getResult[1].list || []) as FileNode[];
const nodes = await this.fetchAllFileNodes(accountId);
for (const node of nodes) {
all.push({
...withDecodedName(node),