mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix(email): refetch body value when JMAP truncates the displayed part
Large HTML emails (e.g. image-only reports with a data:image src cut mid-tag by the 256 KB body cap) rendered completely blank because Email.bodyValues[partId].isTruncated was declared but never acted on. On truncation, issue one follow-up Email/get with a much larger cap for the parts actually shown, batched across a thread instead of one refetch per message. Closes #884
This commit is contained in:
@@ -0,0 +1,175 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { JMAPClient } from '../jmap/client';
|
||||
|
||||
// Minimal valid JMAP session response
|
||||
function makeSession(overrides?: Record<string, unknown>) {
|
||||
return {
|
||||
capabilities: { 'urn:ietf:params:jmap:core': {} },
|
||||
accounts: { 'acct-1': { name: 'test', isPersonal: true, accountCapabilities: {} } },
|
||||
primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1' },
|
||||
apiUrl: 'https://mail.example.com/jmap/api',
|
||||
downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
|
||||
uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
|
||||
eventSourceUrl: 'https://mail.example.com/jmap/eventsource',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function mockFetchResponse(status: number, body?: unknown): Response {
|
||||
return new Response(body ? JSON.stringify(body) : null, {
|
||||
status,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
describe('JMAPClient truncated body refetch (#884)', () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.spyOn(globalThis, 'fetch');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
async function createConnectedClient(): Promise<JMAPClient> {
|
||||
fetchSpy.mockResolvedValueOnce(mockFetchResponse(200, makeSession()));
|
||||
const client = new JMAPClient('https://mail.example.com', 'user@test.com', 'pass123');
|
||||
await client.connect();
|
||||
fetchSpy.mockReset();
|
||||
return client;
|
||||
}
|
||||
|
||||
function truncatedEmail() {
|
||||
return {
|
||||
id: 'email-1',
|
||||
threadId: 'thread-1',
|
||||
mailboxIds: { mb1: true },
|
||||
keywords: {},
|
||||
size: 600000,
|
||||
receivedAt: '2026-08-01T00:00:00Z',
|
||||
from: [{ email: 'a@example.com' }],
|
||||
to: [],
|
||||
subject: 'Big report',
|
||||
preview: '',
|
||||
htmlBody: [{ partId: 'html1', blobId: 'b1', size: 600000, type: 'text/html' }],
|
||||
textBody: [],
|
||||
bodyValues: {
|
||||
html1: { value: '<html><img src="data:image/jpeg;base64,AAAA', isTruncated: true },
|
||||
},
|
||||
attachments: [],
|
||||
};
|
||||
}
|
||||
|
||||
it('getEmail refetches with a larger cap when the displayed html part is truncated', async () => {
|
||||
const client = await createConnectedClient();
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [['Email/get', { list: [truncatedEmail()] }, '0']],
|
||||
}),
|
||||
);
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [
|
||||
[
|
||||
'Email/get',
|
||||
{
|
||||
list: [
|
||||
{
|
||||
id: 'email-1',
|
||||
bodyValues: { html1: { value: '<html><img src="data:image/jpeg;base64,AAAA...full" />', isTruncated: false } },
|
||||
},
|
||||
],
|
||||
},
|
||||
'0',
|
||||
],
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const email = await client.getEmail('email-1');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
const refetchBody = JSON.parse((fetchSpy.mock.calls[1]![1] as RequestInit).body as string);
|
||||
expect(refetchBody.methodCalls[0][1].maxBodyValueBytes).toBe(8_000_000);
|
||||
expect(refetchBody.methodCalls[0][1].ids).toEqual(['email-1']);
|
||||
expect(email?.bodyValues?.html1.value).toContain('full');
|
||||
expect(email?.bodyValues?.html1.isTruncated).toBe(false);
|
||||
});
|
||||
|
||||
it('getEmail does not refetch when the body is not truncated', async () => {
|
||||
const client = await createConnectedClient();
|
||||
const email = truncatedEmail();
|
||||
email.bodyValues.html1.isTruncated = false;
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, { methodResponses: [['Email/get', { list: [email] }, '0']] }),
|
||||
);
|
||||
|
||||
await client.getEmail('email-1');
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('getEmail keeps the truncated value if the refetch fails', async () => {
|
||||
const client = await createConnectedClient();
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [['Email/get', { list: [truncatedEmail()] }, '0']],
|
||||
}),
|
||||
);
|
||||
fetchSpy.mockRejectedValueOnce(new TypeError('Failed to fetch'));
|
||||
fetchSpy.mockRejectedValueOnce(new TypeError('Failed to fetch'));
|
||||
|
||||
const email = await client.getEmail('email-1');
|
||||
|
||||
expect(email?.bodyValues?.html1.isTruncated).toBe(true);
|
||||
expect(email?.bodyValues?.html1.value).toContain('AAAA');
|
||||
});
|
||||
|
||||
it('getThreadEmails batches truncated messages into a single follow-up Email/get', async () => {
|
||||
const client = await createConnectedClient();
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [['Thread/get', { list: [{ id: 'thread-1', emailIds: ['email-1', 'email-2'] }] }, '0']],
|
||||
}),
|
||||
);
|
||||
|
||||
const secondTruncated = { ...truncatedEmail(), id: 'email-2' };
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [['Email/get', { list: [truncatedEmail(), secondTruncated] }, '0']],
|
||||
}),
|
||||
);
|
||||
|
||||
fetchSpy.mockResolvedValueOnce(
|
||||
mockFetchResponse(200, {
|
||||
methodResponses: [
|
||||
[
|
||||
'Email/get',
|
||||
{
|
||||
list: [
|
||||
{ id: 'email-1', bodyValues: { html1: { value: 'full-1', isTruncated: false } } },
|
||||
{ id: 'email-2', bodyValues: { html1: { value: 'full-2', isTruncated: false } } },
|
||||
],
|
||||
},
|
||||
'0',
|
||||
],
|
||||
],
|
||||
}),
|
||||
);
|
||||
|
||||
const emails = await client.getThreadEmails('thread-1');
|
||||
|
||||
// Thread/get, Email/get, then exactly ONE batched refetch (not one per message)
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(3);
|
||||
const refetchBody = JSON.parse((fetchSpy.mock.calls[2]![1] as RequestInit).body as string);
|
||||
expect(refetchBody.methodCalls[0][1].ids.sort()).toEqual(['email-1', 'email-2']);
|
||||
expect(emails.find((e) => e.id === 'email-1')?.bodyValues?.html1.value).toBe('full-1');
|
||||
expect(emails.find((e) => e.id === 'email-2')?.bodyValues?.html1.value).toBe('full-2');
|
||||
});
|
||||
});
|
||||
+66
-1
@@ -17,6 +17,12 @@ import { decodeFileNodeName } from "./filenode-name";
|
||||
import { getEffectiveTimeZone } from "@/lib/timezone";
|
||||
import { buildEmailSort, compareEmails, hasKeywordLevels, type KeywordSortPolarity, type SortLevel } from "@/lib/message-list-order";
|
||||
|
||||
// Cap for the follow-up Email/get issued when a displayed body part comes
|
||||
// back truncated at the normal 256000-byte limit (see refetchTruncatedBodyValues
|
||||
// / #884). Bounded well above the default so it stays rare in practice while
|
||||
// still guarding against unbounded memory use on a malicious/oversized body.
|
||||
const TRUNCATED_BODY_REFETCH_MAX_BYTES = 8_000_000;
|
||||
|
||||
// Names of nodes created over WebDAV come back percent-encoded from
|
||||
// FileNode/get (see decodeFileNodeName / #869). Normalize at the boundary so
|
||||
// every consumer (browser, sidebar, breadcrumbs, path resolution) sees the
|
||||
@@ -1961,6 +1967,8 @@ export class JMAPClient implements IJMAPClient {
|
||||
namespaceMailboxIds([email], accountId);
|
||||
}
|
||||
|
||||
await this.refetchTruncatedBodyValues([email], targetAccountId);
|
||||
|
||||
if (email.headers) {
|
||||
await this.parseEmailHeaders(email);
|
||||
}
|
||||
@@ -1972,6 +1980,56 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
}
|
||||
|
||||
// The 256000-byte body cap keeps list/thread fetches fast for the common
|
||||
// case, but truncation lands mid-tag on large HTML bodies (e.g. a data:image
|
||||
// src cut in half), which drops the whole element and can render blank. Only
|
||||
// the parts actually shown to the user are checked — fetchAllBodyValues also
|
||||
// returns truncated text from attachment parts, which must not trigger a
|
||||
// refetch. See #884.
|
||||
private hasTruncatedDisplayedBody(email: Email): boolean {
|
||||
const bodyValues = email.bodyValues;
|
||||
if (!bodyValues) return false;
|
||||
const htmlPartId = email.htmlBody?.[0]?.partId;
|
||||
const textPartId = email.textBody?.[0]?.partId;
|
||||
return Boolean(
|
||||
(htmlPartId && bodyValues[htmlPartId]?.isTruncated) ||
|
||||
(textPartId && bodyValues[textPartId]?.isTruncated)
|
||||
);
|
||||
}
|
||||
|
||||
private async refetchTruncatedBodyValues(emails: Email[], accountId: string): Promise<void> {
|
||||
const truncated = emails.filter((email) => this.hasTruncatedDisplayedBody(email));
|
||||
if (truncated.length === 0) return;
|
||||
|
||||
try {
|
||||
const refetchedById = new Map<string, Email['bodyValues']>();
|
||||
for (const batchIds of batched(truncated.map((email) => email.id), this.getMaxObjectsInGet())) {
|
||||
const response = await this.request([
|
||||
["Email/get", {
|
||||
accountId,
|
||||
ids: batchIds,
|
||||
properties: ["id", "bodyValues"],
|
||||
fetchTextBodyValues: true,
|
||||
fetchHTMLBodyValues: true,
|
||||
fetchAllBodyValues: true,
|
||||
maxBodyValueBytes: TRUNCATED_BODY_REFETCH_MAX_BYTES,
|
||||
}, "0"],
|
||||
]);
|
||||
for (const refetched of (response.methodResponses?.[0]?.[1]?.list || []) as Email[]) {
|
||||
refetchedById.set(refetched.id, refetched.bodyValues);
|
||||
}
|
||||
}
|
||||
|
||||
for (const email of truncated) {
|
||||
const bodyValues = refetchedById.get(email.id);
|
||||
if (bodyValues) email.bodyValues = bodyValues;
|
||||
}
|
||||
} catch (error) {
|
||||
// Degrade to the already-truncated values - no new error path for callers.
|
||||
console.error('Failed to refetch truncated body values:', error);
|
||||
}
|
||||
}
|
||||
|
||||
private async parseEmailHeaders(email: Email): Promise<void> {
|
||||
const { parseAuthenticationResults, parseSpamScore, parseSpamLLM } = await import('@/lib/email-headers');
|
||||
|
||||
@@ -2988,6 +3046,11 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
|
||||
if (emails.length > 0) {
|
||||
// One batched refetch for the whole thread rather than per-message,
|
||||
// to avoid an N+1 request pattern on threads with several oversized
|
||||
// messages (#884).
|
||||
await this.refetchTruncatedBodyValues(emails, targetAccountId);
|
||||
|
||||
if (accountId && accountId !== this.accountId) {
|
||||
namespaceMailboxIds(emails, accountId);
|
||||
}
|
||||
@@ -8376,7 +8439,9 @@ export class JMAPClient implements IJMAPClient {
|
||||
maxBodyValueBytes: 256000,
|
||||
}, '0'],
|
||||
]);
|
||||
for (const email of ((emailResponse.methodResponses?.[0]?.[1]?.list ?? []) as Email[])) {
|
||||
const fetchedEmails = (emailResponse.methodResponses?.[0]?.[1]?.list ?? []) as Email[];
|
||||
await this.refetchTruncatedBodyValues(fetchedEmails, accountId);
|
||||
for (const email of fetchedEmails) {
|
||||
emailById.set(email.id, email);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user