diff --git a/lib/__tests__/jmap-client-resilience.test.ts b/lib/__tests__/jmap-client-resilience.test.ts index 6bcbef2e9..b0c176866 100644 --- a/lib/__tests__/jmap-client-resilience.test.ts +++ b/lib/__tests__/jmap-client-resilience.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { JMAPClient, RequestTimeoutError } from '../jmap/client'; +import { JMAPClient, RequestTimeoutError, isReplaySafeRequest } from '../jmap/client'; /** * A connection that accepts the request and then goes silent: the promise never @@ -131,6 +131,39 @@ describe('JMAPClient resilience', () => { await expect(client.ping()).rejects.toThrow('Failed to fetch'); expect(fetchSpy).toHaveBeenCalledTimes(2); }); + + it('does not replay a request that changes something', async () => { + // The connection may have failed after the server acted on the batch: + // replaying an Email/set or an EmailSubmission/set does it twice. + const client = await createConnectedClient(); + fetchSpy.mockRejectedValueOnce(new TypeError('Failed to fetch')); + + await expect(client.deleteEmail('email-1')).rejects.toThrow('Failed to fetch'); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('isReplaySafeRequest', () => { + const batch = (...methods: string[]) => ({ + method: 'POST', + body: JSON.stringify({ using: [], methodCalls: methods.map((m, i) => [m, {}, String(i)]) }), + }); + + it('allows GETs, uploads and read-only batches', () => { + expect(isReplaySafeRequest(undefined)).toBe(true); + expect(isReplaySafeRequest({ method: 'GET' })).toBe(true); + expect(isReplaySafeRequest({ method: 'POST', body: new Blob(['x']) })).toBe(true); + expect(isReplaySafeRequest(batch('Email/query', 'Email/get', 'Mailbox/changes', 'Core/echo'))).toBe(true); + }); + + it('refuses anything that writes, and bodies it cannot read', () => { + expect(isReplaySafeRequest(batch('Email/get', 'Email/set'))).toBe(false); + expect(isReplaySafeRequest(batch('Email/set', 'EmailSubmission/set'))).toBe(false); + expect(isReplaySafeRequest(batch('Email/copy'))).toBe(false); + expect(isReplaySafeRequest(batch('Email/import'))).toBe(false); + expect(isReplaySafeRequest({ method: 'POST', body: 'not json' })).toBe(false); + expect(isReplaySafeRequest({ method: 'POST', body: JSON.stringify({ methodCalls: [] }) })).toBe(false); + }); }); describe('authenticatedFetch - stalled request deadline', () => { diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 628262d4d..a58e83b9f 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -165,6 +165,29 @@ export class RequestTimeoutError extends Error { } } +const READ_ONLY_METHOD = /\/(?:get|query|changes|queryChanges|parse)$|^Core\/echo$/; + +/** + * Whether a request may be sent again after the connection failed without + * an answer. The server may already have acted on it, so only requests that + * change nothing qualify: GETs, blob uploads (a new upload just yields + * another blob), and JMAP batches made only of read methods. Anything + * with a /set, /copy, /import or a submission would be done twice - one + * Send delivering two messages. + */ +export function isReplaySafeRequest(init?: Parameters[1]): boolean { + const method = (init?.method ?? 'GET').toUpperCase(); + if (method === 'GET' || method === 'HEAD') return true; + if (typeof init?.body !== 'string') return init?.body instanceof Blob; + try { + const request = JSON.parse(init.body) as { methodCalls?: unknown }; + if (!Array.isArray(request.methodCalls) || request.methodCalls.length === 0) return false; + return request.methodCalls.every((call) => Array.isArray(call) && typeof call[0] === 'string' && READ_ONLY_METHOD.test(call[0])); + } catch { + return false; + } +} + /** A scheduled send later than the server's hold limit. */ export class ScheduleTooLateError extends Error { constructor(readonly maxSeconds?: number) { @@ -1074,6 +1097,10 @@ export class JMAPClient implements IJMAPClient { // idempotent: replaying an EmailSubmission/set would send the mail twice. // Surface it instead so the caller can report a failure the user can act on. if (error instanceof RequestTimeoutError) throw error; + // The same holds for a connection that failed after the request was + // written (a reset, a proxy dropping the response): only a request that + // changes nothing may be replayed. + if (!isReplaySafeRequest(init)) throw error; await new Promise(r => setTimeout(r, 1000)); response = await this.timedFetch(url, init, headers, timeoutMs); }