From 89262a0be67d424cc4c30a566c128fb02bd8bfe1 Mon Sep 17 00:00:00 2001
From: Jean-Luc Makiola <business@jeanlucmakiola.de>
Date: Mon, 31 Aug 2026 19:11:18 +0200
Subject: [PATCH] fix(ics): reject an RRULE carrying both UNTIL and COUNT
 (#225)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

EventRecurrence.parse ends with two global checks, not one: a missing FREQ
throws, and so does UNTIL together with COUNT. Only the first was mirrored, so a
rule with both still threw out of insert — the failure sanitizeRrule exists to
prevent. UNTIL is kept; it bounds the series at a date a stale COUNT can't
outrun.

Also mark a part that carries text but no '=' as a repair. It took the branch
meant for a trailing ';', so the rule was altered while the user was told
nothing had changed.
---
 .../calendula/domain/ics/IcsRecurrence.kt     | 15 ++++++--
 .../calendula/domain/ics/IcsRecurrenceTest.kt | 34 +++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrence.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrence.kt
index abd659c..7f67573 100644
--- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrence.kt
+++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrence.kt
@@ -80,10 +80,12 @@ fun sanitizeRrule(raw: String?): SanitizedRrule {
     }
 
     for (part in rule.split(';')) {
+        // A stray separator ("FREQ=DAILY;") normalises away; nothing was lost.
+        // A part that carries text but no '=' ("FREQ=WEEKLY;BYDAY") is a part
+        // being dropped, so it falls through and marks the rule repaired.
+        if (part.isBlank()) continue
         val key = part.substringBefore('=', "").trim().uppercase()
         val value = if ('=' in part) part.substringAfter('=').trim() else ""
-        // A stray separator ("FREQ=DAILY;") normalises away; nothing was lost.
-        if (key.isEmpty() && value.isEmpty()) continue
         val kept = when {
             key.isEmpty() || value.isEmpty() -> null
             key == "FREQ" ->
@@ -105,6 +107,15 @@ fun sanitizeRrule(raw: String?): SanitizedRrule {
         }
         if (kept == null) repaired = true else parts += kept
     }
+    // `EventRecurrence.parse` ends with *two* global checks, not one: a missing
+    // FREQ throws, and so does an UNTIL that arrives together with a COUNT. A
+    // rule carrying both is malformed per RFC 5545 §3.3.10 but real producers
+    // emit it, and left alone it would throw straight out of `insert` — the very
+    // failure this function exists to prevent. UNTIL is kept: it bounds the
+    // series at an absolute date, so a stale COUNT can't over-generate past it.
+    if (parts.any { it.startsWith("UNTIL=") }) {
+        if (parts.removeAll { it.startsWith("COUNT=") }) repaired = true
+    }
     return if (freqSeen) {
         SanitizedRrule(rule = parts.joinToString(";"), repaired = repaired)
     } else {
diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrenceTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrenceTest.kt
index 1d196fd..6284708 100644
--- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrenceTest.kt
+++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrenceTest.kt
@@ -129,4 +129,38 @@ class IcsRecurrenceTest {
             .isEqualTo("FREQ=DAILY;UNTIL=20260819T235959Z")
         assertThat(sanitizeRrule("FREQ=DAILY;UNTIL=2026-08-19").rule).isEqualTo("FREQ=DAILY")
     }
+
+    @Test
+    fun `UNTIL and COUNT together keep UNTIL`() {
+        // EventRecurrence.parse throws outright on a rule carrying both.
+        val both = sanitizeRrule("FREQ=WEEKLY;COUNT=10;UNTIL=20260101T000000Z")
+
+        assertThat(both.rule).isEqualTo("FREQ=WEEKLY;UNTIL=20260101T000000Z")
+        assertThat(both.repaired).isTrue()
+    }
+
+    @Test
+    fun `a COUNT the UNTIL check never sees is left alone`() {
+        // An UNTIL dropped for its shape doesn't take the COUNT down with it.
+        val staleUntil = sanitizeRrule("FREQ=WEEKLY;COUNT=10;UNTIL=2026-01-01")
+
+        assertThat(staleUntil.rule).isEqualTo("FREQ=WEEKLY;COUNT=10")
+        assertThat(staleUntil.repaired).isTrue()
+    }
+
+    @Test
+    fun `a part without a value separator is reported as repaired`() {
+        val noEquals = sanitizeRrule("FREQ=WEEKLY;BYDAY")
+
+        assertThat(noEquals.rule).isEqualTo("FREQ=WEEKLY")
+        assertThat(noEquals.repaired).isTrue()
+    }
+
+    @Test
+    fun `a trailing separator is normalisation, not a repair`() {
+        val trailing = sanitizeRrule("FREQ=DAILY;")
+
+        assertThat(trailing.rule).isEqualTo("FREQ=DAILY")
+        assertThat(trailing.repaired).isFalse()
+    }
 }