From a5a06bb25fd4e1087dafed0be346da0f3a34e7fd Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Sat, 26 Sep 2026 15:20:29 +0200
Subject: [PATCH] fix: never replay a JMAP write after a dropped connection

A network error on a JMAP request was retried once after a second.
Timeouts were already excluded, but a connection that failed after the
request was written (a reset, a proxy dropping the answer) still
replayed the whole batch, so one click on Send could queue two
submissions and deliver the mail twice. Only requests that change
nothing are retried now: GETs, blob uploads and batches made only of
read methods.
---
 lib/__tests__/jmap-client-resilience.test.ts | 35 +++++++++++++++++++-
 lib/jmap/client.ts                           | 27 +++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)

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<typeof fetch>[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);
     }