From d2bb479ac6ed80d4117eb00610dc4b9f3e7b80b5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 01:53:57 +0100 Subject: [PATCH] fix: keep inline images of drafts and sent mail on Stalwart 1.0 --- src/api/__tests__/email-inline-parts.test.ts | 69 ++++++++++++++++++++ src/api/__tests__/live-stalwart.test.ts | 13 +++- src/api/email.ts | 17 +++-- src/lib/__tests__/email-detail-cache.test.ts | 11 ++++ src/lib/__tests__/email-inline-parts.test.ts | 26 ++++++++ src/lib/email-detail-cache.ts | 6 +- src/lib/email-inline-parts.ts | 48 ++++++++++++++ 7 files changed, 180 insertions(+), 10 deletions(-) create mode 100644 src/api/__tests__/email-inline-parts.test.ts create mode 100644 src/lib/__tests__/email-inline-parts.test.ts create mode 100644 src/lib/email-inline-parts.ts diff --git a/src/api/__tests__/email-inline-parts.test.ts b/src/api/__tests__/email-inline-parts.test.ts new file mode 100644 index 00000000..7a5ad182 --- /dev/null +++ b/src/api/__tests__/email-inline-parts.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../jmap-client', () => ({ + jmapClient: { + accountId: 'acc-1', + request: vi.fn(), + getMaxCallsInRequest: vi.fn(() => 16), + getMaxObjectsInGet: vi.fn(() => 500), + getMaxObjectsInSet: vi.fn(() => 500), + }, +})); + +import { jmapClient } from '../jmap-client'; +import { getFullEmail, getFullEmails, saveDraftVersion } from '../email'; + +const mockRequest = jmapClient.request as ReturnType; + +// A draft with an inline image as Stalwart 1.0.0 returns it (measured +// 2026-10-29): the image sits in mixed next to the alternative, so it is in +// textBody and htmlBody but not in `attachments`. +const IMAGE = { partId: '4', blobId: 'b-img', type: 'image/png', disposition: 'inline', cid: 'img1@live.test', name: 'img1.png', size: 70 }; +const on10 = { + id: 'd1', + attachments: [], + textBody: [{ partId: '2', blobId: 'b-text', type: 'text/plain' }, IMAGE], + htmlBody: [{ partId: '3', blobId: 'b-html', type: 'text/html' }, IMAGE], +}; +// The same message on 0.16.27: the image is in all three lists. +const on016 = { ...on10, attachments: [IMAGE] }; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('inline images that Stalwart 1.0 lists only in the body lists', () => { + it('getFullEmail takes them into attachments', async () => { + mockRequest.mockResolvedValue({ methodResponses: [['Email/get', { list: [on10], notFound: [] }, '0']] }); + const email = await getFullEmail('d1'); + expect(email.attachments).toEqual([IMAGE]); + }); + + it('getFullEmails does too, and lists a 0.16 part once', async () => { + mockRequest.mockResolvedValue({ + methodResponses: [['Email/get', { list: [on10, { ...on016, id: 'd2' }], notFound: [] }, '0']], + }); + const [a, b] = await getFullEmails(['d1', 'd2']); + expect(a.attachments?.map((x) => x.cid)).toEqual(['img1@live.test']); + expect(b.attachments).toHaveLength(1); + }); + + it('saveDraftVersion reads the new version\'s inline part back from the body lists', async () => { + mockRequest.mockImplementation(async (calls: Array<[string, Record, string]>) => { + const get = calls.find(([m]) => m === 'Email/get')![1]; + expect(get.properties).toEqual(expect.arrayContaining(['attachments', 'textBody', 'htmlBody'])); + return { + methodResponses: [ + ['Email/set', { created: { draft: { id: 'd1' } } }, '0'], + ['Email/get', { list: [on10] }, '1'], + ], + }; + }); + const saved = await saveDraftVersion({ + from: [{ email: 'me@example.org' }], to: [], subject: 's', + htmlBody: '

', + attachments: [{ blobId: 'up-1', type: 'image/png', name: 'img1.png', disposition: 'inline', cid: 'img1@live.test' }], + }, 'drafts'); + expect(saved.attachments.map((a) => a.blobId)).toEqual(['b-img']); + }); +}); diff --git a/src/api/__tests__/live-stalwart.test.ts b/src/api/__tests__/live-stalwart.test.ts index f15d5999..253a48b8 100644 --- a/src/api/__tests__/live-stalwart.test.ts +++ b/src/api/__tests__/live-stalwart.test.ts @@ -267,9 +267,16 @@ live('live Stalwart', () => { draftId = saved.id; state = rebaseDraftPartBlobs(state, sent, saved.attachments); } - // Without the rebase the seeded part blob is gone with D0. - await expect(saveDraftVersion({ ...base, attachments: [img(seeded)] }, drafts)) - .rejects.toThrow(/does not exist|blobNotFound/); + // Each save re-pointed the image at the new version's part (on 1.0 + // the read-back finds it in the body lists only). + expect(liveBlobId(state, seeded)).not.toBe(seeded); + // Without the rebase the seeded part blob is gone with D0. 0.16 says + // "blobId … does not exist", 1.0 "One or more blobIds do not exist"; + // 1.0 sometimes still resolves it for a moment after the destroy. + const stale = await saveDraftVersion({ ...base, attachments: [img(seeded)] }, drafts) + .then((v) => v, (e: Error) => e); + if (stale instanceof Error) expect(stale.message).toMatch(/does not exist|do not exist|blobNotFound/); + else created.push(stale.id); const [last] = await getFullEmails([draftId]); expect(last.attachments?.[0]?.cid).toContain('img1@live.test'); } finally { diff --git a/src/api/email.ts b/src/api/email.ts index 787a0fba..55786467 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -16,6 +16,7 @@ import { sanitizeDisplayName } from '../lib/rfc5322-mailbox'; import { generateMessageId, stripMessageIdBrackets } from '../lib/email-threading'; import { buildMdnMessage, type MdnOptions } from '../lib/mdn'; import { hasTruncatedDisplayedBody } from '../lib/email-body'; +import { withInlineBodyParts } from '../lib/email-inline-parts'; import { stalwartStateWentBack } from './stalwart-state'; import { isTransientNetworkError } from '../lib/network-error'; @@ -936,8 +937,10 @@ export async function getFullEmail(id: string, accountIdOverride?: string): Prom ['Email/get', { accountId, ids: [id], ...FULL_BODY_ARGS }, '0'], ]); const body = requireMethodResult(res, '0', 'Email/get'); - const email = (body.list as Email[] | undefined)?.[0]; - if (!email) throw new Error(`Email ${id} not found`); + const fetched = (body.list as Email[] | undefined)?.[0]; + if (!fetched) throw new Error(`Email ${id} not found`); + // Stalwart 1.0 lists an inline image in mixed only in the body lists. + const email = withInlineBodyParts(fetched); await refetchTruncatedBodyValues([email], accountId); return email; } @@ -972,7 +975,7 @@ export async function getFullEmailsWithState( ['Email/get', { accountId, ids: slice, ...FULL_BODY_ARGS }, '0'], ]); const body = requireMethodResult(res, '0', 'Email/get'); - out.list.push(...((body.list as Email[]) ?? [])); + out.list.push(...((body.list as Email[]) ?? []).map(withInlineBodyParts)); out.notFound.push(...((body.notFound as string[] | undefined) ?? [])); out.state = body.state as string | undefined; } @@ -1908,7 +1911,8 @@ export async function saveDraftVersion( ['Email/get', { accountId, ids: ['#draft'], - properties: ['id', 'attachments'], + // The body lists too: Stalwart 1.0 lists the inline images there only. + properties: ['id', 'attachments', 'textBody', 'htmlBody'], bodyProperties: DRAFT_PART_BODY_PROPERTIES, }, '1'], ]); @@ -1920,8 +1924,9 @@ export async function saveDraftVersion( let attachments: Attachment[] = []; try { const got = requireMethodResult(res, '1', 'Email/get'); - const saved = (got.list as Array<{ id: string; attachments?: Attachment[] }> | undefined)?.find((e) => e.id === id); - attachments = (saved?.attachments ?? []).filter((a) => !!a.blobId); + const saved = (got.list as Array> | undefined) + ?.find((e) => e.id === id); + attachments = (saved ? withInlineBodyParts(saved).attachments ?? [] : []).filter((a) => !!a.blobId); } catch (err) { console.warn('[email] could not read back the saved draft parts:', err); } diff --git a/src/lib/__tests__/email-detail-cache.test.ts b/src/lib/__tests__/email-detail-cache.test.ts index 414cffbb..5248f445 100644 --- a/src/lib/__tests__/email-detail-cache.test.ts +++ b/src/lib/__tests__/email-detail-cache.test.ts @@ -184,6 +184,17 @@ describe('loading a message', () => { expect((await useOfflineCacheStore.getState().get('e1'))?.keywords).toEqual({ $flagged: true }); }); + it('takes the inline image of an offline copy saved from Stalwart 1.0 into its attachments', async () => { + const image = { partId: '4', blobId: 'b-img', type: 'image/png', disposition: 'inline', cid: 'logo@x' }; + const offline = { ...full('e1'), attachments: [], htmlBody: [{ partId: '1', type: 'text/html' }, image] } as Email; + await useOfflineCacheStore.getState().put(offline, 100); + flagsGet.mockReturnValue(new Promise(() => undefined)); + + void loadDetail('e1'); + await flush(); + expect(peekDetail('e1')?.attachments).toEqual([image]); + }); + it('batches what it holds and what it lacks into one request each', async () => { fullGet.mockResolvedValueOnce(fullResponse([full('a'), full('b')], 's1')); await loadDetails(['a', 'b']); diff --git a/src/lib/__tests__/email-inline-parts.test.ts b/src/lib/__tests__/email-inline-parts.test.ts new file mode 100644 index 00000000..22f39808 --- /dev/null +++ b/src/lib/__tests__/email-inline-parts.test.ts @@ -0,0 +1,26 @@ +import { describe, it, expect } from 'vitest'; +import { inlineBodyParts, withInlineBodyParts } from '../email-inline-parts'; + +const image = { partId: '4', blobId: 'b-img', type: 'image/png', disposition: 'inline', cid: 'x@y' }; +const text = { partId: '2', blobId: 'b-text', type: 'text/plain' }; +const html = { partId: '3', blobId: 'b-html', type: 'text/html' }; + +describe('inlineBodyParts', () => { + it('takes non-text body parts once, in body order', () => { + const audio = { partId: '5', blobId: 'b-audio', type: 'audio/ogg' }; + expect(inlineBodyParts({ attachments: [], textBody: [text, image, audio], htmlBody: [html, image] })) + .toEqual([image, audio]); + }); + + it('skips parts already in attachments and text parts in either list', () => { + expect(inlineBodyParts({ attachments: [image], textBody: [text, image], htmlBody: [html, image] })).toEqual([]); + expect(inlineBodyParts({ textBody: [text], htmlBody: [html, { partId: '6', type: 'TEXT/calendar', blobId: 'c' }] })).toEqual([]); + }); + + it('returns the same object when there is nothing to add', () => { + const email = { attachments: [image], textBody: [text], htmlBody: [html] }; + expect(withInlineBodyParts(email)).toBe(email); + const on10 = { attachments: [], textBody: [text, image], htmlBody: [html, image] }; + expect(withInlineBodyParts(on10).attachments).toEqual([image]); + }); +}); diff --git a/src/lib/email-detail-cache.ts b/src/lib/email-detail-cache.ts index aea6259c..4e6f114e 100644 --- a/src/lib/email-detail-cache.ts +++ b/src/lib/email-detail-cache.ts @@ -18,6 +18,7 @@ import { getEmailFlags, getFullEmailsWithState, getThreadsHeaders } from '../api import { useOfflineCacheStore } from '../stores/offline-cache-store'; import { useSettingsStore } from '../stores/settings-store'; import { onStateChangeType } from './state-change-bus'; +import { withInlineBodyParts } from './email-inline-parts'; /** What the list row the viewer was opened from says about the message. */ export type FlagsHint = Partial>; @@ -378,7 +379,10 @@ function startLoads( const copies = await Promise.all(fromOffline.map((id) => offline.get(id, accountId).catch(() => null))); let shown = false; fromOffline.forEach((id, i) => { - const email = gen === generation ? copies[i] : null; + // A copy saved from Stalwart 1.0 before the inline parts were taken + // into `attachments` gets them here. + const copy = gen === generation ? copies[i] : null; + const email = copy ? withInlineBodyParts(copy) : null; if (!email) { missing.push(id); return; diff --git a/src/lib/email-inline-parts.ts b/src/lib/email-inline-parts.ts new file mode 100644 index 00000000..e9f0561b --- /dev/null +++ b/src/lib/email-inline-parts.ts @@ -0,0 +1,48 @@ +// Non-text parts that sit in a message's body lists but not in `attachments`. +// +// RFC 8621 §4.1.4 puts an inline image, audio or video part that sits +// directly in multipart/mixed into textBody and htmlBody only: it is meant to +// show at that place in the body, so it is not an attachment. Stalwart 1.0 +// follows that; 0.16 also listed such a part in `attachments`. The app saves +// a draft with an inline image exactly that way (an HTML body plus an inline +// part with a Content-ID), and everything that resolves `cid:` images or +// carries parts over (viewer, re-opened draft, forward, the draft's part +// blobs) reads `attachments`. So on 1.0 the image vanished from all of them. +// Taking those parts into the attachment list makes 1.0 read like 0.16. + +import type { Attachment, BodyPart, Email } from '../api/types'; + +const isTextPart = (part: BodyPart) => (part.type ?? '').toLowerCase().startsWith('text/'); + +// Body parts carry a partId; an attachment without one is matched by blob. +const partKey = (part: { partId?: string; blobId?: string }) => part.partId ?? `blob:${part.blobId ?? ''}`; + +/** + * The non-text parts of textBody/htmlBody that `attachments` lacks, in body + * order, deduped by partId. + */ +export function inlineBodyParts( + email: Pick, +): Attachment[] { + const seen = new Set((email.attachments ?? []).map((a) => partKey(a as Attachment & { partId?: string }))); + const out: Attachment[] = []; + for (const part of [...(email.htmlBody ?? []), ...(email.textBody ?? [])]) { + if (!part.blobId || isTextPart(part)) continue; + const key = partKey(part); + if (seen.has(key)) continue; + seen.add(key); + out.push({ ...part, blobId: part.blobId, type: part.type ?? 'application/octet-stream' }); + } + return out; +} + +/** + * The email with {@link inlineBodyParts} appended to its attachments. Returns + * the same object when there is nothing to add (0.16, or no such parts), so + * calling it twice is harmless. + */ +export function withInlineBodyParts>(email: T): T { + const extra = inlineBodyParts(email); + if (extra.length === 0) return email; + return { ...email, attachments: [...(email.attachments ?? []), ...extra] }; +}