mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: keep inline images of drafts and sent mail on Stalwart 1.0
This commit is contained in:
@@ -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']);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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
@@ -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]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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;
|
||||||
|
|||||||
@@ -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] };
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user