From 888b606fcb939272154b62237fb880a8440c426c Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Tue, 29 Sep 2026 18:17:19 +1300 Subject: [PATCH 1/3] fix(email-composer): normalize bracketed content-ids for inline reply/forward images :bug: Attachment.cid may still carry RFC angle brackets while body `src="cid:...` references never do. The viewers already stripped them before matching; the reply/forward inline-image hydration didn't, so the match silently failed and the image stayed a blank placeholder in compose and never made it into the outgoing message. Extracted the stripping into a shared normalizeCid() and applied it everywhere cids get compared. --- .../composer-inline-image-cid.test.tsx | 208 ++++++++++++++++++ components/email/email-composer.tsx | 15 +- components/email/email-viewer.tsx | 6 +- components/email/thread-conversation-view.tsx | 4 +- lib/attachment-visibility.ts | 13 +- 5 files changed, 234 insertions(+), 12 deletions(-) create mode 100644 components/email/__tests__/composer-inline-image-cid.test.tsx diff --git a/components/email/__tests__/composer-inline-image-cid.test.tsx b/components/email/__tests__/composer-inline-image-cid.test.tsx new file mode 100644 index 000000000..150a4f3f9 --- /dev/null +++ b/components/email/__tests__/composer-inline-image-cid.test.tsx @@ -0,0 +1,208 @@ +import { render, screen, act, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { EmailComposer } from '../email-composer'; +import { useAuthStore } from '@/stores/auth-store'; + +// ─── Heavy component mocks (mirrors composer-draft-attachments.test.tsx) ────── + +vi.mock('@/components/email/rich-text-editor', () => ({ + RichTextEditor: () => React.createElement('div', { 'data-testid': 'rich-text-editor' }), +})); + +vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null })); +vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null })); +vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null })); +vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null })); +vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null })); +vi.mock('@/hooks/use-focus-trap', () => ({ useFocusTrap: () => ({ current: null }) })); +vi.mock('@/hooks/use-pro-multi-account-identities', () => ({ + useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }), + stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }), +})); + +vi.mock('@/stores/auth-store', () => { + const state = { + client: null, + identities: [], + primaryIdentity: null, + isAuthenticated: false, + isDemoMode: false, + activeAccountId: null, + connectionLost: false, + getClientForAccount: () => undefined, + getAllConnectedClients: () => new Map(), + syncIdentities: () => {}, + refreshIdentities: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAuthStore: hook }; +}); + +vi.mock('@/stores/identity-store', () => { + const state = { + identities: [{ id: 'id-me', email: 'me@example.com', name: 'Me' }], + defaultIdentityId: 'id-me', + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useIdentityStore: hook }; +}); + +vi.mock('@/stores/account-store', () => { + const state = { accounts: [], getAccountById: () => undefined }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useAccountStore: hook }; +}); + +vi.mock('@/stores/email-store', () => { + const state = { draftSaveEnabled: false, sendRawEmail: async () => ({ sent: true }) }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useEmailStore: hook }; +}); + +vi.mock('@/stores/settings-store', () => { + const state = { + timeFormat: '24h', + plainTextMode: false, + subAddressDelimiter: '+', + autoSelectReplyIdentity: true, + attachmentReminderEnabled: false, + attachmentReminderKeywords: [], + emptySubjectWarningEnabled: true, + sendDelaySeconds: 0, + signaturePosition: 'above_quote', + signatureSeparatorEnabled: false, + requestReadReceiptDefault: false, + addTrustedSender: () => {}, + trustedSendersAddressBook: null, + updateSetting: () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useSettingsStore: hook }; +}); + +vi.mock('@/stores/contact-store', () => { + const state = { + contacts: [], + getAutocomplete: async () => [], + addToTrustedSendersBook: async () => {}, + }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useContactStore: hook }; +}); + +vi.mock('@/stores/template-store', () => { + const state = { templates: [], addTemplate: async () => {} }; + const hook = (sel?: (s: typeof state) => unknown) => + typeof sel === 'function' ? sel(state) : state; + hook.getState = () => state; + hook.setState = (p: Partial) => Object.assign(state, p); + return { useTemplateStore: hook }; +}); + +vi.mock('@/stores/toast-store', () => ({ + toast: { info: () => {}, error: () => {}, success: () => {} }, +})); + +vi.mock('@/lib/plugin-hooks', () => ({ + emailHooks: { + onComposerOpen: { call: async () => [] }, + onRecipientChange: { call: async () => [] }, + getRecipientSuggestions: { call: async () => [] }, + onRecipientChipsChange: { transform: async (chips: unknown) => chips }, + onDraftChange: { emit: () => {} }, + onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft }, + onBeforeEmailSend: { intercept: async () => true }, + onComposeSend: { intercept: async () => true }, + onTransformOutgoingEmail: { transform: async (email: unknown) => email }, + }, + contactHooks: { + search: { call: async () => [] }, + onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial }, + }, +})); + +vi.mock('@/lib/email-sanitization', () => ({ + sanitizeSignatureHtml: (v: string) => v, + sanitizeEmailHtml: (v: string) => v, + parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'), +})); + +vi.mock('@/lib/email-threading', () => ({ + computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }), +})); +vi.mock('@/lib/signature-utils', () => ({ + appendPlainTextSignature: (body: string) => body, + getPlainTextSignature: () => '', + plainTextBodyHasSignature: () => false, + plainTextBodyWithoutSignature: (body: string) => body, +})); +vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' })); +vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } })); +vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s })); + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +/** + * Senders may declare `Content-ID: ` (angle brackets kept) + * while the body always references it bracket-free as `src="cid:logo123@sender"`. + * The viewers already normalize both sides before matching; the reply/forward + * hydration path didn't, so any such image silently stayed a blank placeholder + * in the composer and was dropped from the outgoing message on send. + */ +describe('reply/forward inline image cid normalization', () => { + afterEach(() => { + useAuthStore.setState({ client: null }); + vi.clearAllMocks(); + }); + + it('hydrates an inline image whose attachment cid still carries angle brackets', async () => { + const fetchBlobArrayBuffer = vi.fn().mockResolvedValue( + new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]).buffer, // PNG magic + ); + useAuthStore.setState({ + client: { + fetchBlobArrayBuffer, + hasDelayedSend: () => false, + getMaxDelayedSend: () => 0, + } as never, + }); + + render( + Hi

', + quoteHeaderHtml: 'On X, Sender wrote:', + attachments: [ + { blobId: 'blob-1', name: 'logo.png', type: 'image/png', size: 100, cid: '' }, + ], + }} + onClose={vi.fn()} + /> + ); + + await waitFor(() => expect(fetchBlobArrayBuffer).toHaveBeenCalledTimes(1)); + expect(fetchBlobArrayBuffer).toHaveBeenCalledWith('blob-1', 'logo.png', 'image/png'); + }); +}); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 4a234a895..c0875ca40 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -65,6 +65,7 @@ import { ICON_MAP, } from "@/lib/email-composer-utils"; import { isValidEmail } from "@/lib/validation"; +import { normalizeCid } from "@/lib/attachment-visibility"; import { RichTextEditor } from "@/components/email/rich-text-editor"; import type { Editor } from "@tiptap/react"; import { htmlToPlainText as htmlToPlainTextShared } from "@/lib/html-to-text"; @@ -910,15 +911,20 @@ export function EmailComposer({ // lenient (email-viewer.tsx: `att.cid && att.blobId`), so reading and // replying now agree. The real type is sniffed from the bytes below. const embeddedCids = collectInlineImageCids(body); - const inlineAtts = replyTo.attachments.filter((att) => - att.cid && att.blobId && embeddedCids.has(att.cid) - ); + // Attachment cids may still carry the RFC angle brackets (e.g. + // ``) while `src="cid:logo123@sender"` in the body never + // does - normalize here the same way email-viewer.tsx and + // thread-conversation-view.tsx do, so the comparisons below actually + // match (#912: inline images silently missing from every reply/forward). + const inlineAtts = replyTo.attachments + .filter((att) => att.cid && att.blobId) + .map((att) => ({ ...att, cid: normalizeCid(att.cid!) })) + .filter((att) => embeddedCids.has(att.cid)); if (inlineAtts.length === 0) return; // Seed the ref synchronously so a fast Send still attaches the right blobs // even if the FileReader work below hasn't resolved yet. for (const att of inlineAtts) { - if (!att.cid) continue; if (inlineImagesRef.current.some((e) => e.cid === att.cid)) continue; inlineImagesRef.current.push({ cid: att.cid, @@ -934,7 +940,6 @@ export function EmailComposer({ (async () => { const updates = new Map(); for (const att of inlineAtts) { - if (!att.cid) continue; try { const buffer = await composerClient.fetchBlobArrayBuffer( att.blobId, diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index ceb79774c..7d7f308c3 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -6,7 +6,7 @@ import { emailExportFilename, attachmentDownloadFilename, attachmentsBundleFilen import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import"; import { applyNewTabToAnchor, escapeHtml, isOpenableLinkHref, plainTextToSafeHtml, sanitizeEmailBodyForIframe, sanitizeEmailHtml, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization"; import { getRenderableHtmlBody } from "@/lib/email-body-selection"; -import { collectReferencedCids, isEmbeddedInBody } from "@/lib/attachment-visibility"; +import { collectReferencedCids, isEmbeddedInBody, normalizeCid } from "@/lib/attachment-visibility"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { fitEmailBodyWidth } from "@/lib/email-fit-width"; import { withBasePath } from "@/lib/browser-navigation"; @@ -1546,7 +1546,7 @@ export function EmailViewer({ decryptedCidAttachments.forEach((att) => { const bytes = getAttachmentContentBytes(att); if (!bytes) return; - const cidValue = att.contentId!.replace(/^<|>$/g, ''); + const cidValue = normalizeCid(att.contentId!); const buffer = bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer; // Sender-typed part: never let a script-bearing MIME type reach a // blob: URL in our origin (GHSA-xvjh-v9c6-qcvc). @@ -1582,7 +1582,7 @@ export function EmailViewer({ async function fetchCidBlobs() { const urls: Record = {}; await Promise.all(cidAttachments.map(async (att) => { - const cidValue = att.cid!.replace(/^<|>$/g, ''); + const cidValue = normalizeCid(att.cid!); try { // The download URL echoes the sender-declared Content-Type back as // the Blob type; re-type anything that could execute as our origin diff --git a/components/email/thread-conversation-view.tsx b/components/email/thread-conversation-view.tsx index 82b8a49e6..f53ac8536 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -5,7 +5,7 @@ import DOMPurify from "dompurify"; import { Email, ThreadGroup } from "@/lib/jmap/types"; import { EMAIL_SANITIZE_CONFIG, collapseBlockedImageContainers, plainTextToSafeHtml, restrictDataUriResourcesOnNode, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization"; import { getRenderableHtmlBody } from "@/lib/email-body-selection"; -import { collectReferencedCids, isEmbeddedInBody } from "@/lib/attachment-visibility"; +import { collectReferencedCids, isEmbeddedInBody, normalizeCid } from "@/lib/attachment-visibility"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { fitEmailBodyWidth } from "@/lib/email-fit-width"; import { transformInlineStyles, transformColorForDarkMode, transformBgColorForDarkMode } from "@/lib/color-transform"; @@ -299,7 +299,7 @@ function EmailCard({ async function fetchCidBlobs() { const urls: Record = {}; await Promise.all(cidAttachments.map(async (att) => { - const cidValue = att.cid!.replace(/^<|>$/g, ''); + const cidValue = normalizeCid(att.cid!); try { // Re-type sender-declared script-bearing parts before they become a // blob: URL in our origin (GHSA-xvjh-v9c6-qcvc). diff --git a/lib/attachment-visibility.ts b/lib/attachment-visibility.ts index e20044777..85fc8e746 100644 --- a/lib/attachment-visibility.ts +++ b/lib/attachment-visibility.ts @@ -21,6 +21,16 @@ const CID_REFERENCE = /\bcid:([^"'\s)]+)/gi; const EMPTY: ReadonlySet = new Set(); +/** + * Strips the RFC angle brackets a Content-ID header may still carry (e.g. + * ``). Body `cid:` references never have them, so every + * comparison between an attachment's `cid`/`contentId` and a body reference + * must normalize through this first. + */ +export function normalizeCid(cid: string): string { + return cid.replace(/^<|>$/g, ''); +} + /** * Content-IDs a raw HTML body references via `cid:` URLs (img src, table * background, style url(...) - anything), exactly as written. `null` (the @@ -65,6 +75,5 @@ export function isEmbeddedInBody( if (att.disposition === 'attachment') return false; const genericType = !type || type === 'application/octet-stream'; if (!isImage && !genericType) return false; - // The Content-ID may still carry its angle brackets; body references never do. - return referencedCids.has(att.cid.replace(/^<|>$/g, '')); + return referencedCids.has(normalizeCid(att.cid)); } From 115c66ec8dc820e39e1ae147671ee784c18b9265 Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Tue, 29 Sep 2026 18:18:01 +1300 Subject: [PATCH 2/3] chore(email-composer): drop unused test imports flagged by lint :broom: --- components/email/__tests__/composer-inline-image-cid.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/email/__tests__/composer-inline-image-cid.test.tsx b/components/email/__tests__/composer-inline-image-cid.test.tsx index 150a4f3f9..4bc7c97ae 100644 --- a/components/email/__tests__/composer-inline-image-cid.test.tsx +++ b/components/email/__tests__/composer-inline-image-cid.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, act, waitFor } from '@testing-library/react'; +import { render, waitFor } from '@testing-library/react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import React from 'react'; import { EmailComposer } from '../email-composer'; From adf30bef406f3c38428f0dd87f1d2c6fae1f9448 Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Tue, 29 Sep 2026 18:30:25 +1300 Subject: [PATCH 3/3] fix(email-composer): normalize forwarded inline-image cids to prevent duplicate blobs :bug: The forward attachment-list filter compared the raw (possibly bracketed) Content-ID against the body's bracket-free cid: references, so a bracketed cid with no inline disposition (Foxmail's octet-stream case) slipped through as a regular attachment chip *and* got re-attached as the matched inline part, sending the same blob twice. Normalize before that comparison too, and strengthen the regression test to exercise Send and assert on the actual outgoing htmlBody/attachments for both the reply and forward paths. --- .../composer-inline-image-cid.test.tsx | 83 +++++++++++++++++-- components/email/email-composer.tsx | 5 +- 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/components/email/__tests__/composer-inline-image-cid.test.tsx b/components/email/__tests__/composer-inline-image-cid.test.tsx index 4bc7c97ae..52d4b62ee 100644 --- a/components/email/__tests__/composer-inline-image-cid.test.tsx +++ b/components/email/__tests__/composer-inline-image-cid.test.tsx @@ -1,4 +1,4 @@ -import { render, waitFor } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import React from 'react'; import { EmailComposer } from '../email-composer'; @@ -174,10 +174,10 @@ describe('reply/forward inline image cid normalization', () => { vi.clearAllMocks(); }); - it('hydrates an inline image whose attachment cid still carries angle brackets', async () => { - const fetchBlobArrayBuffer = vi.fn().mockResolvedValue( - new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]).buffer, // PNG magic - ); + const PNG_MAGIC = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]).buffer; + + function mockClient() { + const fetchBlobArrayBuffer = vi.fn().mockResolvedValue(PNG_MAGIC); useAuthStore.setState({ client: { fetchBlobArrayBuffer, @@ -185,10 +185,17 @@ describe('reply/forward inline image cid normalization', () => { getMaxDelayedSend: () => 0, } as never, }); + return { fetchBlobArrayBuffer }; + } + + it('sends a reply with the image inlined by cid, not duplicated as a regular attachment', async () => { + const { fetchBlobArrayBuffer } = mockClient(); + const onSend = vi.fn(); render( { /> ); - await waitFor(() => expect(fetchBlobArrayBuffer).toHaveBeenCalledTimes(1)); - expect(fetchBlobArrayBuffer).toHaveBeenCalledWith('blob-1', 'logo.png', 'image/png'); + // Hydration: the placeholder gets swapped for the fetched blob's data URL. + await waitFor(() => expect(fetchBlobArrayBuffer).toHaveBeenCalledWith('blob-1', 'logo.png', 'image/png')); + + // Reply auto-fills "to" from replyTo.from, so Send is reachable immediately. + fireEvent.click(screen.getAllByTestId('composer-send')[0]); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + + const sent = onSend.mock.calls[0][0] as { htmlBody?: string; attachments?: Array<{ cid?: string; blobId: string; disposition?: string }> }; + // The data URL must have been rewritten back to a cid: reference for sending. + expect(sent.htmlBody).toContain('cid:logo123@sender'); + expect(sent.htmlBody).not.toContain('data:image'); + // Exactly one attachment for the one image - a bracket-mismatch used to + // mean it was never matched at all; matching on both regular AND inline + // paths would instead send the same blob twice (see forward test below). + expect(sent.attachments).toEqual([ + expect.objectContaining({ blobId: 'blob-1', cid: 'logo123@sender', disposition: 'inline' }), + ]); + }); + + it('does not duplicate a forwarded inline image (declared octet-stream, no disposition) as a regular attachment', async () => { + const { fetchBlobArrayBuffer } = mockClient(); + const onSend = vi.fn(); + + render( + Hi

', + quoteHeaderHtml: 'Forwarded message', + // Foxmail-style part (#543): no inline disposition, generic type - + // only the cid: body reference marks it as embedded. + attachments: [ + { blobId: 'blob-1', name: 'logo.png', type: 'application/octet-stream', size: 100, cid: '' }, + ], + }} + onClose={vi.fn()} + /> + ); + + await waitFor(() => expect(fetchBlobArrayBuffer).toHaveBeenCalledWith('blob-1', 'logo.png', 'application/octet-stream')); + + // The forward attachment-list filter must have recognized blob-1 as + // embedded and kept it out of the regular attachment chips. + expect(screen.queryByText('logo.png')).not.toBeInTheDocument(); + + // Forward doesn't auto-fill "to" - add a recipient so Send is reachable. + // A trailing delimiter is needed: a bare single-address paste is left as + // uncommitted input text rather than immediately chipped. + fireEvent.paste(screen.getByPlaceholderText('to_placeholder'), { + clipboardData: { getData: () => 'bob@example.com;' }, + }); + + fireEvent.click(screen.getAllByTestId('composer-send')[0]); + await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1)); + + const sent = onSend.mock.calls[0][0] as { attachments?: Array<{ cid?: string; blobId: string }> }; + // Same blobId must appear exactly once, as the re-typed inline part - + // never also as a plain attachment (that would send it twice). + expect(sent.attachments).toEqual([ + expect.objectContaining({ blobId: 'blob-1', cid: 'logo123@sender' }), + ]); }); }); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index c0875ca40..a9c781fb3 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -610,9 +610,10 @@ export function EmailComposer({ const embeddedCids = collectInlineImageCids(body); return replyTo.attachments // Skip inline cid-referenced images - they're embedded in the forwarded HTML body - // (matches the viewer's hideInlineImageAttachments logic). + // (matches the viewer's hideInlineImageAttachments logic). att.cid may still + // carry angle brackets while embeddedCids (parsed from the body) never does. .filter(att => !(att.cid && ( - embeddedCids.has(att.cid) || + embeddedCids.has(normalizeCid(att.cid)) || (att.disposition === 'inline' && (att.type || '').startsWith('image/')) ))) .map(att => ({