diff --git a/app/api/dev-jmap/[...path]/route.ts b/app/api/dev-jmap/[...path]/route.ts index 653ba59cf..9af29017f 100644 --- a/app/api/dev-jmap/[...path]/route.ts +++ b/app/api/dev-jmap/[...path]/route.ts @@ -2292,6 +2292,11 @@ function handleFileNodeSet(args: MethodArgs, callId: string): MethodResult { } const blobId = typeof props.blobId === 'string' ? props.blobId : null; const now = new Date().toISOString(); + // The client leaves `size` to the server, which reads it off the blob: + // an upload from this process, or the blob of the node being copied. + const blobSize = blobId + ? uploadedBlobs.get(blobId)?.bytes.byteLength ?? fileNodes.find((n) => n.blobId === blobId)?.size ?? 0 + : 0; const node: FileNode = { id: generateDemoId('file'), parentId: typeof props.parentId === 'string' ? props.parentId : null, @@ -2299,7 +2304,7 @@ function handleFileNodeSet(args: MethodArgs, callId: string): MethodResult { // A node without a blob is a folder (draft-ietf-jmap-filenode). type: blobId ? (typeof props.type === 'string' ? props.type : 'application/octet-stream') : 'd', blobId, - size: typeof props.size === 'number' ? props.size : 0, + size: typeof props.size === 'number' ? props.size : blobSize, created: now, modified: now, }; diff --git a/lib/__tests__/dev-jmap-mock.test.ts b/lib/__tests__/dev-jmap-mock.test.ts index 2a2d22070..56950158c 100644 --- a/lib/__tests__/dev-jmap-mock.test.ts +++ b/lib/__tests__/dev-jmap-mock.test.ts @@ -435,4 +435,33 @@ describe('dev-jmap mock server', () => { expect(data.type).toBe('image/png'); }); }); + + describe('POST /api - FileNode/set', () => { + it('reads the size of a new file off its blob, like a real server', async () => { + const upload = await POST(makeRequest('http://localhost:3000/api/dev-jmap/upload/dev-account-001/', { + method: 'POST', + headers: { 'Content-Type': 'text/plain', host: 'localhost:3000' }, + body: 'hello', + }), { params: Promise.resolve({ path: ['upload', 'dev-account-001'] }) }); + const { blobId } = await upload.json(); + + const res = await POST(makeRequest('http://localhost:3000/api/dev-jmap/api', { + method: 'POST', + headers: { 'Content-Type': 'application/json', host: 'localhost:3000' }, + body: JSON.stringify({ + methodCalls: [['FileNode/set', { + accountId: 'dev-account-001', + create: { + uploaded: { name: 'hello.txt', type: 'text/plain', blobId }, + // A copy points at the blob of a seeded node. + copied: { name: 'copy.pdf', type: 'application/pdf', blobId: 'demo-blob-file-2' }, + }, + }, '0']], + }), + }), { params: Promise.resolve({ path: ['api'] }) }); + const { created } = (await res.json()).methodResponses[0][1]; + expect(created.uploaded.size).toBe(5); + expect(created.copied.size).toBe(148480); + }); + }); }); diff --git a/lib/__tests__/jmap-filenode-writes.test.ts b/lib/__tests__/jmap-filenode-writes.test.ts index 8cf8d3474..4df152af3 100644 --- a/lib/__tests__/jmap-filenode-writes.test.ts +++ b/lib/__tests__/jmap-filenode-writes.test.ts @@ -3,9 +3,14 @@ import { JMAPClient } from '../jmap/client'; type Call = [string, Record, string]; +interface FakeOptions { + /** The SetError the server answers a create with, if it refuses it. */ + refuseCreate?: (props: Record) => Record | undefined; +} + // A primary account "me" plus a shared drive "grp", as listed by // listAllFileNodesAcrossAccounts ("grp:"). -function makeClient(nodes: Record>>) { +function makeClient(nodes: Record>>, options: FakeOptions = {}) { const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); const fileCap = { 'urn:ietf:params:jmap:filenode': {} }; Object.assign(client, { @@ -33,6 +38,8 @@ function makeClient(nodes: Record>>) { return { methodResponses: [['Blob/copy', { copied: Object.fromEntries(blobIds.map(b => [b, `${b}-copy`])) }, id]] }; } if (method === 'FileNode/set' && args.create) { + const refusal = options.refuseCreate?.((args.create as { n0: Record }).n0); + if (refusal) return { methodResponses: [['FileNode/set', { notCreated: { n0: refusal } }, id]] }; return { methodResponses: [['FileNode/set', { created: { n0: { id: `new${++seq}` } } }, id]] }; } if (method === 'FileNode/set' && args.update) { @@ -80,15 +87,37 @@ describe('FileNode writes', () => { await client.copyFileNode('d1', 'Docs (copy)', null); const creates = sets(calls).map(a => (a.create as { n0: Record }).n0); + // No `size`: the server takes it from the blob, and 1.0 refuses a mismatch. expect(creates).toEqual([ { name: 'Docs (copy)' }, - { name: 'a.txt', parentId: 'new1', type: 'text/plain', blobId: 'b1', size: 3 }, + { name: 'a.txt', parentId: 'new1', type: 'text/plain', blobId: 'b1' }, { name: 'Sub', parentId: 'new1' }, - { name: 'b.txt', parentId: 'new3', type: 'text/plain', blobId: 'b2', size: 4 }, + { name: 'b.txt', parentId: 'new3', type: 'text/plain', blobId: 'b2' }, ]); expect(calls.some(([m]) => m === 'Blob/copy')).toBe(false); }); + it('copies WebDAV-created names inside a folder the way they are shown (#869)', async () => { + const { client, calls } = makeClient({ + me: [ + { id: 'd1', name: 'Docs', parentId: null, blobId: null }, + { id: 'f1', name: 'My%20Notes.txt', parentId: 'd1', blobId: 'b1', type: 'text/plain', size: 3 }, + ], + }); + await client.copyFileNode('d1', 'Docs (copy)', null); + + const creates = sets(calls).map(a => (a.create as { n0: Record }).n0); + expect(creates[1]).toMatchObject({ name: 'My Notes.txt' }); + }); + + it('leaves the size of a new file to the server', async () => { + const { client, calls } = makeClient({}); + await client.createFileNode('a.txt', 'b1', 'text/plain', 3, null); + expect(sets(calls)).toEqual([ + { accountId: 'me', onExists: 'rename', create: { n0: { name: 'a.txt', type: 'text/plain', blobId: 'b1' } } }, + ]); + }); + it('shares with the mayWrite rights of servers before 0.16.6', async () => { const { client, calls } = makeClient({}); await client.setFileNodeShare('n1', 'p1', { @@ -110,14 +139,74 @@ describe('FileNode writes', () => { }); }); - it('copies file content across accounts with Blob/copy', async () => { + // Stalwart (0.16.6+ and 1.0) accepts another account's FileNode blobId + // from anyone who may read that node, so copies reuse it. Blob/copy would + // reserve the content to the target account, usable by its members only. + it('copies a file out of a shared drive by reusing its blobId, without Blob/copy', async () => { const { client, calls } = makeClient({ grp: [{ id: 'f1', name: 'shared.pdf', parentId: null, blobId: 'b1', type: 'application/pdf', size: 9 }], }); const copy = await client.copyFileNode('grp:f1', 'shared.pdf', null); - expect(calls.find(([m]) => m === 'Blob/copy')?.[1]).toEqual({ fromAccountId: 'grp', accountId: 'me', blobIds: ['b1'] }); - expect(sets(calls)[0]).toMatchObject({ accountId: 'me', create: { n0: { blobId: 'b1-copy' } } }); + expect(sets(calls)).toEqual([ + { accountId: 'me', onExists: 'rename', create: { n0: { name: 'shared.pdf', type: 'application/pdf', blobId: 'b1' } } }, + ]); + expect(calls.some(([m]) => m === 'Blob/copy')).toBe(false); expect(copy.id).toBe('new1'); }); + + it('copies an own folder into someone else\'s shared drive with the same blobs', async () => { + const { client, calls } = makeClient({ + me: [ + { id: 'd1', name: 'Docs', parentId: null, blobId: null }, + { id: 'f1', name: 'a.txt', parentId: 'd1', blobId: 'b1', type: 'text/plain', size: 3 }, + ], + }); + const copy = await client.copyFileNode('d1', 'Docs', 'grp:d9'); + + expect(sets(calls)).toEqual([ + { accountId: 'grp', onExists: 'rename', create: { n0: { name: 'Docs', parentId: 'd9' } } }, + { accountId: 'grp', onExists: 'rename', create: { n0: { name: 'a.txt', parentId: 'new1', type: 'text/plain', blobId: 'b1' } } }, + ]); + expect(calls.some(([m]) => m === 'Blob/copy')).toBe(false); + expect(copy.id).toBe('grp:new1'); + }); + + const docs = [ + { id: 'd1', name: 'Docs', parentId: null, blobId: null }, + { id: 'f1', name: 'a.txt', parentId: 'd1', blobId: 'b1', type: 'text/plain', size: 3 }, + { id: 'f2', name: 'b.txt', parentId: 'd1', blobId: 'b2', type: 'text/plain', size: 4 }, + ]; + + it('removes the half-made copy and reports why when a create fails part-way on Stalwart 1.0', async () => { + const { client, calls } = makeClient({ me: docs }, { + // 1.0 once an administrator has set maxFiles. + refuseCreate: (props) => props.name === 'b.txt' + ? { type: 'overQuota', description: 'There are too many files, please delete some before adding a new one.' } + : undefined, + }); + + await expect(client.copyFileNode('d1', 'Docs', 'grp:d9')).rejects.toThrow('There are too many files'); + expect(sets(calls).at(-1)).toEqual({ accountId: 'grp', destroy: ['new1'], onDestroyRemoveChildren: true }); + }); + + it('removes the half-made copy when the server refuses a blob on 0.16', async () => { + const { client, calls } = makeClient({ grp: docs }, { + refuseCreate: (props) => props.blobId === 'b2' + ? { type: 'forbidden', description: 'You do not have access to blobId b2.' } + : undefined, + }); + + await expect(client.copyFileNode('grp:d1', 'Docs', null)).rejects.toThrow('You do not have access to blobId b2.'); + expect(sets(calls).at(-1)).toEqual({ accountId: 'me', destroy: ['new1'], onDestroyRemoveChildren: true }); + }); + + it('has nothing to remove when the copy itself is refused', async () => { + const { client, calls } = makeClient({ me: docs }, { + refuseCreate: () => ({ type: 'forbidden', description: 'You are not allowed to create file nodes in this folder.' }), + }); + + await expect(client.copyFileNode('d1', 'Docs', 'grp:d9')).rejects.toThrow('not allowed'); + expect(sets(calls).some(a => a.destroy)).toBe(false); + }); }); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index de2d5ee59..fc0bd55e0 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -8212,14 +8212,18 @@ export class JMAPClient implements IJMAPClient { return this.namespaceFileNode(created, parent.accountId); } - async createFileNode(name: string, blobId: string, type: string, size: number, parentId: string | null): Promise { + /** + * `size` is not sent: every server works it out from the blob, and + * Stalwart 1.0 refuses a create whose size differs from the blob's. + */ + async createFileNode(name: string, blobId: string, type: string, _size: number, parentId: string | null): Promise { const parent = this.resolveFileNodeId(parentId); // Servers before 0.16.6 refuse MIME types over 30 characters (most OOXML // types); later ones take up to 255. const maxTypeLength = this.isLegacyFileNodeServer(parent.accountId) ? 30 : 255; const safeType = type.length > maxTypeLength ? 'application/octet-stream' : type; - const fileProps: Record = { name, type: safeType, blobId, size }; + const fileProps: Record = { name, type: safeType, blobId }; if (parent.id !== null) { fileProps.parentId = parent.id; } @@ -8359,7 +8363,13 @@ export class JMAPClient implements IJMAPClient { * Copy a file, or a folder with everything in it, into `parentId` (which * may be in another account). A folder node carries no content, so creating * one "copy" node would only make an empty folder: the subtree is recreated - * level by level. Across accounts the file contents go through Blob/copy. + * level by level. + * + * The copies point at the source's blobs. Stalwart lets anyone who may read + * a FileNode use its blobId in any account, so this works both ways between + * drives. Blob/copy reserved the content to the target account, which only + * that account's members may use: copying into someone else's shared drive + * failed with it. */ async copyFileNode(id: string, newName: string, parentId: string | null): Promise { const source = this.resolveFileNodeId(id); @@ -8370,38 +8380,55 @@ export class JMAPClient implements IJMAPClient { if (!original) throw new Error('File node not found'); const isFolderNode = (n: FileNode) => n.blobId == null; - const copyBlob = async (blobId: string): Promise => { - if (source.accountId === target.accountId) return blobId; - const response = await this.request([ - ["Blob/copy", { fromAccountId: source.accountId, accountId: target.accountId, blobIds: [blobId] }, "bc0"], - ], this.fileUsing()); - const result = response.methodResponses?.[0]; - const copied = result?.[0] === "Blob/copy" ? result[1].copied?.[blobId] : undefined; - if (!copied) { - throw new Error(result?.[1]?.notCopied?.[blobId]?.description || result?.[1]?.description || "Failed to copy file content"); - } - return copied as string; + // The whole subtree is laid out from one listing before the first create. + const childrenOf = new Map(); + for (const node of tree) { + if (node.parentId == null) continue; + const siblings = childrenOf.get(node.parentId); + if (siblings) siblings.push(node); + else childrenOf.set(node.parentId, [node]); + } + type CopyEntry = { node: FileNode; children: CopyEntry[] }; + const seen = new Set(); + const collect = (node: FileNode): CopyEntry => { + seen.add(node.id); + const below = isFolderNode(node) ? childrenOf.get(node.id) ?? [] : []; + return { + node: withDecodedName(node), + children: below.filter(child => !seen.has(child.id)).map(collect), + }; }; + const plan = collect(original); - const copyInto = async (node: FileNode, name: string, parent: string | null): Promise => { + const createCopy = (node: FileNode, name: string, parent: string | null): Promise => { const props: Record = { name }; if (parent !== null) props.parentId = parent; if (!isFolderNode(node)) { props.type = node.type; - props.blobId = await copyBlob(node.blobId as string); - props.size = node.size; + props.blobId = node.blobId; } - const created = await this.createFileNodeIn(target.accountId, props, 'copy'); - if (isFolderNode(node)) { - for (const child of tree.filter(n => n.parentId === node.id)) { - await copyInto(child, child.name, created.id); - } + return this.createFileNodeIn(target.accountId, props, 'copy'); + }; + const copyChildren = async (entry: CopyEntry, parent: string): Promise => { + for (const child of entry.children) { + const created = await createCopy(child.node, child.node.name, parent); + await copyChildren(child, created.id); } - return created; }; - const created = await copyInto(withDecodedName(original), newName, target.id); - return this.namespaceFileNode(created, target.accountId); + const root = await createCopy(plan.node, newName, target.id); + const copy = this.namespaceFileNode(root, target.accountId); + try { + await copyChildren(plan, root.id); + } catch (error) { + // Take the half-made copy away again (its children go with it) and + // report what stopped it. + await this.destroyFileNodes([copy.id]).catch((cleanup) => { + console.warn('[Files] Could not remove a partly created copy:', cleanup instanceof Error ? cleanup.message : cleanup); + }); + throw error; + } + return copy; } async downloadBlob(blobId: string, name?: string, type?: string, accountId?: string): Promise {