diff --git a/src/api/__tests__/read-receipt.test.ts b/src/api/__tests__/read-receipt.test.ts new file mode 100644 index 00000000..6ae980d4 --- /dev/null +++ b/src/api/__tests__/read-receipt.test.ts @@ -0,0 +1,154 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../jmap-client', () => ({ + jmapClient: { + accountId: 'acc-1', + request: vi.fn(), + getMaxObjectsInSet: vi.fn(() => 500), + }, +})); + +// blob.ts pulls in expo-file-system; the receipt only needs the byte upload. +const uploadBytes = vi.fn(async (..._args: unknown[]) => ({ blobId: 'blob-1', size: 1, type: 'message/rfc822' })); +vi.mock('../blob', () => ({ + uploadBytes: (...args: unknown[]) => uploadBytes(...args), +})); + +import { jmapClient } from '../jmap-client'; +import { sendReadReceipt } from '../email'; + +const mockRequest = jmapClient.request as ReturnType; + +type Call = [string, Record, string]; + +/** Answer each JMAP method with the given handler; records every call. */ +function serve(handlers: Record) => unknown>) { + const calls: Call[] = []; + mockRequest.mockImplementation(async (methodCalls: Call[]) => { + calls.push(...methodCalls); + return { + methodResponses: methodCalls.map(([name, args, id]) => { + const handler = handlers[name]; + if (!handler) throw new Error(`unexpected ${name}`); + const result = handler(args); + return (Array.isArray(result) ? [...result, id] : [name, result, id]) as unknown; + }), + }; + }); + return calls; +} + +const opts = { + to: 'sender@remote.example', + fromEmail: 'me@x.example', + identityId: 'ident-1', + sentMailboxId: 'sent-1', + originalSubject: 'Hello', + automatic: true, +}; + +const imported = () => ({ created: { 'import-0': { id: 'mdn-email-1' } } }); +const destroyed = (args: Record) => ({ destroyed: args.destroy }); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('sendReadReceipt', () => { + it('imports the receipt into Sent and submits it', async () => { + const calls = serve({ + 'Email/import': imported, + 'EmailSubmission/set': () => ({ created: { mdn: { id: 'sub-1' } } }), + 'Email/set': destroyed, + }); + + await expect(sendReadReceipt(opts)).resolves.toBe('mdn-email-1'); + + expect(calls.map((c) => c[0])).toEqual(['Email/import', 'EmailSubmission/set']); + expect(calls[0][1].emails['import-0']).toMatchObject({ blobId: 'blob-1', mailboxIds: { 'sent-1': true } }); + expect(calls[1][1].create.mdn).toMatchObject({ + emailId: 'mdn-email-1', + identityId: 'ident-1', + envelope: { mailFrom: { email: 'me@x.example' }, rcptTo: [{ email: 'sender@remote.example' }] }, + }); + }); + + it('uploads a receipt whose headers a CRLF in the original subject cannot extend', async () => { + serve({ + 'Email/import': imported, + 'EmailSubmission/set': () => ({ created: { mdn: { id: 'sub-1' } } }), + }); + + await sendReadReceipt({ + ...opts, + subject: 'Read: Contract update\r\nX-Injected: yes\r\nReply-To: attacker@evil.example', + }); + + const raw = new TextDecoder().decode(uploadBytes.mock.calls[0][0] as Uint8Array); + expect(raw).not.toMatch(/^X-Injected:/m); + expect(raw).not.toMatch(/^Reply-To:/m); + expect(raw).toContain('Subject: Read: Contract update X-Injected: yes Reply-To: attacker@evil.example\r\n'); + }); + + it('destroys the imported copy when the server refuses the submission', async () => { + const calls = serve({ + 'Email/import': imported, + 'EmailSubmission/set': () => ({ + notCreated: { mdn: { type: 'invalidRecipients', description: 'No valid recipients' } }, + }), + 'Email/set': destroyed, + }); + + await expect(sendReadReceipt({ ...opts, accountId: 'grp-1' })).rejects.toThrow(/No valid recipients/); + + const destroy = calls.find((c) => c[0] === 'Email/set'); + expect(destroy?.[1]).toEqual({ accountId: 'grp-1', destroy: ['mdn-email-1'] }); + }); + + it('destroys the imported copy when the submission call errors', async () => { + const calls = serve({ + 'Email/import': imported, + 'EmailSubmission/set': () => ['error', { type: 'forbiddenFrom', description: 'Not allowed' }], + 'Email/set': destroyed, + }); + + await expect(sendReadReceipt(opts)).rejects.toThrow(/Not allowed/); + expect(calls.find((c) => c[0] === 'Email/set')?.[1]).toEqual({ accountId: 'acc-1', destroy: ['mdn-email-1'] }); + }); + + it('destroys the imported copy when the submission request fails in transit', async () => { + const calls: Call[] = []; + mockRequest.mockImplementation(async (methodCalls: Call[]) => { + calls.push(...methodCalls); + const [name, args, id] = methodCalls[0]; + if (name === 'Email/import') return { methodResponses: [[name, imported(), id]] }; + if (name === 'EmailSubmission/set') throw new Error('Network request failed'); + return { methodResponses: [[name, destroyed(args), id]] }; + }); + + await expect(sendReadReceipt(opts)).rejects.toThrow('Network request failed'); + expect(calls.map((c) => c[0])).toEqual(['Email/import', 'EmailSubmission/set', 'Email/set']); + }); + + it('still reports the submission error when the cleanup fails too', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + serve({ + 'Email/import': imported, + 'EmailSubmission/set': () => ({ notCreated: { mdn: { type: 'forbiddenToSend' } } }), + 'Email/set': () => ({ notDestroyed: { 'mdn-email-1': { type: 'forbidden' } } }), + }); + + await expect(sendReadReceipt(opts)).rejects.toThrow(/forbiddenToSend/); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('does not submit or destroy anything when the import fails', async () => { + const calls = serve({ + 'Email/import': () => ({ notCreated: { 'import-0': { type: 'overQuota' } } }), + }); + + await expect(sendReadReceipt(opts)).rejects.toThrow(/overQuota/); + expect(calls.map((c) => c[0])).toEqual(['Email/import']); + }); +}); diff --git a/src/api/email.ts b/src/api/email.ts index 27fe3ca2..77b906f6 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -1450,8 +1450,9 @@ export interface SendReadReceiptOptions extends MdnOptions { * Send an RFC 8098 read receipt (MDN). JMAP has no MDN primitive, so the * multipart/report is built client-side (`lib/mdn.ts`), uploaded as a blob, * imported into Sent and submitted with an explicit envelope - mirrors the - * webmail's `client.sendReadReceipt`. The caller flags the original - * `$mdnsent` afterwards. + * webmail's `client.sendReadReceipt`. When the submission fails the imported + * copy is destroyed again, so a refused receipt does not sit in Sent as if + * it had gone out. The caller flags the original `$mdnsent` afterwards. */ export async function sendReadReceipt(opts: SendReadReceiptOptions): Promise { const accountId = opts.accountId ?? jmapClient.accountId; @@ -1462,25 +1463,32 @@ export async function sendReadReceipt(opts: SendReadReceiptOptions): Promise { + console.warn('[mdn] failed to remove the unsent receipt from Sent:', cleanupErr); + }); + throw err; + } return emailId; } diff --git a/src/components/email/MessageContent.tsx b/src/components/email/MessageContent.tsx index 19f38a93..cfec898a 100644 --- a/src/components/email/MessageContent.tsx +++ b/src/components/email/MessageContent.tsx @@ -19,6 +19,8 @@ export interface MessageContentProps { jmapAccountId?: string; identities: Identity[]; currentMailboxRole?: string | null; + /** On screen rather than pre-rendered by the pager (gates auto-sent read receipts). */ + active: boolean; themeOverride?: 'light' | 'dark' | null; onSwipe?: (direction: 'prev' | 'next') => void; onZoomChange?: (zoom: { pinching: boolean; zoomed: boolean }) => void; @@ -37,7 +39,7 @@ export interface MessageContentProps { * cards. */ export function MessageContent({ - email, jmapAccountId, identities, currentMailboxRole, themeOverride, onSwipe, onZoomChange, + email, jmapAccountId, identities, currentMailboxRole, active, themeOverride, onSwipe, onZoomChange, onToggleStar, onAddressPress, onEmailPatched, compact, }: MessageContentProps) { const c = useColors(); @@ -80,6 +82,7 @@ export function MessageContent({ requestedBy={headerInfo.readReceiptRequestedBy} jmapAccountId={jmapAccountId} currentMailboxRole={currentMailboxRole} + active={active} onHandled={onEmailPatched} /> )} diff --git a/src/components/email/ReadReceiptBanner.tsx b/src/components/email/ReadReceiptBanner.tsx index 94baecd1..417016eb 100644 --- a/src/components/email/ReadReceiptBanner.tsx +++ b/src/components/email/ReadReceiptBanner.tsx @@ -8,6 +8,7 @@ import { useLocaleStore } from '../../stores/locale-store'; import { useSettingsStore } from '../../stores/settings-store'; import { useEmailStore } from '../../stores/email-store'; import { sendReadReceipt, patchKeywordsForEmails } from '../../api/email'; +import { jmapClient } from '../../api/jmap-client'; import { findReceivingIdentity } from '../../lib/email-headers'; import { mailboxesForSiblingOf } from '../../lib/mailbox-tree'; @@ -18,17 +19,24 @@ interface Props { jmapAccountId?: string; /** Role of the folder the message was opened from (receipts only in received folders). */ currentMailboxRole?: string | null; + /** The message is on screen, not in a neighbour page the pager pre-renders. */ + active: boolean; /** Reflect `$mdnsent` in the caller's cache. */ onHandled: (email: Email) => void; } +// Messages "always" mode has already tried to answer this session, keyed by +// account and id. One attempt each: a failed send is not retried, and a page +// the pager unmounts and mounts again does not send a second time. +const autoAttempted = new Set(); + /** * Read-receipt (MDN, RFC 8098) request banner: Send / Ignore, or auto-send * in "always" mode. Either answer flags the message `$mdnsent` (RFC 3503) so * the request is suppressed in every client, not just here. Never offered * for the user's own copies, trash or spam. */ -export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMailboxRole, onHandled }: Props) { +export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMailboxRole, active, onHandled }: Props) { const c = useColors(); const styles = React.useMemo(() => makeStyles(c), [c]); const t = useLocaleStore((s) => s.t); @@ -39,7 +47,6 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa const currentMailboxId = useEmailStore((s) => s.currentMailboxId); const [busy, setBusy] = React.useState(false); const [handledLocally, setHandledLocally] = React.useState(false); - const autoRef = React.useRef(null); React.useEffect(() => { setHandledLocally(false); }, [email.id]); React.useEffect(() => { if (identities.length === 0) void fetchIdentities(); }, [identities.length, fetchIdentities]); @@ -88,17 +95,17 @@ export function ReadReceiptBanner({ email, requestedBy, jmapAccountId, currentMa await flagSent(); }, [identity, mailboxes, currentMailboxId, requestedBy, jmapAccountId, email.messageId, email.subject, t, flagSent]); - // "always" mode: auto-send once when the message is opened. + // "always" mode: auto-send once when the message is actually displayed. React.useEffect(() => { - if (readReceiptResponse !== 'always' || !shouldOffer) return; - if (autoRef.current === email.id) return; - autoRef.current = email.id; + if (readReceiptResponse !== 'always' || !shouldOffer || !active) return; + const key = `${jmapAccountId ?? jmapClient.accountId}:${email.id}`; + if (autoAttempted.has(key)) return; + autoAttempted.add(key); setHandledLocally(true); send(true).catch((err) => { console.warn('[mdn] auto-send failed', err); - autoRef.current = null; }); - }, [readReceiptResponse, shouldOffer, email.id, send]); + }, [readReceiptResponse, shouldOffer, active, jmapAccountId, email.id, send]); if (!shouldOffer || readReceiptResponse === 'always') return null; diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index 90c33591..0e3a032d 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -648,6 +648,7 @@ export default function EmailThreadScreen({ route, navigation }: Props) { ; @@ -815,7 +818,7 @@ interface EmailPaneProps { // expand). The pager keeps three of these mounted (prev, current, next) so a // swipe slides ready content into view. function EmailPane({ - id, threadIdHint, email, detailCache, threadIds, threading, jmapAccountId, currentMailboxRole, + id, active, threadIdHint, email, detailCache, threadIds, threading, jmapAccountId, currentMailboxRole, identities, themeOverrides, ensureDetail, ensureThread, scheduleMarkRead, styles, bottomBarHeight, onToggleStar, onAddressPress, onEmailPatched, onReply, onSwipe, onZoomChange, }: EmailPaneProps) { @@ -923,6 +926,7 @@ function EmailPane({ jmapAccountId={jmapAccountId} identities={identities} currentMailboxRole={currentMailboxRole} + active={active} themeOverride={themeOverrides[m.id] ?? null} onSwipe={onSwipe} onZoomChange={(z) => { setPinching(z.pinching); onZoomChange(z); }} @@ -937,6 +941,7 @@ function EmailPane({ jmapAccountId={jmapAccountId} identities={identities} currentMailboxRole={currentMailboxRole} + active={active} themeOverride={themeOverrides[email.id] ?? null} onSwipe={onSwipe} onZoomChange={(z) => { setPinching(z.pinching); onZoomChange(z); }}