From adf30bef406f3c38428f0dd87f1d2c6fae1f9448 Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Tue, 29 Sep 2026 18:30:25 +1300 Subject: [PATCH] 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 => ({