fix: split "Name <addr>" recipients into the JMAP name and email fields #672

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<string, unknown>; 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<string, unknown>;
+      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<string, unknown> {
+  const setCall = captured[captured.length - 1].methodCalls[0];
+  expect(setCall[0]).toBe('Email/set');
+  return Object.values(setCall[1].create as Record<string, Record<string, unknown>>)[0];
+}
+
+describe('outgoing recipient mailboxes', () => {
+  beforeEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('splits "Name <addr>" into the JMAP name and email fields when sending', async () => {
+    const client = createClient();
+    const captured = mockFlow();
+
+    await client.sendEmail(
+      ['Jane Doe <jane.doe@example.com>'],
+      'Subject', 'body',
+      ['"Doe, John" <john@example.com>'], 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 <jane.doe@example.com>" <jane.doe@example.com>'],
+      '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<jane.doe@example.com'],
+      'Subject', 'body',
+      undefined, undefined, 'identity-1', 'user@example.com',
+    );
+
+    expect(createdEmail(captured).to).toEqual([{ name: 'janedoe', email: 'jane.doe@example.com' }]);
+  });
+
+  it('keeps a bare address without a display name', async () => {
+    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 <jane.doe@example.com>'],
+      'Subject', 'body',
+      ['"Jane Doe <jane.doe@example.com>" <jane.doe@example.com>'],
+      ['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' }]);
+  });
+});

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 <jane@example.com>')).toBe('Jane Doe');
+    expect(sanitizeDisplayName('"Jane Doe <jane@example.com>"')).toBe('Jane Doe');
+  });
+
+  it('drops an inline address with an unbalanced bracket', () => {
+    expect(sanitizeDisplayName('janedoe<jane@example.com')).toBe('janedoe');
+  });
+
+  it('keeps a plain name and tolerates nothing', () => {
+    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 <jane@example.com>')).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" <john@example.com>')).toEqual({ name: 'Doe, John', email: 'john@example.com' });
+  });
+
+  it('collapses a display name that repeats the address (#672)', () => {
+    expect(splitMailbox('"Jane Doe <jane@example.com>" <jane@example.com>'))
+      .toEqual({ name: 'Jane Doe', email: 'jane@example.com' });
+    expect(splitMailbox('Jane Doe <jane@example.com> <jane@example.com>'))
+      .toEqual({ name: 'Jane Doe', email: 'jane@example.com' });
+  });
+
+  it('recovers the address from a mailbox missing its closing bracket (#672)', () => {
+    expect(splitMailbox('janedoe<jane.doe@example.com'))
+      .toEqual({ name: 'janedoe', email: 'jane.doe@example.com' });
+  });
+
+  it('omits a name that is just the address', () => {
+    expect(splitMailbox('jane@example.com <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' });
+  });
+});

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 <email>" or bare "email" into { name?, email }. */
+/**
+ * Parse a recipient string that may be "Name <email>" 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<string, unknown> {
 // 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<string | EmailAddress>): 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<string, boolean>;
       mailboxIds: Record<string, boolean>;
@@ -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 <addr>" 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<addr` (#672).
+      to: to.map(parseRecipientString),
+      cc: cc?.length ? cc.map(parseRecipientString) : undefined,
+      bcc: bcc?.length ? bcc.map(parseRecipientString) : undefined,
       subject,
       keywords: { "$seen": true, "$draft": true },
       mailboxIds: { [draftsMailbox.id]: true },

lib/rfc5322-mailbox.ts

@@ -0,0 +1,58 @@
+/**
+ * Parsing helpers for RFC 5322 mailboxes (`Display Name <addr@example.com>`).
+ *
+ * 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 <j@x.com>" <j@x.com>`), 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 <j@x.com>`, `Jane<j@x.com` → `Jane`).
+ */
+export function sanitizeDisplayName(name: string | undefined | null): string {
+  if (!name) return '';
+  return unquoteDisplayName(unquoteDisplayName(name).replace(/\s*<[^<>]*>?\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<j@x.com` still resolves.
+  const angle = value.match(/<([^<>]*)>?\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 };
+}

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 <jane.doe@example.com>' }, 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 <jane.doe@example.com>' }, emails: { e0: { address: 'Jane Doe <jane.doe@example.com>' } } }),
+        ],
+      });
+      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 <jane.doe@example.com>' }, 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<jane.doe@example.com' }],
+      });
+      const results = useContactStore.getState().getAutocomplete('jane');
+      expect(results).toEqual([{ name: 'janedoe', email: 'jane.doe@example.com' }]);
+    });
   });
 
   describe('getGroups', () => {

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 <j@x.com>", 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<string, number>();
+  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 <j@x.com>" 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<ContactStore>()(
         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<ContactStore>()(
           }
         }
 
-        return results;
+        return normalizeSuggestions(results);
       },
 
       getGroups: () => {