diff --git a/lib/__tests__/contact-wire.test.ts b/lib/__tests__/contact-wire.test.ts index 9268c96f0..90c720868 100644 --- a/lib/__tests__/contact-wire.test.ts +++ b/lib/__tests__/contact-wire.test.ts @@ -32,6 +32,21 @@ describe('contactToWire', () => { expect(contactToWire(cleared, 'create')).toEqual({}); }); + it('never clears id, uid or addressBookIds on update', () => { + const wire = contactToWire({ + id: undefined, uid: undefined, addressBookIds: undefined, nicknames: undefined, + } as Partial, 'update'); + expect(wire).toEqual({ nicknames: null }); + + const nulls = contactToWire({ id: null, uid: null, addressBookIds: null } as unknown as Partial, 'update'); + expect(nulls).toEqual({}); + }); + + it('still sends a uid or addressBookIds that has a value', () => { + expect(contactToWire({ uid: 'urn:uuid:1', addressBookIds: { ab: true } }, 'update')) + .toEqual({ uid: 'urn:uuid:1', addressBookIds: { ab: true } }); + }); + it('leaves calendar links alone when the update does not mention them', () => { expect(contactToWire({ keywords: { vip: true } }, 'update')).toEqual({ keywords: { vip: true } }); }); diff --git a/lib/__tests__/jmap-contact-client.test.ts b/lib/__tests__/jmap-contact-client.test.ts index 6dca116fa..17f439475 100644 --- a/lib/__tests__/jmap-contact-client.test.ts +++ b/lib/__tests__/jmap-contact-client.test.ts @@ -589,6 +589,34 @@ describe('JMAPClient contact methods', () => { .rejects.toThrow('Failed to update contact'); }); + it.each(['1.0', '0.16'] as const)('leaves a cleared id, uid and addressBookIds out of the update on Stalwart %s', async (version) => { + const client = createClient(); + // What ContactCard/set answers to those keys set to null (1.0 contact/set.rs: + // 229-249, 602-607, 669-671; 0.16.23 set.rs:187-199, 524-529, 591-593). + const answer = (update: Record) => { + const refuse = (property: string, description: string) => + ({ notUpdated: { 'contact-1': { type: 'invalidProperties', properties: [property], description } } }); + if ('id' in update) return refuse('id', 'The id property is immutable.'); + if (update.addressBookIds === null) return refuse('addressBookIds', 'Invalid patch operation for addressBookIds.'); + // 0.16 takes a null uid and drops the card's UID without a word. + if (update.uid === null && version === '1.0') return refuse('uid', 'You cannot change the UID of a contact.'); + return { updated: { 'contact-1': null } }; + }; + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const update = JSON.parse(String(init?.body)).methodCalls[0][1].update['contact-1']; + const body = { methodResponses: [['ContactCard/set', answer(update), '0']] }; + return { ok: true, status: 200, text: () => Promise.resolve(JSON.stringify(body)) } as Response; + }); + + // A plugin passes the card back with the keys it did not set. + await client.updateContact('contact-1', { + id: undefined, uid: undefined, addressBookIds: undefined, name: mockContact.name, + } as never); + + const sent = JSON.parse(fetchSpy.mock.calls[0][1]?.body as string).methodCalls[0][1].update['contact-1']; + expect(sent).toEqual({ name: mockContact.name }); + }); + it('passes a Stalwart 1.0 blob photo through unchanged', async () => { const client = createClient(); const fetchSpy = mockFetch({ diff --git a/lib/__tests__/vcard.test.ts b/lib/__tests__/vcard.test.ts index 31869a02f..af850edab 100644 --- a/lib/__tests__/vcard.test.ts +++ b/lib/__tests__/vcard.test.ts @@ -540,6 +540,46 @@ describe("organization-only cards (issue #701)", () => { }); }); +describe("MEMBER export", () => { + const group = (members: Record): ContactCard => ({ + id: "g1", + addressBookIds: {}, + kind: "group", + name: { full: "Team" }, + members, + }); + + it("writes a member key that is already a URI as it is", () => { + // Groups made in Bulwark key members by uid (urn:uuid:); CardDAV groups + // keep whatever MEMBER value the client sent. + const vcf = generateVCard([group({ "urn:uuid:0f3c9d2e": true, "mailto:ann@example.com": true })]); + const lines = vcf.split("\r\n"); + expect(lines).toContain("MEMBER:urn:uuid:0f3c9d2e"); + expect(lines).toContain("MEMBER:mailto:ann@example.com"); + expect(vcf).not.toContain("urn:uuid:urn:uuid:"); + }); + + it("prefixes a bare key with urn:uuid:", () => { + expect(generateVCard([group({ "0f3c9d2e": true, gone: false })]).split("\r\n").filter(l => l.startsWith("MEMBER:"))) + .toEqual(["MEMBER:urn:uuid:0f3c9d2e"]); + }); + + it("round-trips members through import and export", () => { + const [parsed] = parseVCard([ + "BEGIN:VCARD", + "VERSION:3.0", + "FN:Team", + "KIND:group", + "MEMBER:urn:uuid:member-1", + "MEMBER:mailto:ann@example.com", + "END:VCARD", + ].join("\r\n")); + const [reparsed] = parseVCard(generateVCard([parsed])); + expect(reparsed.members).toEqual(parsed.members); + expect(generateVCard([parsed])).toContain("MEMBER:urn:uuid:member-1"); + }); +}); + describe("round-trip: parse → generate → parse", () => { it("produces structurally equivalent data", () => { const original = [ diff --git a/lib/jmap/contact-wire.ts b/lib/jmap/contact-wire.ts index 578e013a7..5e7c8798a 100644 --- a/lib/jmap/contact-wire.ts +++ b/lib/jmap/contact-wire.ts @@ -17,6 +17,15 @@ const CLIENT_ONLY_KEYS = [ /** vCard CALURI / FBURL / CALADRURI, kept flat in the UI. */ const URI_KEYS = ['calendarUri', 'freeBusyUri', 'schedulingUri', 'source'] as const; +/** + * Properties an update never clears. A null `id` fails the whole update ("The + * id property is immutable.") and so does a null `addressBookIds` ("Invalid + * patch operation for addressBookIds."). A null `uid` is refused by Stalwart + * 1.0 ("You cannot change the UID of a contact.") and on 0.16 silently + * deletes the card's UID. + */ +const NEVER_CLEARED_KEYS = ['id', 'uid', 'addressBookIds'] as const; + const LEGACY_ADDRESS_FIELDS: Array<[keyof ContactAddress, AddressComponent['kind']]> = [ ['street', 'name'], ['locality', 'locality'], @@ -49,12 +58,14 @@ type Mode = 'create' | 'update'; * The card as ContactCard/set expects it. On `update`, a key that is present * with the value `undefined` means "the user cleared it" and is sent as * `null`: JSON drops undefined, and an omitted property keeps its old value. + * `id`, `uid` and `addressBookIds` without a value are left out instead. */ export function contactToWire(card: Partial, mode: Mode): Record { const out: Record = {}; for (const [key, value] of Object.entries(card)) { if ((CLIENT_ONLY_KEYS as readonly string[]).includes(key)) continue; if ((URI_KEYS as readonly string[]).includes(key)) continue; + if (mode === 'update' && value == null && (NEVER_CLEARED_KEYS as readonly string[]).includes(key)) continue; if (value === undefined) { if (mode === 'update') out[key] = null; continue; diff --git a/lib/vcard.ts b/lib/vcard.ts index ba6c23e87..52c60ff81 100644 --- a/lib/vcard.ts +++ b/lib/vcard.ts @@ -1405,7 +1405,11 @@ function generateSingleVCard(contact: ContactCard): string { if (contact.members) { for (const memberId of Object.keys(contact.members)) { if (contact.members[memberId]) { - lines.push(`MEMBER:urn:uuid:${memberId}`); + // Keys are URIs already for groups made here (members are keyed by + // uid, a urn:uuid:) and for groups from CardDAV clients (Stalwart + // keeps MEMBER values verbatim); only a bare key gets the prefix. + const uri = /^[a-z][a-z0-9+.-]*:/i.test(memberId) ? memberId : `urn:uuid:${memberId}`; + lines.push(`MEMBER:${uri}`); } } }