diff --git a/lib/__tests__/jmap-recipient-mailboxes.test.ts b/lib/__tests__/jmap-recipient-mailboxes.test.ts new file mode 100644 index 000000000..1b0d2af88 --- /dev/null +++ b/lib/__tests__/jmap-recipient-mailboxes.test.ts @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; + +/** + * #672: to/cc/bcc reach the client as RFC 5322 mailbox strings. JMAP keeps the + * display name and the address in separate fields, so the mailbox has to be + * split before it goes on the wire - handing the whole string (or a quoted + * name, or a name that repeats the address) to the server makes it emit an + * invalid To header and derive a malformed envelope recipient from it. + */ + +function createClient(): JMAPClient { + const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); + Object.assign(client, { + apiUrl: 'https://jmap.example.com/api', + accountId: 'account-1', + username: 'user@example.com', + }); + return client; +} + +interface JMAPMethodCall { 0: string; 1: Record; 2: string } +interface CapturedRequest { using?: string[]; methodCalls: JMAPMethodCall[] } + +/** Mailbox/get โ†’ Identity/get โ†’ Email/set (+ EmailSubmission/set). */ +function mockFlow() { + const captured: CapturedRequest[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as CapturedRequest; + captured.push(body); + const method = body.methodCalls[0][0]; + + let payload: unknown; + if (method === 'Mailbox/get') { + payload = { + methodResponses: [[ + 'Mailbox/get', + { list: [{ id: 'mb-drafts', name: 'Drafts', role: 'drafts' }, { id: 'mb-sent', name: 'Sent', role: 'sent' }] }, + '0', + ]], + }; + } else if (method === 'Identity/get') { + payload = { + methodResponses: [['Identity/get', { list: [{ id: 'identity-1', email: 'user@example.com', mayDelete: false }] }, '0']], + }; + } else { + const create = body.methodCalls[0][1].create as Record; + payload = { + methodResponses: [ + ['Email/set', { created: { [Object.keys(create)[0]]: { id: 'email-1' } } }, '0'], + ['EmailSubmission/set', { created: { '1': { id: 'sub-1' } } }, '1'], + ], + }; + } + + return { ok: true, status: 200, text: () => Promise.resolve(JSON.stringify(payload)), json: () => Promise.resolve(payload) } as Response; + }); + return captured; +} + +/** The Email/set create object of the last captured request. */ +function createdEmail(captured: CapturedRequest[]): Record { + const setCall = captured[captured.length - 1].methodCalls[0]; + expect(setCall[0]).toBe('Email/set'); + return Object.values(setCall[1].create as Record>)[0]; +} + +describe('outgoing recipient mailboxes', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('splits "Name " into the JMAP name and email fields when sending', async () => { + const client = createClient(); + const captured = mockFlow(); + + await client.sendEmail( + ['Jane Doe '], + 'Subject', 'body', + ['"Doe, John" '], undefined, + 'identity-1', 'user@example.com', + ); + + const draft = createdEmail(captured); + expect(draft.to).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + // The quoting only exists to survive the composer's comma-separated + // string; the structured field takes the bare name. + expect(draft.cc).toEqual([{ name: 'Doe, John', email: 'john@example.com' }]); + }); + + it('drops an address the display name repeats instead of sending it as the name (#672)', async () => { + const client = createClient(); + const captured = mockFlow(); + + await client.sendEmail( + ['"Jane Doe " '], + 'Subject', 'body', + undefined, undefined, 'identity-1', 'user@example.com', + ); + + expect(createdEmail(captured).to).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + }); + + it('recovers the address when the mailbox lost its closing bracket (#672)', async () => { + const client = createClient(); + const captured = mockFlow(); + + await client.sendEmail( + ['janedoe { + const client = createClient(); + const captured = mockFlow(); + + await client.sendEmail( + ['jane.doe@example.com'], + 'Subject', 'body', + undefined, undefined, 'identity-1', 'user@example.com', + ); + + expect(createdEmail(captured).to).toEqual([{ email: 'jane.doe@example.com' }]); + }); + + it('splits the mailbox when saving a draft too (#672)', async () => { + const client = createClient(); + const captured = mockFlow(); + + await client.createDraft( + ['Jane Doe '], + 'Subject', 'body', + ['"Jane Doe " '], + ['bob@example.com'], + ); + + const draft = createdEmail(captured); + expect(draft.to).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + expect(draft.cc).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + expect(draft.bcc).toEqual([{ email: 'bob@example.com' }]); + }); +}); diff --git a/lib/__tests__/rfc5322-mailbox.test.ts b/lib/__tests__/rfc5322-mailbox.test.ts new file mode 100644 index 000000000..6bee1b31b --- /dev/null +++ b/lib/__tests__/rfc5322-mailbox.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect } from 'vitest'; +import { sanitizeDisplayName, splitMailbox, unquoteDisplayName } from '../rfc5322-mailbox'; + +describe('unquoteDisplayName', () => { + it('strips a surrounding quoted-string and its escapes', () => { + expect(unquoteDisplayName('"Doe, John"')).toBe('Doe, John'); + expect(unquoteDisplayName('"Say \\"hi\\""')).toBe('Say "hi"'); + }); + + it('leaves an unquoted name alone', () => { + expect(unquoteDisplayName(' Jane Doe ')).toBe('Jane Doe'); + expect(unquoteDisplayName('6" nails')).toBe('6" nails'); + }); +}); + +describe('sanitizeDisplayName', () => { + it('drops an address the name carries inline', () => { + expect(sanitizeDisplayName('Jane Doe ')).toBe('Jane Doe'); + expect(sanitizeDisplayName('"Jane Doe "')).toBe('Jane Doe'); + }); + + it('drops an inline address with an unbalanced bracket', () => { + expect(sanitizeDisplayName('janedoe { + expect(sanitizeDisplayName('Jane Doe')).toBe('Jane Doe'); + expect(sanitizeDisplayName(undefined)).toBe(''); + expect(sanitizeDisplayName(null)).toBe(''); + }); +}); + +describe('splitMailbox', () => { + it('splits a plain mailbox', () => { + expect(splitMailbox('Jane Doe ')).toEqual({ name: 'Jane Doe', email: 'jane@example.com' }); + }); + + it('passes a bare address through without a name', () => { + expect(splitMailbox(' jane@example.com ')).toEqual({ email: 'jane@example.com' }); + }); + + it('unquotes the display name', () => { + expect(splitMailbox('"Doe, John" ')).toEqual({ name: 'Doe, John', email: 'john@example.com' }); + }); + + it('collapses a display name that repeats the address (#672)', () => { + expect(splitMailbox('"Jane Doe " ')) + .toEqual({ name: 'Jane Doe', email: 'jane@example.com' }); + expect(splitMailbox('Jane Doe ')) + .toEqual({ name: 'Jane Doe', email: 'jane@example.com' }); + }); + + it('recovers the address from a mailbox missing its closing bracket (#672)', () => { + expect(splitMailbox('janedoe { + expect(splitMailbox('jane@example.com ')).toEqual({ email: 'jane@example.com' }); + }); + + it('leaves text that is not a mailbox untouched', () => { + expect(splitMailbox('not an address')).toEqual({ email: 'not an address' }); + }); +}); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 06857f11d..f77da7982 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -5,15 +5,18 @@ import { toWildcardQuery } from "./search-utils"; import { batched, itemsPerRequest } from "./request-limits"; import { debug } from "@/lib/debug"; import { normalizeCalendarEventLike } from "@/lib/calendar-event-normalization"; +import { sanitizeDisplayName, splitMailbox } from "@/lib/rfc5322-mailbox"; -/** Parse a recipient string that may be "Name " or bare "email" into { name?, email }. */ +/** + * Parse a recipient string that may be "Name " or bare "email" into + * { name?, email }. The display name is unquoted and stripped of any address + * it carries inline: JMAP takes the name and the address as separate fields, + * so leaving the quoting or a second copy of the address in there makes the + * server emit an invalid To/Cc mailbox, which it then re-parses into a + * malformed envelope recipient (#672). + */ function parseRecipientString(s: string): { name?: string; email: string } { - const trimmed = s.trim(); - const angleMatch = trimmed.match(/^(.+?)\s*<([^>]+)>$/); - if (angleMatch) { - return { name: angleMatch[1].trim(), email: angleMatch[2].trim() }; - } - return { email: trimmed }; + return splitMailbox(s); } /** @@ -533,8 +536,7 @@ function buildInCalendarFilter(calendarIds: string[]): Record { // whose display-name is invalid per RFC 5322 ยง3.4 and gets rejected by the // submission validator - the email then sits forever in Drafts. function sanitizeIdentityDisplayName(name: string | undefined | null): string { - if (!name) return ''; - return name.replace(/\s*<[^>]*>\s*$/, '').trim(); + return sanitizeDisplayName(name); } function normalizeEnvelopeRecipients(recipients?: Array): Array<{ email: string }> { @@ -2695,9 +2697,9 @@ export class JMAPClient implements IJMAPClient { interface EmailDraft { from: { name?: string; email: string }[]; - to: { email: string }[]; - cc?: { email: string }[]; - bcc?: { email: string }[]; + to: { name?: string; email: string }[]; + cc?: { name?: string; email: string }[]; + bcc?: { name?: string; email: string }[]; subject: string; keywords: Record; mailboxIds: Record; @@ -2710,9 +2712,12 @@ export class JMAPClient implements IJMAPClient { const sanitizedFromName = sanitizeIdentityDisplayName(fromName); const emailData: EmailDraft = { from: [{ ...(sanitizedFromName ? { name: sanitizedFromName } : {}), email: fromEmail || this.username }], - to: to.map(email => ({ email })), - cc: cc?.length ? cc.map(email => ({ email })) : undefined, - bcc: bcc?.length ? bcc.map(email => ({ email })) : undefined, + // "Name " must be split into the two JMAP fields: storing the whole + // mailbox as the address makes the server treat the display name as part + // of the addr-spec, and the draft goes out to `Name`). + * + * Address data reaches us from places that do not keep the display name and + * the address apart: vCard `FN` fields that hold the whole mailbox, Sent-folder + * headers, `Identity.name` on some servers. Handing such a string back to a + * server as a structured display name produces a doubled, invalid mailbox + * (`"Jane " `), which mail servers then re-parse into a + * broken envelope recipient (#672). These helpers reduce any of those forms to + * a clean name/address pair. + */ + +/** Strips a surrounding quoted-string (and its escapes) from a display name. */ +export function unquoteDisplayName(name: string): string { + const trimmed = name.trim(); + if (trimmed.length >= 2 && trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed.slice(1, -1).replace(/\\(["\\])/g, '$1').trim(); + } + return trimmed; +} + +/** + * Reduces a display name to just the name: unquotes it and drops an address it + * carries inline, whether or not the angle brackets are balanced + * (`Jane `, `Jane]*>?\s*$/, '')); +} + +/** The addr-spec a mailbox string carries, or '' when it has none. */ +function extractAddrSpec(value: string): string { + // Prefer the last angle-bracketed run - a display name that embeds an + // address puts an earlier, redundant copy in front of the real one. The + // closing bracket is optional so a truncated `Jane]*)>?\s*$/); + if (angle) return angle[1].trim(); + // No brackets: the whole string is the address, if it looks like one at all. + return value.includes('@') ? value : ''; +} + +/** + * Splits an RFC 5322 mailbox into its display name and addr-spec. Accepts the + * malformed shapes above; the returned name never contains an address and the + * returned email is always a bare addr-spec. `name` is omitted when there is + * no display name distinct from the address. + */ +export function splitMailbox(value: string): { name?: string; email: string } { + const trimmed = value.trim(); + const email = extractAddrSpec(trimmed); + if (!email) return { email: trimmed }; + + // Everything ahead of the address is the display name. + const nameSource = trimmed.slice(0, trimmed.lastIndexOf(email)).replace(/<\s*$/, ''); + const name = sanitizeDisplayName(nameSource); + return name && name !== email ? { name, email } : { email }; +} diff --git a/stores/__tests__/contact-store.test.ts b/stores/__tests__/contact-store.test.ts index d2c9020cf..755185555 100644 --- a/stores/__tests__/contact-store.test.ts +++ b/stores/__tests__/contact-store.test.ts @@ -297,6 +297,49 @@ describe('contact-store', () => { expect(results).toHaveLength(1); expect(results[0].name).toBe('Jane'); }); + + // #672: a flattened vCard import leaves the whole mailbox in the name (and + // sometimes in the address too). Suggesting that raw string composes a + // recipient the receiving server rejects. + it('should split a display name that carries the address', () => { + useContactStore.setState({ + contacts: [ + makeContact({ id: 'raw', name: { full: 'Jane Doe ' }, emails: { e0: { address: 'jane.doe@example.com' } } }), + ], + }); + const results = useContactStore.getState().getAutocomplete('jane'); + expect(results).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + }); + + it('should split a whole mailbox stored as the address', () => { + useContactStore.setState({ + contacts: [ + makeContact({ id: 'raw', name: { full: 'Jane Doe ' }, emails: { e0: { address: 'Jane Doe ' } } }), + ], + }); + const results = useContactStore.getState().getAutocomplete('jane'); + expect(results).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + }); + + it('should not surface a raw duplicate of an already parsed contact', () => { + useContactStore.setState({ + contacts: [ + makeContact({ id: 'clean', name: { components: [{ kind: 'given', value: 'Jane' }, { kind: 'surname', value: 'Doe' }], isOrdered: true }, emails: { e0: { address: 'jane.doe@example.com' } } }), + makeContact({ id: 'raw', name: { full: 'Jane Doe ' }, emails: { e0: { address: 'jane.doe@example.com' } } }), + ], + }); + const results = useContactStore.getState().getAutocomplete('jane'); + expect(results).toEqual([{ name: 'Jane Doe', email: 'jane.doe@example.com' }]); + }); + + it('should repair a recent recipient whose address lost its closing bracket', () => { + useContactStore.setState({ + contacts: [], + recentRecipients: [{ name: '', email: 'janedoe { diff --git a/stores/contact-store.ts b/stores/contact-store.ts index f5c92f48e..9243fe573 100644 --- a/stores/contact-store.ts +++ b/stores/contact-store.ts @@ -5,6 +5,48 @@ import type { IJMAPClient } from '@/lib/jmap/client-interface'; import { generateUUID } from '@/lib/utils'; import { debug } from '@/lib/debug'; import { getClientByLocalAccountId } from './client-registry'; +import { sanitizeDisplayName, splitMailbox } from '@/lib/rfc5322-mailbox'; + +/** One compose-autocomplete suggestion: a person, or a contact group. */ +type RecipientSuggestion = { name: string; email: string; group?: { id: string; memberCount: number } }; + +/** + * Reduces every suggestion to a clean display name plus a bare address and + * collapses the ones that resolve to the same address. + * + * Contacts whose stored name is really a whole mailbox ("Jane ", the + * usual result of a flattened vCard import) otherwise show up next to the + * properly parsed card as a second, raw-looking suggestion, and picking that + * one composes a malformed recipient the receiving server rejects (#672). + * Groups carry no address and pass through untouched. + */ +function normalizeSuggestions(results: RecipientSuggestion[]): RecipientSuggestion[] { + const out: RecipientSuggestion[] = []; + const indexByEmail = new Map(); + for (const r of results) { + if (r.group) { + out.push(r); + continue; + } + const mailbox = splitMailbox(r.email); + if (!mailbox.email) continue; + // The name loses any address it carries; when that leaves nothing, the one + // the address field itself carried ("Jane " stored as the address) + // stands in. + const name = sanitizeDisplayName(r.name) || mailbox.name || ''; + const suggestion = { name: name === mailbox.email ? '' : name, email: mailbox.email }; + const key = mailbox.email.toLowerCase(); + const seenAt = indexByEmail.get(key); + if (seenAt === undefined) { + indexByEmail.set(key, out.length); + out.push(suggestion); + } else if (!out[seenAt].name && suggestion.name) { + // Keep the first hit's position but take a display name it was missing. + out[seenAt].name = suggestion.name; + } + } + return out; +} /** One connected JMAP account for contact multi-account aggregation. */ export interface ContactAccountClient { @@ -590,7 +632,7 @@ export const useContactStore = create()( if (!query || query.length < 1) return []; const lower = query.toLowerCase(); - const results: Array<{ name: string; email: string; group?: { id: string; memberCount: number } }> = []; + const results: RecipientSuggestion[] = []; for (const contact of contacts) { if (contact.kind === 'group') { @@ -669,7 +711,7 @@ export const useContactStore = create()( } } - return results; + return normalizeSuggestions(results); }, getGroups: () => {