mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-06 00:07:28 +00:00
fix: stop treating narrow refusals as total ones in the app's sends and exports
Two places took a narrow refusal from the server for a complete one.
Port of the webmail's 7977a6ed.
- Sending at a full submission quota (Stalwart 1.0): when making room
found nothing, no search ran for five minutes, for any send. Scheduled
mail and undo-send holds can go once their message is out, and until
the next search every send failed with "ask your administrator". A
send the user starts, a read receipt included, now always looks again;
the pause stays for automatic read receipts.
- vCard export: Stalwart answers Blob/get only in accounts the user is a
member of, so for an address book shared with the user it came back
`forbidden` ("You are not an owner of account ...") and the export fell
back to one photo download after the other. It checks the access to
each blob by itself, whichever account the request names, so
`getBlobsAsBase64` now asks again in the user's own account, on 0.16
and 1.0 alike.
This commit is contained in:
@@ -645,7 +645,7 @@ describe('making room where nothing can go (Stalwart 1.0)', () => {
|
||||
const searches = (server: FakeSubmissionServer) =>
|
||||
server.methods().flat().filter((m) => m === 'EmailSubmission/query' || m === 'EmailSubmission/changes').length;
|
||||
|
||||
it('does not search the account again for five minutes', async () => {
|
||||
it('searches again for every send the user starts', async () => {
|
||||
let clock = Date.parse('2026-09-25T12:00:00Z');
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => clock);
|
||||
const server = setup('1.0', { limit: 2 });
|
||||
@@ -655,24 +655,64 @@ describe('making room where nothing can go (Stalwart 1.0)', () => {
|
||||
]);
|
||||
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
const first = searches(server);
|
||||
// The query, and the change-log walk once it found nothing.
|
||||
expect(first).toBe(2);
|
||||
expect(searches(server)).toBe(2);
|
||||
|
||||
// A send and an automatic read receipt right after: one retry each, no search.
|
||||
clock += MINUTE;
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(first);
|
||||
expect(argsOf(server, 'EmailSubmission/set').filter((a) => a.create)).toHaveLength(6);
|
||||
|
||||
clock += 5 * MINUTE;
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(2 * first);
|
||||
expect(searches(server)).toBe(4);
|
||||
expect(server.submissions().map((s) => s.id)).toEqual(pending);
|
||||
});
|
||||
|
||||
it('searches again at once when a search failed', async () => {
|
||||
// A search that found nothing used to hold off the next one for five
|
||||
// minutes, whoever sent: a held send that went out within them could not
|
||||
// make room, and every send failed with "ask your administrator".
|
||||
it('makes room once a held send went out, minutes after a search found nothing', async () => {
|
||||
let clock = Date.parse('2026-09-25T12:00:00Z');
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => clock);
|
||||
const server = setup('1.0', { limit: 2 });
|
||||
const [undoHold, scheduled] = server.seed('acc-1', [
|
||||
{ sendAt: clock + MINUTE, status: 'pending' },
|
||||
{ sendAt: clock + DAY, status: 'pending' },
|
||||
]);
|
||||
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
|
||||
// Two minutes later the undo hold has run out and its message left.
|
||||
clock += 2 * MINUTE;
|
||||
server.deliver('acc-1', undoHold);
|
||||
const result = await sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS);
|
||||
|
||||
expect(result.emailSubmissionId).toBeDefined();
|
||||
expect(server.submissions().map((s) => s.id)).toEqual([scheduled]);
|
||||
});
|
||||
|
||||
it('does not search again for automatic read receipts for five minutes', async () => {
|
||||
let clock = Date.parse('2026-09-25T12:00:00Z');
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => clock);
|
||||
const server = setup('1.0', { limit: 1 });
|
||||
server.seed('acc-1', [{ sendAt: clock + DAY, status: 'pending' }]);
|
||||
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(2);
|
||||
|
||||
// Every further automatic receipt only tries the create once more ...
|
||||
clock += MINUTE;
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(2);
|
||||
expect(argsOf(server, 'EmailSubmission/set').filter((a) => a.create)).toHaveLength(4);
|
||||
|
||||
// ... while one the user sends looks again,
|
||||
await expect(sendReadReceipt({ ...RECEIPT, automatic: false })).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(4);
|
||||
|
||||
// and so do automatic ones after five minutes.
|
||||
clock += 5 * MINUTE;
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(searches(server)).toBe(6);
|
||||
});
|
||||
|
||||
it('searches again at once for an automatic receipt when a search failed', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||
const server = setup('1.0', { limit: 1 });
|
||||
server.seed('acc-1', [{ sendAt: Date.now() + DAY, status: 'pending' }]);
|
||||
@@ -684,23 +724,23 @@ describe('making room where nothing can go (Stalwart 1.0)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
expect(warn).toHaveBeenCalled();
|
||||
const first = argsOf(server, 'EmailSubmission/query').length;
|
||||
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', undefined, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
// A failed look-up proved nothing, so the second attempt looked again.
|
||||
expect(argsOf(server, 'EmailSubmission/query').length).toBe(first + 1);
|
||||
});
|
||||
|
||||
it('keeps no pause once a search found something to remove', async () => {
|
||||
it('keeps no pause for automatic receipts once a search found something to remove', async () => {
|
||||
const server = setup('1.0', { limit: 2 });
|
||||
const now = Date.now();
|
||||
server.seed('acc-1', [{ sendAt: now + DAY, status: 'pending' }, { sendAt: now - HOUR, status: 'final' }]);
|
||||
|
||||
await sendEmail(OUTGOING, 'id-1', 'sent', 10, VIA_DRAFTS);
|
||||
// Full again: a scheduled send holds the freed place.
|
||||
await expect(sendEmail(OUTGOING, 'id-1', 'sent', 10, VIA_DRAFTS)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
await expect(sendReadReceipt(RECEIPT)).rejects.toBeInstanceOf(SubmissionQuotaError);
|
||||
|
||||
expect(argsOf(server, 'EmailSubmission/query')).toHaveLength(2);
|
||||
});
|
||||
|
||||
+14
-3
@@ -325,18 +325,29 @@ function batchedBySize<T>(items: T[], sizeOf: (item: T) => number | undefined, b
|
||||
* Null when the session doesn't advertise the blob capability, which then
|
||||
* must stay out of `using`. Blobs the server doesn't find are left out; once a
|
||||
* request fails, the blobs not fetched yet are listed in `failed`.
|
||||
*
|
||||
* Stalwart answers Blob/get only in accounts the user is a member of: an
|
||||
* address book shared with the user is in an account that isn't one, and the
|
||||
* call comes back `forbidden` ("You are not an owner of account ..."). It
|
||||
* checks the access to each blob by itself, though, whichever account the
|
||||
* request names, so the rest are asked for in the user's own account
|
||||
* (webmail 7977a6ed).
|
||||
*/
|
||||
export async function getBlobsAsBase64(blobIds: string[], accountId?: string): Promise<BlobBatch | null> {
|
||||
if (!jmapClient.hasCapability(CAPABILITIES.BLOB)) return null;
|
||||
const account = accountId || jmapClient.accountId;
|
||||
let account = accountId || jmapClient.accountId;
|
||||
const ids = [...new Set(blobIds)];
|
||||
const data = new Map<string, string>();
|
||||
const blobGet = async (chunk: string[], property: 'size' | 'data:asBase64') => {
|
||||
const res = await jmapClient.request(
|
||||
const call = () => jmapClient.request(
|
||||
[['Blob/get', { accountId: account, ids: chunk, properties: [property] }, '0']],
|
||||
[CAPABILITIES.CORE, CAPABILITIES.BLOB],
|
||||
);
|
||||
const [name, result] = res.methodResponses?.[0] ?? [];
|
||||
let [name, result] = (await call()).methodResponses?.[0] ?? [];
|
||||
if (name === 'error' && result?.type === 'forbidden' && account !== jmapClient.accountId) {
|
||||
account = jmapClient.accountId;
|
||||
[name, result] = (await call()).methodResponses?.[0] ?? [];
|
||||
}
|
||||
if (name !== 'Blob/get') {
|
||||
throw new Error(`Blob/get failed: ${result?.description || result?.type || 'unknown'}`);
|
||||
}
|
||||
|
||||
+1
-1
@@ -2283,7 +2283,7 @@ export async function sendReadReceipt(opts: SendReadReceiptOptions): Promise<str
|
||||
let res = await submit();
|
||||
// A full submission quota (Stalwart 1.0): make room and try once more.
|
||||
if (submissionCreateError(res, 'mdn')?.type === 'overQuota') {
|
||||
await makeRoomForSubmission(accountId);
|
||||
await makeRoomForSubmission(accountId, { automatic: opts.automatic });
|
||||
res = await submit();
|
||||
}
|
||||
const body = requireMethodResult(res, '0', 'EmailSubmission/set');
|
||||
|
||||
+10
-7
@@ -22,7 +22,7 @@ export const SUBMISSION_ONLY_USING = [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION
|
||||
const PRUNE_INTERVAL_MS = 10 * 60 * 1000;
|
||||
/**
|
||||
* How long an account where making room found nothing that could go is not
|
||||
* searched again; see makeRoomForSubmission.
|
||||
* searched again for an automatic read receipt; see makeRoomForSubmission.
|
||||
*/
|
||||
const ROOM_RECHECK_MS = 5 * 60 * 1000;
|
||||
/** Upper bound on the pages one prune, change-log walk or scheduled list reads. */
|
||||
@@ -318,15 +318,18 @@ function submissionList(res: JMAPResponseBody, callId: string): SubmissionStatus
|
||||
* create either way, since another device may have made room meanwhile.
|
||||
*
|
||||
* At a full quota every attempt ends up here - each send, reschedule and
|
||||
* read receipt. When a search found nothing that can go, the account is left
|
||||
* alone for five minutes: what is left is pending or out of reach, and
|
||||
* looking again at once would cost the prune, the change-log walk and the
|
||||
* gets once more for the same answer.
|
||||
* read receipt. When a search found nothing that can go, it is not repeated
|
||||
* for automatic read receipts for five minutes: what is left is pending or
|
||||
* out of reach, and looking again at once would cost the prune, the
|
||||
* change-log walk and the gets once more for the same answer. A send the
|
||||
* user starts always looks again: pending records - scheduled mail,
|
||||
* undo-send holds - can go once their message is out, and until the next
|
||||
* search every send would fail with "ask your administrator".
|
||||
*/
|
||||
export async function makeRoomForSubmission(accountId: string): Promise<void> {
|
||||
export async function makeRoomForSubmission(accountId: string, options: { automatic?: boolean } = {}): Promise<void> {
|
||||
const key = pruneKey(accountId);
|
||||
const recheckAt = roomRecheckAt.get(key);
|
||||
if (recheckAt !== undefined && Date.now() < recheckAt) return;
|
||||
if (options.automatic && recheckAt !== undefined && Date.now() < recheckAt) return;
|
||||
const now = Date.now();
|
||||
prunedAt.set(key, now);
|
||||
let found = 0;
|
||||
|
||||
@@ -446,6 +446,65 @@ describe('inlineContactMediaForExport', () => {
|
||||
expect(mockRequest.mock.calls[0][0][0][1].accountId).toBe('team');
|
||||
});
|
||||
|
||||
// Stalwart answers Blob/get only in accounts the user is a member of - the
|
||||
// own account and groups, not one that merely shares an address book - and
|
||||
// then serves every blob the user may read, whichever account the request
|
||||
// names. Live: userc's Blob/get naming userb's account `e` (a book shared
|
||||
// with userc) is refused on 1.0 and 0.16.23 alike, and on 1.0 the same
|
||||
// blobId asked for in userc's own account comes back. 0.16 keeps contact
|
||||
// photos as data: URIs, so an export never gets here there.
|
||||
function memberServer(sizes: Record<string, number>, memberOf = ['f', 'team']) {
|
||||
mockRequest.mockImplementation(async (calls: Array<[string, { accountId: string; ids: string[]; properties: string[] }, string]>) => {
|
||||
const [, args, callId] = calls[0];
|
||||
if (!memberOf.includes(args.accountId)) {
|
||||
return { methodResponses: [['error', { type: 'forbidden', description: `You are not an owner of account ${args.accountId}` }, callId]] };
|
||||
}
|
||||
const found = args.ids.filter((id) => id in sizes);
|
||||
const list = found.map((id) => (args.properties[0] === 'size' ? { id, size: sizes[id] } : { id, 'data:asBase64': 'AAAA' }));
|
||||
return { methodResponses: [['Blob/get', { accountId: args.accountId, list, notFound: args.ids.filter((id) => !(id in sizes)) }, callId]] };
|
||||
});
|
||||
}
|
||||
const sharedCard = (blobId: string) =>
|
||||
card(`e:${blobId}`, { photo: { blobId, kind: 'photo', mediaType: 'image/jpeg' } }, { isShared: true, accountId: 'e', originalId: blobId });
|
||||
|
||||
it('reads the photos of an address book shared with the user through the own account (Stalwart 1.0)', async () => {
|
||||
memberServer({ 'b-1': 40_000, 'b-2': 2_000 });
|
||||
|
||||
const out = await inlineContactMediaForExport([sharedCard('b-1'), sharedCard('b-2')]);
|
||||
|
||||
expect(mockRequest.mock.calls.map(([calls]) => calls[0][1].accountId)).toEqual(['e', 'f']);
|
||||
expect(mockRequest.mock.calls[1][0][0]).toEqual(['Blob/get', { accountId: 'f', ids: ['b-1', 'b-2'], properties: ['data:asBase64'] }, '0']);
|
||||
expect(out.map((c) => c.media?.photo?.uri)).toEqual(['data:image/jpeg;base64,AAAA', 'data:image/jpeg;base64,AAAA']);
|
||||
expect(mockFetchBlob).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('switches to the own account once when a shared address book has many photos (Stalwart 1.0)', async () => {
|
||||
const MiB = 1024 * 1024;
|
||||
memberServer({ 'b-1': 3 * MiB, 'b-2': 2 * MiB, 'b-3': 2 * MiB });
|
||||
|
||||
await inlineContactMediaForExport([sharedCard('b-1'), sharedCard('b-2'), sharedCard('b-3')]);
|
||||
|
||||
expect(mockRequest.mock.calls.map(([calls]) => [calls[0][1].accountId, calls[0][1].properties[0]])).toEqual([
|
||||
['e', 'size'],
|
||||
['f', 'size'],
|
||||
['f', 'data:asBase64'],
|
||||
['f', 'data:asBase64'],
|
||||
]);
|
||||
expect(mockFetchBlob).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not ask the own account again when it refuses too', async () => {
|
||||
memberServer({ 'b-1': 10 }, []);
|
||||
mockFetchBlob.mockResolvedValue(new Uint8Array(JPEG_BYTES).buffer);
|
||||
|
||||
const out = await inlineContactMediaForExport([card('One', { photo: { blobId: 'b-1', kind: 'photo', mediaType: 'image/jpeg' } })]);
|
||||
|
||||
expect(mockRequest).toHaveBeenCalledTimes(1);
|
||||
// What the batch could not deliver is downloaded one by one.
|
||||
expect(mockFetchBlob).toHaveBeenCalledTimes(1);
|
||||
expect(out[0].media?.photo?.uri).toBe(JPEG_URI);
|
||||
});
|
||||
|
||||
it('leaves 0.16 cards, whose photos are data: URIs, alone', async () => {
|
||||
const cards = [card('One', { photo: { uri: JPEG_URI, kind: 'photo', mediaType: 'image/jpeg' } })];
|
||||
const out = await inlineContactMediaForExport(cards);
|
||||
|
||||
Reference in New Issue
Block a user