fix: keep inline images of drafts and sent mail on Stalwart 1.0

This commit is contained in:
Linus Rath
2026-10-29 01:53:57 +01:00
parent 24232bf2e9
commit d2bb479ac6
7 changed files with 180 additions and 10 deletions
@@ -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<typeof vi.fn>;
// 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, unknown>, 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: '<p><img src="cid:img1@live.test"></p>',
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']);
});
});
+10 -3
View File
@@ -267,9 +267,16 @@ live('live Stalwart', () => {
draftId = saved.id; draftId = saved.id;
state = rebaseDraftPartBlobs(state, sent, saved.attachments); state = rebaseDraftPartBlobs(state, sent, saved.attachments);
} }
// Without the rebase the seeded part blob is gone with D0. // Each save re-pointed the image at the new version's part (on 1.0
await expect(saveDraftVersion({ ...base, attachments: [img(seeded)] }, drafts)) // the read-back finds it in the body lists only).
.rejects.toThrow(/does not exist|blobNotFound/); 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]); const [last] = await getFullEmails([draftId]);
expect(last.attachments?.[0]?.cid).toContain('img1@live.test'); expect(last.attachments?.[0]?.cid).toContain('img1@live.test');
} finally { } finally {
+11 -6
View File
@@ -16,6 +16,7 @@ import { sanitizeDisplayName } from '../lib/rfc5322-mailbox';
import { generateMessageId, stripMessageIdBrackets } from '../lib/email-threading'; import { generateMessageId, stripMessageIdBrackets } from '../lib/email-threading';
import { buildMdnMessage, type MdnOptions } from '../lib/mdn'; import { buildMdnMessage, type MdnOptions } from '../lib/mdn';
import { hasTruncatedDisplayedBody } from '../lib/email-body'; import { hasTruncatedDisplayedBody } from '../lib/email-body';
import { withInlineBodyParts } from '../lib/email-inline-parts';
import { stalwartStateWentBack } from './stalwart-state'; import { stalwartStateWentBack } from './stalwart-state';
import { isTransientNetworkError } from '../lib/network-error'; 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'], ['Email/get', { accountId, ids: [id], ...FULL_BODY_ARGS }, '0'],
]); ]);
const body = requireMethodResult(res, '0', 'Email/get'); const body = requireMethodResult(res, '0', 'Email/get');
const email = (body.list as Email[] | undefined)?.[0]; const fetched = (body.list as Email[] | undefined)?.[0];
if (!email) throw new Error(`Email ${id} not found`); 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); await refetchTruncatedBodyValues([email], accountId);
return email; return email;
} }
@@ -972,7 +975,7 @@ export async function getFullEmailsWithState(
['Email/get', { accountId, ids: slice, ...FULL_BODY_ARGS }, '0'], ['Email/get', { accountId, ids: slice, ...FULL_BODY_ARGS }, '0'],
]); ]);
const body = requireMethodResult(res, '0', 'Email/get'); 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.notFound.push(...((body.notFound as string[] | undefined) ?? []));
out.state = body.state as string | undefined; out.state = body.state as string | undefined;
} }
@@ -1908,7 +1911,8 @@ export async function saveDraftVersion(
['Email/get', { ['Email/get', {
accountId, accountId,
ids: ['#draft'], 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, bodyProperties: DRAFT_PART_BODY_PROPERTIES,
}, '1'], }, '1'],
]); ]);
@@ -1920,8 +1924,9 @@ export async function saveDraftVersion(
let attachments: Attachment[] = []; let attachments: Attachment[] = [];
try { try {
const got = requireMethodResult(res, '1', 'Email/get'); const got = requireMethodResult(res, '1', 'Email/get');
const saved = (got.list as Array<{ id: string; attachments?: Attachment[] }> | undefined)?.find((e) => e.id === id); const saved = (got.list as Array<Pick<Email, 'id' | 'attachments' | 'textBody' | 'htmlBody'>> | undefined)
attachments = (saved?.attachments ?? []).filter((a) => !!a.blobId); ?.find((e) => e.id === id);
attachments = (saved ? withInlineBodyParts(saved).attachments ?? [] : []).filter((a) => !!a.blobId);
} catch (err) { } catch (err) {
console.warn('[email] could not read back the saved draft parts:', err); console.warn('[email] could not read back the saved draft parts:', err);
} }
@@ -184,6 +184,17 @@ describe('loading a message', () => {
expect((await useOfflineCacheStore.getState().get('e1'))?.keywords).toEqual({ $flagged: true }); 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 () => { it('batches what it holds and what it lacks into one request each', async () => {
fullGet.mockResolvedValueOnce(fullResponse([full('a'), full('b')], 's1')); fullGet.mockResolvedValueOnce(fullResponse([full('a'), full('b')], 's1'));
await loadDetails(['a', 'b']); await loadDetails(['a', 'b']);
@@ -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]);
});
});
+5 -1
View File
@@ -18,6 +18,7 @@ import { getEmailFlags, getFullEmailsWithState, getThreadsHeaders } from '../api
import { useOfflineCacheStore } from '../stores/offline-cache-store'; import { useOfflineCacheStore } from '../stores/offline-cache-store';
import { useSettingsStore } from '../stores/settings-store'; import { useSettingsStore } from '../stores/settings-store';
import { onStateChangeType } from './state-change-bus'; 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. */ /** What the list row the viewer was opened from says about the message. */
export type FlagsHint = Partial<Pick<Email, 'keywords' | 'mailboxIds'>>; export type FlagsHint = Partial<Pick<Email, 'keywords' | 'mailboxIds'>>;
@@ -378,7 +379,10 @@ function startLoads(
const copies = await Promise.all(fromOffline.map((id) => offline.get(id, accountId).catch(() => null))); const copies = await Promise.all(fromOffline.map((id) => offline.get(id, accountId).catch(() => null)));
let shown = false; let shown = false;
fromOffline.forEach((id, i) => { 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) { if (!email) {
missing.push(id); missing.push(id);
return; return;
+48
View File
@@ -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<Email, 'attachments' | 'textBody' | 'htmlBody'>,
): 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<T extends Pick<Email, 'attachments' | 'textBody' | 'htmlBody'>>(email: T): T {
const extra = inlineBodyParts(email);
if (extra.length === 0) return email;
return { ...email, attachments: [...(email.attachments ?? []), ...extra] };
}