diff --git a/src/lib/sieve/__tests__/external-rules.test.ts b/src/lib/sieve/__tests__/external-rules.test.ts index b5271ee6..bc795147 100644 --- a/src/lib/sieve/__tests__/external-rules.test.ts +++ b/src/lib/sieve/__tests__/external-rules.test.ts @@ -253,6 +253,58 @@ describe('external rule preservation (issue #201)', () => { }); }); + // elsif and else were taken for statements of their own and cut at the + // first ";" in their body: a save wrote the chain back without its closing + // brace and without the comments after it, and the server refused it. + describe('if blocks with elsif and else', () => { + const read = (file: string) => + readFileSync(join(__dirname, 'fixtures', file), 'utf-8').replace(/\r\n/g, '\n'); + const resave = (script: string) => { + const parsed = parseScript(script); + return generateScript(parsed.rules, parsed.vacation, { + externalRequires: parsed.externalRequires, + extensions: ['fileinto', 'copy', 'imap4flags', 'mailbox', 'mailboxid'], + }); + }; + + it('keeps a hand-written if/elsif/else chain whole', () => { + const fixture = read('hand-written-elsif.sieve'); + const parsed = parseScript(fixture); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules).toHaveLength(1); + expect(parsed.rules[0].origin).toBe('opaque'); + expect(parsed.rules[0].name).toBe('Sorted by hand'); + + const saved = resave(fixture); + expect(saved).toContain(fixture.slice(fixture.indexOf('# Sorted by hand')).trim()); + expect(resave(saved)).toBe(saved); + }); + + it('keeps the category tabs section with its END marker', () => { + const fixture = read('category-tabs.sieve'); + const parsed = parseScript(fixture); + expect(parsed.rules.filter((r) => !r.origin)).toHaveLength(4); + const external = parsed.rules.filter((r) => r.origin); + expect(external).toHaveLength(1); + expect(external[0].origin).toBe('opaque'); + + const section = fixture.slice(fixture.indexOf('# BEGIN category-tabs')).trim(); + expect(external[0].rawBlock!.trim()).toBe(section); + const saved = resave(fixture); + expect(saved).toContain(section); + expect(saved.trimEnd().endsWith('# END category-tabs')).toBe(true); + expect(resave(saved)).toBe(saved); + }); + + it('still reads a lone if as a rule', () => { + const parsed = parseScript('require ["fileinto"];\nif header :contains "from" "a@example.org" {\n fileinto "A";\n}\n# trailing note\n'); + expect(parsed.rules).toHaveLength(1); + expect(parsed.rules[0].origin).toBe('external'); + expect(parsed.rules[0].actions).toEqual([{ type: 'move', value: 'A' }]); + expect(parsed.rules[0].rawBlock).toContain('# trailing note'); + }); + }); + describe('round-trip', () => { it('preserves external rules through parse → generate → parse', () => { const initial = `require ["fileinto", "imap4flags"];\n\n# rule:[VIP]\nif header :contains "From" "boss@company.com" {\n fileinto "VIP";\n addflag "\\\\Flagged";\n}\n\n# Nextcloud Mail - begin\nif header :contains "Subject" "invoice" {\n fileinto "Finance";\n}\n# Nextcloud Mail - end\n`; diff --git a/src/lib/sieve/__tests__/fixtures/category-tabs.sieve b/src/lib/sieve/__tests__/fixtures/category-tabs.sieve new file mode 100644 index 00000000..e1f64a7c --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/category-tabs.sieve @@ -0,0 +1,41 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"invoices","name":"Invoices","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"contains","value":["invoice","Rechnung"]}],"actions":[{"type":"move","value":"Finance/Invoices","mailboxId":"m12"},{"type":"star"},{"type":"add_label","value":"finance"}],"stopProcessing":false},{"id":"backup","name":"Backup","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"copy","value":"Archive","mailboxId":"m7"}],"stopProcessing":false},{"id":"legacy","name":"Legacy path","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"move","value":"Old/Path"}],"stopProcessing":false},{"id":"fwd","name":"Forward","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"forward","value":"me@elsewhere.example","keepCopy":true}],"stopProcessing":false}]} +@metadata:end */ + +require ["copy", "fileinto", "imap4flags", "mailbox", "mailboxid"]; + +# Rule: Invoices +if header :contains "Subject" ["invoice", "Rechnung"] { + addflag "\\Flagged"; + addflag "$label:finance"; + fileinto :mailboxid "m12" "Finance/Invoices"; +} + +# Rule: Backup +if header :contains "From" "news@example.com" { + fileinto :copy :mailboxid "m7" "Archive"; +} + +# Rule: Legacy path +if header :contains "From" "news@example.com" { + fileinto "Old/Path"; +} + +# Rule: Forward +if header :contains "From" "news@example.com" { + redirect :copy "me@elsewhere.example"; +} + +# BEGIN category-tabs (managed by the Inbox Category Tabs plugin) +if header :contains "from" ["friend@example.org"] { } +elsif header :contains "from" ["shop@example.com", "deals@example.net"] { + setflag "$category-promotions"; +} +elsif anyof ( + header :contains "subject" ["newsletter"], + exists "list-unsubscribe", + header :contains "precedence" ["bulk", "list"] +) { + setflag "$category-updates"; +} +# END category-tabs diff --git a/src/lib/sieve/__tests__/fixtures/hand-written-elsif.sieve b/src/lib/sieve/__tests__/fixtures/hand-written-elsif.sieve new file mode 100644 index 00000000..f38e141d --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/hand-written-elsif.sieve @@ -0,0 +1,15 @@ +require ["fileinto", "imap4flags"]; + +# Sorted by hand +if header :contains "from" "boss@example.org" { + addflag "\\Flagged"; + fileinto "Work"; +} elsif header :contains "list-id" "lists.example.org" { + fileinto "Lists"; +} +# Everything else stays in the inbox +else { + keep; +} + +# end of my rules diff --git a/src/lib/sieve/parser.ts b/src/lib/sieve/parser.ts index 38d9172f..ff115a9e 100644 --- a/src/lib/sieve/parser.ts +++ b/src/lib/sieve/parser.ts @@ -206,9 +206,21 @@ function skipIfStatement(s: string, i: number): number { return skipBalanced(s, i, '{', '}'); } +// Past whitespace and comments, to the next token. +function skipSpaceAndComments(s: string, i: number): number { + while (i < s.length) { + if (/\s/.test(s[i])) { i++; continue; } + if (s[i] === '#') { i = skipHashComment(s, i); continue; } + if (s[i] === '/' && s[i + 1] === '*') { i = skipBlockComment(s, i); continue; } + break; + } + return i; +} + interface TopBlock { kind: 'require' | 'if' | 'vacation' | 'other'; raw: string; // from start-of-leading-text to end of statement + leading: string; // the comments and whitespace in front of the statement statement: string; // the statement itself (no leading comments/whitespace) startIdx: number; endIdx: number; @@ -223,6 +235,7 @@ function scanTopLevel(content: string): TopBlock[] { blocks.push({ kind, raw: content.slice(segmentStart, stmtEnd), + leading: content.slice(segmentStart, stmtStart), statement: content.slice(stmtStart, stmtEnd), startIdx: segmentStart, endIdx: stmtEnd, @@ -254,6 +267,15 @@ function scanTopLevel(content: string): TopBlock[] { consume('require', stmtStart, i); } else if (ident === 'if') { i = skipIfStatement(content, i); + // elsif and else belong to the if in front of them. Taken as + // statements of their own they were cut at the first ";" in their + // body, and a save wrote back a block without its closing brace. + for (;;) { + const next = skipSpaceAndComments(content, i); + const branch = /^(elsif|else)(?![a-zA-Z0-9_])/.exec(content.slice(next, next + 6)); + if (!branch) break; + i = skipIfStatement(content, next + branch[1].length); + } consume('if', stmtStart, i); } else if (ident === 'vacation') { i = skipStatement(content, i); @@ -264,6 +286,15 @@ function scanTopLevel(content: string): TopBlock[] { } } + // Comments after the last statement, such as the "# END category-tabs" + // marker a plugin closes its section with, go with that statement, or a + // save would drop them. + const last = blocks[blocks.length - 1]; + if (last && content.slice(segmentStart).trim() !== '') { + last.raw += content.slice(segmentStart); + last.endIdx = content.length; + } + return blocks; } @@ -632,6 +663,8 @@ function parseIfBlockToRule(block: TopBlock, idPrefix: string, index: number): F // statement always terminates at the matching `}`. const lastBraceIdx = afterIf.length - 1; if (braceIdx === -1 || afterIf[lastBraceIdx] !== '}' || lastBraceIdx < braceIdx) return null; + // An if with elsif or else branches stays as written. + if (skipBalanced(afterIf, braceIdx, '{', '}') !== afterIf.length) return null; const condStr = afterIf.slice(0, braceIdx).trim(); const bodyStr = afterIf.slice(braceIdx + 1, lastBraceIdx).trim(); @@ -657,7 +690,7 @@ function parseIfBlockToRule(block: TopBlock, idPrefix: string, index: number): F } } - const leading = block.raw.slice(0, block.statement ? block.raw.length - block.statement.length : 0); + const leading = block.leading; const originLabel = detectOriginLabel(leading); const name = extractName(leading, `Rule ${index + 1}`); @@ -676,7 +709,7 @@ function parseIfBlockToRule(block: TopBlock, idPrefix: string, index: number): F } function makeOpaqueRule(block: TopBlock, idPrefix: string, index: number): FilterRule { - const leading = block.raw.slice(0, block.raw.length - block.statement.length); + const leading = block.leading; const originLabel = detectOriginLabel(leading); const name = extractName(leading, `External rule ${index + 1}`); return {