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..52d4b62ee --- /dev/null +++ b/components/email/__tests__/composer-inline-image-cid.test.tsx @@ -0,0 +1,277 @@ +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'; +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(); + }); + + 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, + hasDelayedSend: () => false, + 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( + Hi

', + quoteHeaderHtml: 'On X, Sender wrote:', + attachments: [ + { blobId: 'blob-1', name: 'logo.png', type: 'image/png', size: 100, cid: '' }, + ], + }} + onClose={vi.fn()} + /> + ); + + // 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 8353eb3d3..116a7639d 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -121,6 +121,7 @@ import type { ComposePrefill } from "@/lib/plugin-compose-gate"; import { composerContextMode, composerMounted, composerUnmounted, draftSaved } from "@/lib/plugin-composer-hooks"; import { isValidEmail } from "@/lib/validation"; import { buildMentionCandidates } from "@/lib/recipient-mentions"; +import { normalizeCid } from "@/lib/attachment-visibility"; import { RichTextEditor } from "@/components/email/rich-text-editor"; import { isRecipientMentionActive } from "@/components/email/recipient-mention"; import type { Editor } from "@tiptap/react"; @@ -867,9 +868,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 => ({ @@ -1302,15 +1304,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, @@ -1327,7 +1334,6 @@ export function EmailComposer({ (async () => { const updates = new Map(); for (const att of inlineAtts) { - if (!att.cid) continue; try { const buffer = await originClient.fetchBlobArrayBuffer( att.blobId, diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 4a12b219a..dba3d4ac4 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, emailIframeCsp, escapeHtml, isExternalResourceUrl, 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 { DARK_PENDING_CLASS, applyEmailDarkMode, emailColorScheme, emailDarkModeCss, emailHasNativeDarkMode as htmlHasNativeDarkMode, resolveColorSchemeMedia } from "@/lib/email-dark-mode"; import { fitEmailBodyWidth } from "@/lib/email-fit-width"; @@ -2112,7 +2112,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). @@ -2148,7 +2148,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 4e58a136a..73ec423c1 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -8,7 +8,7 @@ import { emailDisplayDate } from "@/lib/email-date"; import { getRenderableHtmlBody } from "@/lib/email-body-selection"; import { findVerificationCode, verificationCodeBodyText } from "@/lib/verification-code"; import { VerificationCodeChip } from "./verification-code-chip"; -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 { DARK_PENDING_CLASS, applyEmailDarkMode, emailDarkModeCss } from "@/lib/email-dark-mode"; @@ -547,7 +547,7 @@ export function ConversationEmailBody({ 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)); }