feat: warn about the server's forward limit and an auto-reply too long for it (bulwarkmail/webmail#1155, bulwarkmail/webmail#1208)

This commit is contained in:
Linus Rath
2026-10-31 04:05:57 +01:00
parent 6e697baa46
commit dcbfc907fa
14 changed files with 614 additions and 7 deletions
+10
View File
@@ -33,6 +33,16 @@ export class FiltersLoginChangedError extends Error {
} }
} }
/** The account's filters as the server has them now (its active script, parsed). */
export async function readAccountFilters(accountId: string = getSieveAccountId()): Promise<ParseResult> {
const all = await getSieveScripts(accountId);
const scripts = all.filter((s) => s.name !== VACATION_SCRIPT_NAME);
const script = scripts.find((s) => s.isActive) || scripts[0] || null;
return script
? parseScript(await getSieveScriptContent(script.blobId, accountId))
: { rules: [], isOpaque: false, externalRequires: [] };
}
/** /**
* Read the account's script from the server, let `modify` change its rules, * Read the account's script from the server, let `modify` change its rules,
* and write the result back as the active script, keeping the vacation * and write the result back as the active script, keeping the vacation
+41 -2
View File
@@ -42,6 +42,8 @@ import type {
FilterActionType, FilterActionType,
} from '../../lib/sieve/types'; } from '../../lib/sieve/types';
import { SafeAreaModal } from '../SafeAreaModal'; import { SafeAreaModal } from '../SafeAreaModal';
import { useFilterStore } from '../../stores/filter-store';
import { forwardsOfEditedRule, ruleForwardLimit } from '../../lib/sieve/forward-limit';
import { MODAL_ORIENTATIONS } from '../../lib/modal-orientations'; import { MODAL_ORIENTATIONS } from '../../lib/modal-orientations';
const ALL_FIELDS: FilterConditionField[] = ['from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment', 'all']; const ALL_FIELDS: FilterConditionField[] = ['from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment', 'all'];
@@ -72,12 +74,19 @@ interface FilterRuleModalProps {
mailboxes: Mailbox[]; mailboxes: Mailbox[];
/** The server can confine a rule to a period (Sieve "date" and "relational"). */ /** The server can confine a rule to a period (Sieve "date" and "relational"). */
periodsSupported?: boolean; periodsSupported?: boolean;
/**
* The server's cap on forwards per message, and the forwards a message can
* have collected when it reaches this rule / still collect below it (see
* lib/sieve/forward-limit.ts). Left out: read from the filter list the
* Filters settings show.
*/
forwardContext?: { maxRedirects: number | null | undefined; before: number; after: number };
onSave: (rule: FilterRule) => void; onSave: (rule: FilterRule) => void;
onClose: () => void; onClose: () => void;
} }
export function FilterRuleModal({ export function FilterRuleModal({
visible, rule: editedRule, initialRule, mailboxes, periodsSupported = false, onSave, onClose, visible, rule: editedRule, initialRule, mailboxes, periodsSupported = false, forwardContext, onSave, onClose,
}: FilterRuleModalProps) { }: FilterRuleModalProps) {
const c = useColors(); const c = useColors();
const styles = useMemo(() => makeStyles(c), [c]); const styles = useMemo(() => makeStyles(c), [c]);
@@ -120,6 +129,25 @@ export function FilterRuleModal({
const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]); const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]);
// A message can collect only so many forwards on the server (#1155, #1208).
const listedRules = useFilterStore((s) => s.rules);
const listedForward = useFilterStore((s) => s.vacationForward);
const listedMaxRedirects = useFilterStore((s) => s.sieveCapabilities?.maxNumberRedirects);
const listedAround = useMemo(
() => forwardsOfEditedRule(listedRules, listedForward, editedRule?.id),
[listedRules, listedForward, editedRule?.id],
);
const forwardLimit = ruleForwardLimit(
{ actions, stopProcessing },
forwardContext ? forwardContext.maxRedirects : listedMaxRedirects,
forwardContext ?? listedAround,
);
const ruleForwardCount = actions.filter((a) => a.type === 'forward').length;
const actionOptionsFor = (action: FilterAction) =>
action.type === 'forward' || forwardLimit.canAddForward
? actionTypeOptions
: actionTypeOptions.filter((o) => o.value !== 'forward');
const fieldOptions = useMemo( const fieldOptions = useMemo(
() => ALL_FIELDS.map((f) => ({ value: f, label: t(`settings.filters.condition_fields.${f}`, f) })), () => ALL_FIELDS.map((f) => ({ value: f, label: t(`settings.filters.condition_fields.${f}`, f) })),
[t], [t],
@@ -407,7 +435,7 @@ export function FilterRuleModal({
<Select <Select
value={action.type} value={action.type}
onChange={(v) => updateAction(index, { type: v as FilterActionType })} onChange={(v) => updateAction(index, { type: v as FilterActionType })}
options={actionTypeOptions} options={actionOptionsFor(action)}
style={{ flex: 1 }} style={{ flex: 1 }}
/> />
<Pressable <Pressable
@@ -489,6 +517,16 @@ export function FilterRuleModal({
</View> </View>
))} ))}
</View> </View>
{forwardLimit.unavailable && ruleForwardCount > 0 && (
<Text style={styles.forwardWarning}>
{t('settings.filters.forward_unavailable', 'This server does not allow forwarding. Forward actions in filter rules have no effect.')}
</Text>
)}
{forwardLimit.overLimit && (
<Text style={styles.forwardWarning}>
{t('settings.filters.forward_limit', 'Forward limit per message on this server: {count}. Extra forwards are skipped.', { count: forwardLimit.limit ?? 0 })}
</Text>
)}
<Pressable <Pressable
onPress={() => setActions((prev) => [...prev, makeEmptyAction()])} onPress={() => setActions((prev) => [...prev, makeEmptyAction()])}
style={styles.addRow} style={styles.addRow}
@@ -645,6 +683,7 @@ function makeStyles(c: ThemePalette) {
}, },
optionLabel: { ...typography.body, color: c.text, flex: 1 }, optionLabel: { ...typography.body, color: c.text, flex: 1 },
hint: { ...typography.caption, color: c.mutedForeground }, hint: { ...typography.caption, color: c.mutedForeground },
forwardWarning: { ...typography.caption, color: c.warning, marginTop: spacing.sm },
footer: { footer: {
flexDirection: 'row', flexDirection: 'row',
+26 -1
View File
@@ -5,7 +5,8 @@ import { MoveSheet } from '../MoveSheet';
import { FilterRuleModal } from './FilterRuleModal'; import { FilterRuleModal } from './FilterRuleModal';
import type { Email, Mailbox } from '../../api/types'; import type { Email, Mailbox } from '../../api/types';
import { getSieveAccountId, getSieveCapabilities } from '../../api/sieve'; import { getSieveAccountId, getSieveCapabilities } from '../../api/sieve';
import { FiltersLoginChangedError, OpaqueFiltersError, updateAccountFilters } from '../../api/sieve-rules'; import { FiltersLoginChangedError, OpaqueFiltersError, readAccountFilters, updateAccountFilters } from '../../api/sieve-rules';
import { forwardsAround, inRunOrder } from '../../lib/sieve/forward-limit';
import { jmapClient } from '../../api/jmap-client'; import { jmapClient } from '../../api/jmap-client';
import { servedLoginOf } from '../../lib/draft-login'; import { servedLoginOf } from '../../lib/draft-login';
import { supportsPeriods } from '../../lib/sieve/period'; import { supportsPeriods } from '../../lib/sieve/period';
@@ -87,6 +88,26 @@ export function QuickRuleSheet({ visible, email, mailboxes, onClose }: QuickRule
const junk = React.useMemo(() => findJunkMailbox(mailboxes), [mailboxes]); const junk = React.useMemo(() => findJunkMailbox(mailboxes), [mailboxes]);
const periodsSupported = supportsPeriods(getSieveCapabilities()?.sieveExtensions); const periodsSupported = supportsPeriods(getSieveCapabilities()?.sieveExtensions);
// Where the new rule runs among the account's rules, for the server's
// redirect limit: at the top, behind the out of office forwarding
// (webmail quick-rule-host.tsx; lib/sieve/forward-limit.ts).
const editorOpen = visible && step.kind === 'editor';
const [forwards, setForwards] = React.useState({ before: 0, after: 0 });
React.useEffect(() => {
setForwards({ before: 0, after: 0 });
if (!editorOpen) return;
let current = true;
const accountId = openedIn.current.accountId ?? undefined;
readAccountFilters(accountId)
.then((parsed) => {
if (!current || parsed.isOpaque) return;
const runOrder = inRunOrder(parsed.rules, parsed.vacationForward);
setForwards(forwardsAround(runOrder, runOrder.length - parsed.rules.length, false));
})
.catch(() => undefined);
return () => { current = false; };
}, [editorOpen]);
const [sender] = subject.senders; const [sender] = subject.senders;
const senderLabel = subject.senders.length === 1 const senderLabel = subject.senders.length === 1
? (sender.name || sender.email) ? (sender.name || sender.email)
@@ -248,6 +269,10 @@ export function QuickRuleSheet({ visible, email, mailboxes, onClose }: QuickRule
initialRule={step.kind === 'editor' ? step.rule : undefined} initialRule={step.kind === 'editor' ? step.rule : undefined}
mailboxes={mailboxes} mailboxes={mailboxes}
periodsSupported={periodsSupported} periodsSupported={periodsSupported}
forwardContext={{
maxRedirects: getSieveCapabilities(openedIn.current.accountId ?? undefined)?.maxNumberRedirects,
...forwards,
}}
onSave={(rule) => void saveEditorRule(rule)} onSave={(rule) => void saveEditorRule(rule)}
onClose={finish} onClose={finish}
/> />
+37 -3
View File
@@ -26,6 +26,9 @@ import {
parseLocalInput, parseLocalInput,
utcIsoToLocalInput, utcIsoToLocalInput,
} from '../../lib/vacation-dates'; } from '../../lib/vacation-dates';
import { STALWART_VACATION_LIMITS, vacationOversize } from '../../lib/vacation-limits';
import { jmapClient } from '../../api/jmap-client';
import { getSieveCapabilities } from '../../api/sieve';
// Ported from the webmail's components/settings/vacation-settings.tsx. Dates // Ported from the webmail's components/settings/vacation-settings.tsx. Dates
// are typed as local wall-clock time and sent as RFC 8621 UTCDate strings; // are typed as local wall-clock time and sent as RFC 8621 UTCDate strings;
@@ -122,6 +125,11 @@ export function VacationSettings() {
// Only what was changed here is sent: the rest stays as the server has it, // Only what was changed here is sent: the rest stays as the server has it,
// which another device may have changed since this one loaded. // which another device may have changed since this one loaded.
const forwardChanged = forwardSettings !== undefined && !sameForward(forwardSettings, store.forward); const forwardChanged = forwardSettings !== undefined && !sameForward(forwardSettings, store.forward);
// Kept here, the message goes on through the filter rules, whose forwards
// share the server's limit with this one (#1208).
const forwardLimit = getSieveCapabilities(managedAccountId ?? undefined)?.maxNumberRedirects;
const forwardOverLimit = store.forwardAvailable && forwardEnabled && forwardKeep
&& typeof forwardLimit === 'number' && forwardLimit > 0 && 1 + store.otherForwards > forwardLimit;
const fromParsed = parseLocalInput(fromDate); const fromParsed = parseLocalInput(fromDate);
const toParsed = parseLocalInput(toDate); const toParsed = parseLocalInput(toDate);
@@ -137,6 +145,22 @@ export function VacationSettings() {
return fromParsed < todayStart; return fromParsed < todayStart;
}, [fromParsed]); }, [fromParsed]);
// What a save sends, measured against the server's limits as you type.
const sizeLimits = jmapClient.hasAccountCapability('urn:stalwart:jmap', managedAccountId ?? undefined)
? STALWART_VACATION_LIMITS
: null;
const oversize = useMemo(() => {
const sanitizedHtml = hasHtmlContent ? stripDangerousTags(htmlBody) : null;
const textBody = body.trim() || !sanitizedHtml ? body : htmlToPlainText(sanitizedHtml);
return vacationOversize({ subject: subject.trim(), textBody, htmlBody: sanitizedHtml }, sizeLimits);
}, [hasHtmlContent, htmlBody, body, subject, sizeLimits]);
const subjectTooLong = oversize.subject && sizeLimits
? t('settings.vacation.warnings.subject_too_long', 'The subject is too long for the server (at most {max} bytes).', { max: sizeLimits.subject })
: null;
const bodyTooLong = oversize.body && sizeLimits
? t('settings.vacation.warnings.body_too_long', 'The message is too long for the server (at most {max} bytes, formatting included).', { max: sizeLimits.body })
: null;
const warnings: string[] = []; const warnings: string[] = [];
if (endBeforeStart) warnings.push(t('settings.vacation.warnings.end_before_start', 'End date must be after start date')); if (endBeforeStart) warnings.push(t('settings.vacation.warnings.end_before_start', 'End date must be after start date'));
if (startInPast) warnings.push(t('settings.vacation.warnings.start_in_past', 'Start date is in the past')); if (startInPast) warnings.push(t('settings.vacation.warnings.start_in_past', 'Start date is in the past'));
@@ -155,7 +179,8 @@ export function VacationSettings() {
forwardChanged || forwardChanged ||
audienceChanged; audienceChanged;
const canSave = hasChanges && !endBeforeStart && !formatError && !forwardInvalid && !store.isSaving; const canSave = hasChanges && !endBeforeStart && !formatError && !forwardInvalid
&& !oversize.subject && !oversize.body && !store.isSaving;
const handleSave = useCallback(async () => { const handleSave = useCallback(async () => {
// Read the live editor DOM rather than trusting onChange state (issue #9). // Read the live editor DOM rather than trusting onChange state (issue #9).
@@ -342,8 +367,9 @@ export function VacationSettings() {
placeholder={t('settings.vacation.message.subject_placeholder', 'Out of Office')} placeholder={t('settings.vacation.message.subject_placeholder', 'Out of Office')}
placeholderTextColor={c.mutedForeground} placeholderTextColor={c.mutedForeground}
accessibilityLabel={t('settings.vacation.message.subject_label', 'Subject')} accessibilityLabel={t('settings.vacation.message.subject_label', 'Subject')}
style={styles.subjectInput} style={[styles.subjectInput, oversize.subject && styles.inputInvalid]}
/> />
{subjectTooLong && <Text style={styles.fieldError}>{subjectTooLong}</Text>}
</View> </View>
<View style={styles.bodyBlock}> <View style={styles.bodyBlock}>
<Text style={styles.bodyLabel}>{t('settings.vacation.message.body_label', 'Message Body')}</Text> <Text style={styles.bodyLabel}>{t('settings.vacation.message.body_label', 'Message Body')}</Text>
@@ -353,11 +379,12 @@ export function VacationSettings() {
onChangeText={setBody} onChangeText={setBody}
placeholder={t('settings.vacation.message.body_placeholder', 'Thank you for your email. I am currently out of the office and will respond when I return.')} placeholder={t('settings.vacation.message.body_placeholder', 'Thank you for your email. I am currently out of the office and will respond when I return.')}
placeholderTextColor={c.mutedForeground} placeholderTextColor={c.mutedForeground}
style={styles.bodyInput} style={[styles.bodyInput, oversize.body && styles.inputInvalid]}
multiline multiline
numberOfLines={6} numberOfLines={6}
textAlignVertical="top" textAlignVertical="top"
/> />
{bodyTooLong && <Text style={styles.fieldError}>{bodyTooLong}</Text>}
</View> </View>
<SettingItem <SettingItem
label={t('settings.vacation.message.html_label', 'Formatted message (HTML)')} label={t('settings.vacation.message.html_label', 'Formatted message (HTML)')}
@@ -466,6 +493,11 @@ export function VacationSettings() {
accessibilityLabel={t('settings.vacation.forward.keep_label', 'Keep in inbox')} accessibilityLabel={t('settings.vacation.forward.keep_label', 'Keep in inbox')}
/> />
</SettingItem> </SettingItem>
{forwardOverLimit && (
<Text style={styles.forwardWarning}>
{t('settings.filters.forward_limit', 'Forward limit per message on this server: {count}. Extra forwards are skipped.', { count: forwardLimit })}
</Text>
)}
</> </>
)} )}
</SettingsSection> </SettingsSection>
@@ -570,6 +602,8 @@ function makeStyles(c: ThemePalette) {
...typography.body, ...typography.body,
}, },
inputInvalid: { borderColor: c.error }, inputInvalid: { borderColor: c.error },
fieldError: { ...typography.caption, color: c.error, marginTop: spacing.xs },
forwardWarning: { ...typography.caption, color: c.warning, paddingHorizontal: spacing.lg, paddingBottom: spacing.md },
subjectInput: { subjectInput: {
paddingHorizontal: spacing.md, paddingHorizontal: spacing.md,
paddingVertical: 6, paddingVertical: 6,
+28
View File
@@ -0,0 +1,28 @@
import { describe, expect, it } from 'vitest';
import { STALWART_VACATION_LIMITS, utf8Length, vacationOversize } from '../vacation-limits';
describe('vacationOversize', () => {
const limits = STALWART_VACATION_LIMITS;
it('flags a subject of 512 bytes and a body of 2048 bytes, not one byte less', () => {
expect(vacationOversize({ subject: 'a'.repeat(511), textBody: 'b'.repeat(2047), htmlBody: null }, limits))
.toEqual({ subject: false, body: false });
expect(vacationOversize({ subject: 'a'.repeat(512), textBody: 'b'.repeat(2048), htmlBody: null }, limits))
.toEqual({ subject: true, body: true });
});
it('counts bytes, not characters, and the HTML part with its formatting', () => {
// 256 umlauts are 512 bytes.
expect(vacationOversize({ subject: 'ä'.repeat(256), textBody: '', htmlBody: null }, limits).subject).toBe(true);
expect(vacationOversize({ subject: '', textBody: 'short', htmlBody: `<p>${'x'.repeat(2045)}</p>` }, limits).body).toBe(true);
});
it('says nothing for a server without known limits', () => {
expect(vacationOversize({ subject: 'a'.repeat(9000), textBody: '', htmlBody: null }, null))
.toEqual({ subject: false, body: false });
});
it('measures UTF-8 like the server', () => {
expect(utf8Length('aä€😀')).toBe(1 + 2 + 3 + 4);
});
});
@@ -0,0 +1,62 @@
import { describe, expect, it } from 'vitest';
import type { FilterAction, FilterRule, VacationForward } from '../types';
import { forwardsOfEditedRule, ruleForwardLimit, rulesForwardLimit } from '../forward-limit';
const forward: FilterAction = { type: 'forward', value: 'x@example.com' };
const rule = (id: string, actions: FilterAction[], extra: Partial<FilterRule> = {}): FilterRule => ({
id,
name: id,
enabled: true,
matchType: 'all',
conditions: [{ field: 'from', comparator: 'contains', value: 'a' }],
actions,
stopProcessing: false,
...extra,
});
const deputy: VacationForward = { enabled: true, to: 'deputy@example.com', keepCopy: true } as VacationForward;
describe('ruleForwardLimit', () => {
it('warns when one message would collect more forwards than the server runs (Stalwart: 1)', () => {
const state = ruleForwardLimit({ actions: [forward], stopProcessing: false }, 1, { before: 1, after: 0 });
expect(state).toEqual({ limit: 1, overLimit: true, unavailable: false, canAddForward: false });
});
it('lets a rule that stops ignore the forwards below it', () => {
expect(ruleForwardLimit({ actions: [forward], stopProcessing: true }, 1, { before: 0, after: 3 }).overLimit).toBe(false);
expect(ruleForwardLimit({ actions: [forward], stopProcessing: false }, 1, { before: 0, after: 3 }).overLimit).toBe(true);
});
it('offers a forward while there is room and none on a server without forwarding', () => {
expect(ruleForwardLimit({ actions: [{ type: 'move', value: 'A' }], stopProcessing: false }, 1, { before: 0, after: 0 }).canAddForward).toBe(true);
const off = ruleForwardLimit({ actions: [forward], stopProcessing: false }, 0, { before: 0, after: 0 });
expect(off).toMatchObject({ limit: null, overLimit: false, unavailable: true, canAddForward: false });
});
it('says nothing without a cap from the server', () => {
expect(ruleForwardLimit({ actions: [forward, forward], stopProcessing: false }, undefined, { before: 5, after: 5 }))
.toEqual({ limit: null, overLimit: false, unavailable: false, canAddForward: true });
});
});
describe('forwardsOfEditedRule', () => {
const rules = [rule('a', [forward]), rule('b', [{ type: 'move', value: 'A' }]), rule('ext', [forward], { origin: 'external' })];
it('counts the forwards above and below the edited rule, the out of office forwarding first', () => {
expect(forwardsOfEditedRule(rules, deputy, 'b')).toEqual({ before: 2, after: 1 });
expect(forwardsOfEditedRule(rules, null, 'a')).toEqual({ before: 0, after: 1 });
});
it('puts a new rule below Bulwark\'s own rules, above the external ones', () => {
expect(forwardsOfEditedRule(rules, null, undefined)).toEqual({ before: 1, after: 1 });
});
});
describe('rulesForwardLimit', () => {
it('flags rules whose forwards add up past the cap, and forwards a server ignores', () => {
const rules = [rule('a', [forward]), rule('b', [forward])];
expect(rulesForwardLimit(rules, null, 1)).toEqual({ tooMany: true, ignored: false });
expect(rulesForwardLimit(rules, null, 2)).toEqual({ tooMany: false, ignored: false });
expect(rulesForwardLimit([rules[0]], deputy, 1)).toEqual({ tooMany: true, ignored: false });
expect(rulesForwardLimit(rules, null, 0)).toEqual({ tooMany: false, ignored: true });
});
});
@@ -0,0 +1,59 @@
import { describe, expect, it } from 'vitest';
import type { FilterAction, FilterRule, VacationForward } from '../types';
import { generateScript } from '../generator';
import { VACATION_FORWARD_MARKER } from '../vacation-forward';
import { forwardsAround, inRunOrder, ruleForwards, ruleStops, worstCaseForwards } from '../forward-limit';
const forward: FilterAction = { type: 'forward', value: 'office@example.com' };
const rule = (actions: FilterAction[], stopProcessing = false): FilterRule => ({
id: `r-${Math.random()}`,
name: 'R',
enabled: true,
matchType: 'all',
conditions: [{ field: 'from', comparator: 'contains', value: 'a' }],
actions,
stopProcessing,
});
const vacationForward = (keepCopy: boolean, enabled = true): VacationForward => ({
enabled,
to: 'deputy@example.com',
keepCopy,
});
describe('the out of office forwarding in the run order', () => {
const rules = [rule([forward])];
it('goes ahead of the rules, and stops unless it keeps a copy', () => {
expect(inRunOrder(rules, vacationForward(true))).toEqual([
{ enabled: true, actions: [{ type: 'forward', value: 'deputy@example.com' }], stopProcessing: false },
...rules,
]);
expect(inRunOrder(rules, vacationForward(false))[0].stopProcessing).toBe(true);
});
it('is left out while it is off', () => {
expect(inRunOrder(rules, vacationForward(true, false))).toBe(rules);
expect(inRunOrder(rules, null)).toBe(rules);
expect(inRunOrder(rules, undefined)).toBe(rules);
});
it('agrees with the script: one redirect ahead of every rule, and a stop unless a copy is kept', () => {
for (const keepCopy of [true, false]) {
const script = generateScript(rules, undefined, { vacationForward: vacationForward(keepCopy) });
expect(script.indexOf(VACATION_FORWARD_MARKER)).toBeGreaterThan(-1);
expect(script.indexOf(VACATION_FORWARD_MARKER)).toBeLessThan(script.indexOf('# Rule:'));
const lines = script.split(VACATION_FORWARD_MARKER)[1].split('# Rule:')[0].split('\n').map((line) => line.trim());
const [ahead] = inRunOrder([], vacationForward(keepCopy));
expect(lines.filter((line) => line.startsWith('redirect ')).length, `keepCopy ${keepCopy}`).toBe(ruleForwards(ahead));
expect(lines.includes('stop;'), `keepCopy ${keepCopy}`).toBe(ruleStops(ahead));
}
});
it('adds its forward to every rule only when it keeps a copy', () => {
// A new rule goes behind Bulwark's rules, here behind the one rule.
expect(forwardsAround(inRunOrder(rules, vacationForward(true)), 2, false)).toEqual({ before: 2, after: 0 });
expect(forwardsAround(inRunOrder(rules, vacationForward(false)), 2, false)).toEqual({ before: 1, after: 0 });
expect(worstCaseForwards(inRunOrder(rules, vacationForward(true)))).toBe(2);
expect(worstCaseForwards(inRunOrder(rules, vacationForward(false)))).toBe(1);
});
});
@@ -0,0 +1,162 @@
import { describe, expect, it } from 'vitest';
import type { FilterAction, FilterRule } from '../types';
import { generateScript } from '../generator';
import { forwardsAround, ruleForwards, ruleStops, worstCaseForwards } from '../forward-limit';
const forward: FilterAction = { type: 'forward', value: 'x@example.com' };
const rule = (actions: FilterAction[], stopProcessing = false, enabled = true): FilterRule => ({
id: `r-${Math.random()}`,
name: 'R',
enabled,
matchType: 'all',
conditions: [{ field: 'from', comparator: 'contains', value: 'a' }],
actions,
stopProcessing,
});
// "Forward to a colleague, delete silently, stop processing", and a rule for
// every other message that forwards too.
const vendor = rule([forward, { type: 'discard' }], true);
const everyoneElse = rule([forward]);
describe('the forwards of one rule', () => {
it('counts those ahead of a Stop action', () => {
expect(ruleForwards(rule([forward, forward]))).toBe(2);
expect(ruleForwards(rule([forward, { type: 'stop' }, forward]))).toBe(1);
expect(ruleForwards(rule([{ type: 'mark_read' }]))).toBe(0);
});
it('knows a rule stops by its checkbox or a Stop action', () => {
expect(ruleStops(rule([forward], true))).toBe(true);
expect(ruleStops(rule([forward, { type: 'stop' }]))).toBe(true);
// Delete silently and Reject alone do not end the script.
expect(ruleStops(rule([forward, { type: 'discard' }]))).toBe(false);
expect(ruleStops(rule([{ type: 'reject', value: 'no' }]))).toBe(false);
});
});
describe('the most forwards one message can collect', () => {
it('stays at one when a rule that forwards stops ahead of another that forwards', () => {
expect(worstCaseForwards([vendor, everyoneElse])).toBe(1);
});
it('is two once the rule for every message comes first', () => {
// A message for the colleague meets both: the second forward would be skipped.
expect(worstCaseForwards([everyoneElse, vendor])).toBe(2);
});
it('adds up the rules that let a message go on', () => {
expect(worstCaseForwards([everyoneElse, rule([forward]), rule([forward])])).toBe(3);
expect(worstCaseForwards([rule([forward], true), rule([forward, forward], true)])).toBe(2);
});
it('leaves out rules that are off', () => {
expect(worstCaseForwards([rule([forward], false, false), vendor])).toBe(1);
});
it('is none without forwards', () => {
expect(worstCaseForwards([])).toBe(0);
expect(worstCaseForwards([rule([{ type: 'mark_read' }], true)])).toBe(0);
});
});
describe('the forwards around one rule', () => {
it('takes nothing from a rule above that stops, for a new rule below it', () => {
expect(forwardsAround([vendor], 1, false)).toEqual({ before: 0, after: 0 });
});
it('takes the forwards of a rule above that lets messages go on', () => {
const going = rule([forward, { type: 'discard' }]);
expect(forwardsAround([going], 1, false)).toEqual({ before: 1, after: 0 });
});
it('takes the worst case below an edited rule, without the rule itself', () => {
expect(forwardsAround([vendor, everyoneElse], 0, true)).toEqual({ before: 0, after: 1 });
expect(forwardsAround([everyoneElse, vendor], 1, true)).toEqual({ before: 1, after: 0 });
});
it('puts a new rule at the index, keeping the rule there below it', () => {
expect(forwardsAround([everyoneElse, vendor], 1, false)).toEqual({ before: 1, after: 1 });
});
});
describe('the count and the script', () => {
it('agree on the forwards a rule runs and whether it stops', () => {
const variants: FilterAction[][] = [
[forward],
[forward, { type: 'discard' }],
[forward, { type: 'reject', value: 'no' }],
[forward, { type: 'move', value: 'Archive' }],
[{ type: 'mark_read' }, forward, forward],
[forward, { type: 'stop' }, forward],
[{ type: 'stop' }, forward],
];
for (const actions of variants) {
for (const stopProcessing of [false, true]) {
const r = rule(actions, stopProcessing);
const lines = generateScript([r]).split('# Rule:')[1].split('\n').map((line) => line.trim());
const stop = lines.indexOf('stop;');
const runs = stop === -1 ? lines : lines.slice(0, stop);
const label = `${actions.map((a) => a.type).join(', ')}${stopProcessing ? ', stop processing' : ''}`;
expect(runs.filter((line) => line.startsWith('redirect ')).length, label).toBe(ruleForwards(r));
expect(stop !== -1, label).toBe(ruleStops(r));
}
}
});
});
describe('against every set of rules a message can match', () => {
// A message that matches the rules in `matched`: the forwards it collects,
// and the rules that run for it.
function walk(rules: FilterRule[], matched: Set<number>): { forwards: number; ran: Set<number> } {
let forwards = 0;
const ran = new Set<number>();
for (let i = 0; i < rules.length; i++) {
if (!rules[i].enabled || !matched.has(i)) continue;
ran.add(i);
forwards += ruleForwards(rules[i]);
if (ruleStops(rules[i])) break;
}
return { forwards, ran };
}
// The most forwards one message can collect; with `runs`, one the rule
// there runs for.
function mostCollected(rules: FilterRule[], runs?: number): number {
let most = 0;
for (let mask = 0; mask < 1 << rules.length; mask++) {
const matched = new Set(rules.map((_, i) => i).filter((i) => mask & (1 << i)));
const { forwards, ran } = walk(rules, matched);
if (runs === undefined || ran.has(runs)) most = Math.max(most, forwards);
}
return most;
}
// Fixed seed: the same rule lists on every run.
let seed = 7;
const random = () => (seed = (seed * 16807) % 2147483647) / 2147483647;
const pool: FilterAction[][] = [
[], [forward], [forward, forward], [forward, { type: 'discard' }],
[forward, { type: 'stop' }], [{ type: 'stop' }, forward], [{ type: 'mark_read' }],
];
const lists = Array.from({ length: 300 }, () =>
Array.from({ length: Math.floor(random() * 7) }, () =>
rule(pool[Math.floor(random() * pool.length)], random() < 0.4, random() < 0.8)));
it('finds the most forwards one message can collect', () => {
for (const rules of lists) expect(worstCaseForwards(rules)).toBe(mostCollected(rules));
});
it('finds the most a message the rule runs for can collect, for rules in place and new ones', () => {
for (const rules of lists) {
for (let i = 0; i < rules.length; i++) {
const r = rules[i];
if (!r.enabled) continue;
const { before, after } = forwardsAround(rules, i, true);
expect(before + ruleForwards(r) + (ruleStops(r) ? 0 : after)).toBe(mostCollected(rules, i));
// The same rule about to be put there, rather than there already.
const others = rules.filter((_, j) => j !== i);
expect(forwardsAround(others, i, false)).toEqual({ before, after });
}
}
});
});
+136
View File
@@ -0,0 +1,136 @@
import type { FilterRule, VacationForward } from './types';
// Ported from the webmail's lib/filters/forward-limit.ts (#1155, #1208).
/*
* A server lets one message trigger only so many redirects
* (maxNumberRedirects, 1 for user scripts on Stalwart by default) and skips
* the ones over that. The rules run in order, and a rule that stops ("Stop
* processing", or the Stop action) ends the script for the messages it
* matches: the forwards of the rules below it never add to its own. What
* counts is the most forwards a single message can collect, not how many
* forwards there are.
*/
type RuleForwards = Pick<FilterRule, 'enabled' | 'actions' | 'stopProcessing'>;
/** The forwards of a rule that run: those ahead of a Stop action. */
export function ruleForwards(rule: Pick<FilterRule, 'actions'>): number {
let forwards = 0;
for (const action of rule.actions) {
if (action.type === 'stop') break;
if (action.type === 'forward') forwards++;
}
return forwards;
}
/** The rule ends the script for the messages it matches. */
export function ruleStops(rule: Pick<FilterRule, 'actions' | 'stopProcessing'>): boolean {
return rule.stopProcessing || rule.actions.some((action) => action.type === 'stop');
}
/** The most forwards one message can collect going through `rules`, in their order. */
export function worstCaseForwards(rules: RuleForwards[]): number {
// From the rules so far that let the message go on.
let open = 0;
let worst = 0;
for (const rule of rules) {
if (!rule.enabled) continue;
const forwards = ruleForwards(rule);
if (ruleStops(rule)) worst = Math.max(worst, open + forwards);
else open += forwards;
}
return Math.max(worst, open);
}
/**
* For the rule at `index` of `rules` (in the order they run): the forwards a
* message can have collected when it reaches the rule, and the most it can
* still collect below it when the rule lets it go on. `replaces`: the rule
* is at `index` already, rather than about to be put there.
*/
export function forwardsAround(
rules: RuleForwards[],
index: number,
replaces: boolean,
): { before: number; after: number } {
// A rule above that stops lets only the messages it does not match go on,
// so its forwards never meet this rule's.
const before = rules
.slice(0, index)
.filter((rule) => rule.enabled && !ruleStops(rule))
.reduce((n, rule) => n + ruleForwards(rule), 0);
const after = worstCaseForwards(rules.slice(replaces ? index + 1 : index));
return { before, after };
}
/**
* The rules in the order the script runs them: forwarding from the out of
* office card goes ahead of every rule, and ends the script unless it keeps
* a copy here.
*/
export function inRunOrder(rules: RuleForwards[], forward: VacationForward | null | undefined): RuleForwards[] {
if (!forward?.enabled) return rules;
return [{ enabled: true, actions: [{ type: 'forward', value: forward.to }], stopProcessing: !forward.keepCopy }, ...rules];
}
// ── App additions: what the rule editor and the rule list show ──
type ListedRule = RuleForwards & Pick<FilterRule, 'id' | 'origin'>;
/**
* forwardsAround for the rule the editor has open in the filter list: the
* edited rule where it is, a new one below Bulwark's own rules (where
* addRule puts it), behind the out of office forwarding.
*/
export function forwardsOfEditedRule(
rules: ListedRule[],
forward: VacationForward | null | undefined,
editedRuleId: string | undefined,
): { before: number; after: number } {
const runOrder = inRunOrder(rules, forward);
const editedIndex = editedRuleId ? rules.findIndex((r) => r.id === editedRuleId) : -1;
const own = rules.filter((r) => r.origin !== 'external' && r.origin !== 'opaque').length;
return forwardsAround(runOrder, runOrder.length - rules.length + (editedIndex >= 0 ? editedIndex : own), editedIndex >= 0);
}
export interface RuleForwardLimit {
/** The server's cap on forwards per message, when it has one above 0. */
limit: number | null;
/** One message can collect more forwards than the cap with this rule. */
overLimit: boolean;
/** The server runs no forward at all (a cap of 0). */
unavailable: boolean;
/** Whether another action may become a forward. */
canAddForward: boolean;
}
/** What the rule editor says about the server's redirect cap for the rule as edited. */
export function ruleForwardLimit(
rule: Pick<FilterRule, 'actions' | 'stopProcessing'>,
maxRedirects: number | null | undefined,
around: { before: number; after: number },
): RuleForwardLimit {
const count = ruleForwards(rule);
const others = around.before + (ruleStops(rule) ? 0 : around.after);
const limit = typeof maxRedirects === 'number' && maxRedirects > 0 ? maxRedirects : null;
const unavailable = maxRedirects === 0;
return {
limit,
overLimit: limit !== null && count + others > limit,
unavailable,
canAddForward: !unavailable && (limit === null || count + others < limit),
};
}
/** What the rule list says: the rules' forwards over the cap, or ignored by a server without forwarding. */
export function rulesForwardLimit(
rules: RuleForwards[],
forward: VacationForward | null | undefined,
maxRedirects: number | null | undefined,
): { tooMany: boolean; ignored: boolean } {
return {
tooMany: typeof maxRedirects === 'number' && maxRedirects > 0 && worstCaseForwards(inRunOrder(rules, forward)) > maxRedirects,
ignored: maxRedirects === 0 && worstCaseForwards(rules) > 0,
};
}
+29
View File
@@ -0,0 +1,29 @@
// Stalwart refuses a vacation subject of 512 bytes or more and a text or
// HTML body of 2048 bytes or more, answering only "Field could not be set"
// (webmail components/settings/vacation-settings.tsx).
export const STALWART_VACATION_LIMITS = { subject: 511, body: 2047 } as const;
export type VacationLimits = { subject: number; body: number };
/** The UTF-8 length of `value` in bytes. */
export function utf8Length(value: string): number {
let bytes = 0;
for (const ch of value) {
const code = ch.codePointAt(0)!;
bytes += code < 0x80 ? 1 : code < 0x800 ? 2 : code < 0x10000 ? 3 : 4;
}
return bytes;
}
/** Which parts of the reply as a save sends it are over the server's limits; none without limits. */
export function vacationOversize(
reply: { subject: string; textBody: string; htmlBody: string | null },
limits: VacationLimits | null,
): { subject: boolean; body: boolean } {
if (!limits) return { subject: false, body: false };
return {
subject: utf8Length(reply.subject) > limits.subject,
body: utf8Length(reply.textBody) > limits.body
|| (reply.htmlBody !== null && utf8Length(reply.htmlBody) > limits.body),
};
}
+11
View File
@@ -346,6 +346,17 @@ describe('readVacationFilters', () => {
}); });
}); });
it('counts the most forwards the filter rules give one message, for the limit of the forwarding', async () => {
api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS });
api.getSieveScripts.mockResolvedValue([{ id: 's1', name: 'filters', blobId: 'b1', isActive: true }]);
const forward = { type: 'forward' as const, value: 'x@example.org' };
api.getSieveScriptContent.mockResolvedValue(generateScript([
makeRule({ id: 'a', actions: [forward] }),
makeRule({ id: 'b', actions: [forward] }),
]));
expect((await readVacationFilters()).otherForwards).toBe(2);
});
it('sets up forwarding in the vacation period, creating the script when there is none', async () => { it('sets up forwarding in the vacation period, creating the script when there is none', async () => {
api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS }); api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS });
api.getSieveScripts.mockResolvedValue([{ id: 'v1', name: 'vacation', blobId: 'bv', isActive: true }]); api.getSieveScripts.mockResolvedValue([{ id: 'v1', name: 'vacation', blobId: 'bv', isActive: true }]);
+1 -1
View File
@@ -18,7 +18,7 @@ vi.mock('../../api/sieve', () => ({ isSieveSupported: vi.fn(() => false) }));
const NO_FILTERS = { const NO_FILTERS = {
includesVacation: false, forward: null, forwardAvailable: false, includesVacation: false, forward: null, forwardAvailable: false,
audience: null, audienceAvailable: false, notRunning: false, audience: null, audienceAvailable: false, notRunning: false, otherForwards: 0,
}; };
vi.mock('../filter-store', () => ({ vi.mock('../filter-store', () => ({
+8
View File
@@ -7,6 +7,7 @@ import { generateScript, supportsSpamGuard, VACATION_SCRIPT_NAME } from '../lib/
import { supportsPeriods } from '../lib/sieve/period'; import { supportsPeriods } from '../lib/sieve/period';
import { isValidVacationForward, withVacationPeriod } from '../lib/sieve/vacation-forward'; import { isValidVacationForward, withVacationPeriod } from '../lib/sieve/vacation-forward';
import { isValidVacationAudience, normalizeVacationAudience } from '../lib/sieve/vacation-audience'; import { isValidVacationAudience, normalizeVacationAudience } from '../lib/sieve/vacation-audience';
import { worstCaseForwards } from '../lib/sieve/forward-limit';
import { import {
createSieveScript, createSieveScript,
getSieveAccountId, getSieveAccountId,
@@ -344,6 +345,12 @@ export interface VacationFilters {
* client), or no script runs at all. Saving the card sets it right. * client), or no script runs at all. Saving the card sets it right.
*/ */
notRunning: boolean; notRunning: boolean;
/**
* The most forwards the filter rules let one message collect (see
* worstCaseForwards). Forwarding that keeps a copy runs ahead of them, so
* it shares the server's redirect limit with these.
*/
otherForwards: number;
} }
/** See VacationFilters.notRunning. */ /** See VacationFilters.notRunning. */
@@ -381,6 +388,7 @@ export async function readVacationFilters(accountId?: string): Promise<VacationF
// Told apart by the envelope sender, which the auto-reply goes to. // Told apart by the envelope sender, which the auto-reply goes to.
audienceAvailable: filtersUsable && !!capabilities?.sieveExtensions?.includes('envelope'), audienceAvailable: filtersUsable && !!capabilities?.sieveExtensions?.includes('envelope'),
notRunning: storedButIdle(!!target?.isActive, !!vacationScript?.isActive, forward, audience), notRunning: storedButIdle(!!target?.isActive, !!vacationScript?.isActive, forward, audience),
otherForwards: worstCaseForwards(parsed?.rules ?? []),
}; };
} }
+4
View File
@@ -52,6 +52,8 @@ export interface VacationState {
audienceAvailable: boolean; audienceAvailable: boolean;
/** Stored forwarding or recipients do not run until the card is saved (see readVacationFilters). */ /** Stored forwarding or recipients do not run until the card is saved (see readVacationFilters). */
notRunning: boolean; notRunning: boolean;
/** The most forwards the filter rules let one message collect (see readVacationFilters). */
otherForwards: number;
isLoading: boolean; isLoading: boolean;
isSaving: boolean; isSaving: boolean;
error: string | null; error: string | null;
@@ -80,6 +82,7 @@ const NO_FILTERS = {
audience: null, audience: null,
audienceAvailable: false, audienceAvailable: false,
notRunning: false, notRunning: false,
otherForwards: 0,
}; };
const INITIAL = { const INITIAL = {
@@ -106,6 +109,7 @@ function fromFilters(filters: VacationFilters) {
audience: filters.audience, audience: filters.audience,
audienceAvailable: filters.audienceAvailable, audienceAvailable: filters.audienceAvailable,
notRunning: filters.notRunning, notRunning: filters.notRunning,
otherForwards: filters.otherForwards,
}; };
} }