From 409ada94283c12333d2d65820894b05928941c72 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:39:13 +0100 Subject: [PATCH] fix: keep delivery notifications and encrypted delivery when a scheduled send is moved --- src/api/__tests__/email-send-options.test.ts | 39 ++++++++++++++++++- .../live-stalwart-send-options.test.ts | 15 ++++++- src/api/email.ts | 36 +++++++++++++++-- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/api/__tests__/email-send-options.test.ts b/src/api/__tests__/email-send-options.test.ts index 9b0d43f0..a9b80469 100644 --- a/src/api/__tests__/email-send-options.test.ts +++ b/src/api/__tests__/email-send-options.test.ts @@ -20,7 +20,7 @@ vi.mock('../jmap-client', () => ({ })); import { jmapClient } from '../jmap-client'; -import { saveDraftVersion, sendEmail } from '../email'; +import { rescheduleScheduledSend, saveDraftVersion, sendEmail } from '../email'; import { draftContextFromEmail } from '../../lib/draft-context'; import type { Email } from '../types'; @@ -118,3 +118,40 @@ describe('delivery status notifications and REQUIRETLS', () => { expect(sentCalls().submission.envelope).toBeUndefined(); }); }); + +describe('moving a held send', () => { + it('keeps its delivery notification and REQUIRETLS, and only those', async () => { + const set = (body: Record) => ({ methodResponses: [['EmailSubmission/set', body, '0']] }); + mockRequest.mockReset(); + mockRequest + .mockResolvedValueOnce({ + methodResponses: [ + ['EmailSubmission/get', { + list: [{ + id: 'sub-1', + envelope: { + mailFrom: { email: 'me@example.com', parameters: { HOLDFOR: '3600', RET: 'HDRS', REQUIRETLS: null, SIZE: '1234' } }, + rcptTo: [{ email: 'you@example.com', parameters: { NOTIFY: 'SUCCESS,FAILURE,DELAY', ORCPT: 'x' } }], + }, + }], + }, '0'], + ['Email/get', { list: [{ id: 'e-1', to: [{ email: 'you@example.com' }] }] }, '1'], + ], + }) + .mockResolvedValueOnce(set({ updated: { 'sub-1': null } })) + .mockResolvedValueOnce(set({ destroyed: ['sub-1'] })) + .mockResolvedValueOnce(set({ created: { replacement: { id: 'sub-2', sendAt: '2026-11-01T08:00:00Z' } } })); + + await rescheduleScheduledSend( + { emailSubmissionId: 'sub-1', emailId: 'e-1', identityId: 'id-1', from: [{ email: 'me@example.com' }], to: [{ email: 'you@example.com' }] } as never, + 7200, + ); + const create = mockRequest.mock.calls + .map((c) => c[0][0]) + .find(([method, args]: [string, { create?: object }]) => method === 'EmailSubmission/set' && args.create)[1]; + expect(create.create.replacement.envelope).toEqual({ + mailFrom: { email: 'me@example.com', parameters: { RET: 'HDRS', REQUIRETLS: null, HOLDFOR: '7200' } }, + rcptTo: [{ email: 'you@example.com', parameters: { NOTIFY: 'SUCCESS,FAILURE,DELAY' } }], + }); + }); +}); diff --git a/src/api/__tests__/live-stalwart-send-options.test.ts b/src/api/__tests__/live-stalwart-send-options.test.ts index 79c7da2c..34edde36 100644 --- a/src/api/__tests__/live-stalwart-send-options.test.ts +++ b/src/api/__tests__/live-stalwart-send-options.test.ts @@ -16,7 +16,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { JMAPClient } from '../jmap-client'; -import { cancelScheduledSend, destroyEmails, getMailboxes, listScheduledEmails, sendEmail } from '../email'; +import { cancelScheduledSend, destroyEmails, getMailboxes, listScheduledEmails, rescheduleScheduledSend, sendEmail } from '../email'; import { getIdentities } from '../identity'; import { supportsSubmissionExtension } from '../../lib/submission-options'; @@ -103,6 +103,17 @@ live('live Stalwart: composer send options', () => { expect(submission.envelope.rcptTo).toEqual([ expect.objectContaining({ email: identity.email, parameters: expect.objectContaining({ NOTIFY: 'SUCCESS,FAILURE,DELAY' }) }), ]); - await cancelScheduledSend(result.emailSubmissionId!); + + // Moved to another time, it keeps them. + const held = (await listScheduledEmails()).find((s) => s.emailSubmissionId === result.emailSubmissionId)!; + const moved = await rescheduleScheduledSend(held, 7200); + const after = await singleton.request([['EmailSubmission/get', { + accountId: singleton.accountId, ids: [moved.emailSubmissionId], properties: ['envelope'], + }, '0']], ['urn:ietf:params:jmap:core', 'urn:ietf:params:jmap:submission']); + const [replacement] = after.methodResponses[0][1].list; + expect(replacement.envelope.mailFrom.parameters).toMatchObject({ RET: 'HDRS', HOLDFOR: '7200' }); + expect('REQUIRETLS' in replacement.envelope.mailFrom.parameters).toBe(true); + expect(replacement.envelope.rcptTo[0].parameters).toMatchObject({ NOTIFY: 'SUCCESS,FAILURE,DELAY' }); + await cancelScheduledSend(moved.emailSubmissionId!); }, 60_000); }); diff --git a/src/api/email.ts b/src/api/email.ts index 8f5e4821..23f2853c 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -2289,6 +2289,22 @@ export class RescheduleLostError extends Error { * and send time, and the recipients the server refused for the replacement * when it took the others. */ +/** + * The SMTP parameters among `names` that a held submission's envelope + * carries, or undefined with none. Only the ones the composer sets are kept, + * never parameters the server may have added. + */ +function keptSubmissionParameters( + parameters: Record | null | undefined, + names: string[], +): Record | undefined { + if (!parameters) return undefined; + const kept = Object.fromEntries( + Object.entries(parameters).filter(([key]) => names.includes(key.toUpperCase())), + ); + return Object.keys(kept).length > 0 ? kept : undefined; +} + export async function rescheduleScheduledSend( scheduled: { emailSubmissionId: string; @@ -2321,8 +2337,9 @@ export async function rescheduleScheduledSend( ], [CAPABILITIES.CORE, CAPABILITIES.MAIL, CAPABILITIES.SUBMISSION], ); + type EnvelopeAddress = { email?: string; parameters?: Record | null }; const original = (requireMethodResult(lookup, '0', 'EmailSubmission/get').list as Array<{ - envelope?: { mailFrom?: { email?: string }; rcptTo?: Array<{ email?: string }> } | null; + envelope?: { mailFrom?: EnvelopeAddress; rcptTo?: EnvelopeAddress[] } | null; sendAt?: string; identityId?: string; }> | undefined)?.[0]; @@ -2335,7 +2352,15 @@ export async function rescheduleScheduledSend( const held = requireMethodResult(lookup, '1', 'Email/get').list as Email[] | undefined; const headerRecipients = [...(held?.[0]?.to ?? []), ...(held?.[0]?.cc ?? []), ...(held?.[0]?.bcc ?? [])]; const source = recipients ?? (envelope?.rcptTo?.length ? envelope.rcptTo : headerRecipients); - const rcptTo = source.map((r) => ({ email: (r.email ?? '').trim() })).filter((r) => r.email); + // The original's delivery notification (RET / NOTIFY) and REQUIRETLS + // ride along; dropping them turned a moved send into a plain one. + const keptMailFromParams = keptSubmissionParameters(envelope?.mailFrom?.parameters, ['RET', 'REQUIRETLS']); + const keptRcptParams = keptSubmissionParameters( + envelope?.rcptTo?.find((r) => r.parameters)?.parameters, ['NOTIFY'], + ); + const rcptTo = source + .map((r) => ({ email: (r.email ?? '').trim(), ...(keptRcptParams ? { parameters: keptRcptParams } : {}) })) + .filter((r) => r.email); const mailFrom = envelope?.mailFrom?.email || scheduled.from?.[0]?.email; // Without an answer the replacement is made without filing, and a message // that can't be put back stays where it is. @@ -2366,9 +2391,12 @@ export async function rescheduleScheduledSend( emailId: scheduled.emailId, identityId, envelope: { - mailFrom: seconds === undefined + mailFrom: seconds === undefined && !keptMailFromParams ? { email: mailFrom } - : { email: mailFrom, parameters: { HOLDFOR: String(seconds) } }, + : { + email: mailFrom, + parameters: { ...keptMailFromParams, ...(seconds === undefined ? {} : { HOLDFOR: String(seconds) }) }, + }, rcptTo, }, },