fix(ics): harden the import against foreign files (#225)

Review pass over the Fossify import work:

- sanitizeRrule whitelists the parts EventRecurrence actually parses, and
  reports whether it dropped anything — normalising case or a stray ";" no
  longer warns the user about a faulty repeat rule.
- CATEGORIES only stands in for a calendar name when it is single-valued and
  every event agrees; elsewhere it is a tag list and must not pick the target.
- An all-day EXDATE keeps the day it spells out instead of the day its instant
  lands on in UTC.
- An imported all-day reminder is sampled where it will fire, not at a
  recurrence anchor that Fossify dates to 1970.
- Imported colours match against every published key, not the picker's
  curated subset.

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt

@@ -70,6 +70,34 @@ internal fun nextYearlyOccurrence(month: Int, day: Int, today: LocalDate): Local
     return LocalDate.of(today.year, month, day)
 }
 
+/**
+ * The date an **imported** all-day event's reminder offset is sampled at.
+ *
+ * A one-off fires once, on its own date. A series' `DTSTART` is only an anchor
+ * and may be ancient — Fossify writes a year-less contact birthday at 1970, a
+ * year whose rules predate DST across most of Europe, and a real birth year is
+ * usually older still — so sampling there skews every modern occurrence by the
+ * offset delta (the same trap [nextYearlyOccurrence] exists for). A yearly
+ * series is therefore sampled at its next occurrence of the anchor's month/day,
+ * which shares the season; any other series at [today], which its upcoming
+ * occurrences are near.
+ *
+ * The decode side ([fromProviderAllDayMinutes]) only asks which local *day* the
+ * encoded instant falls on, so it still reads the lead time back correctly from
+ * the anchor's own date.
+ */
+internal fun importedAllDayReminderDate(
+    startDate: LocalDate,
+    recurrenceRule: String?,
+    today: LocalDate,
+): LocalDate = when {
+    recurrenceRule.isNullOrBlank() -> startDate
+    recurrenceRule.contains("FREQ=YEARLY", ignoreCase = true) ->
+        nextYearlyOccurrence(startDate.monthValue, startDate.dayOfMonth, today)
+    startDate.isBefore(today) -> today
+    else -> startDate
+}
+
 /**
  * Recover the semantic whole-day lead time from a raw all-day reminder
  * [rawMinutes] — the inverse of [toProviderAllDayMinutes], for the form and the

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt

@@ -85,6 +85,15 @@ interface CalendarDataSource {
      */
     fun eventColorPalette(calendarId: Long): List<EventColorOption>
 
+    /**
+     * The same palette **uncurated** — every key the account publishes, in
+     * provider order. Curation is a display concession (it folds look-alikes and
+     * drops the neutrals outright from an oversized palette), so matching a
+     * colour that came from outside the account has to run against the full set:
+     * those keys are all the calendar accepts. See [eventColorPalette].
+     */
+    fun publishedEventColors(calendarId: Long): List<EventColorOption>
+
     /**
      * Every master/one-off event of the writable local calendars, mapped for a
      * whole-calendar `.ics` backup. Modified-occurrence and cancelled-exception
@@ -106,9 +115,10 @@ interface CalendarDataSource {
      * Insert a parsed `.ics` event into [calendarId], preserving its UID (or
      * minting one when absent); returns the new `Events._ID`.
      *
-     * [colorPalette] is the target account's published event colours, looked up
-     * once per import; [allDayReminderTimeMinutes] is the user's preferred
-     * all-day firing time, applied exactly as a hand-created event's is.
+     * [colorPalette] is the target account's published event colours
+     * ([publishedEventColors]), looked up once per import;
+     * [allDayReminderTimeMinutes] is the user's preferred all-day firing time,
+     * applied exactly as a hand-created event's is.
      */
     fun insertImportedEvent(
         event: ParsedIcsEvent,
@@ -737,7 +747,10 @@ class AndroidCalendarDataSource @Inject constructor(
         }
     }
 
-    override fun eventColorPalette(calendarId: Long): List<EventColorOption> {
+    override fun eventColorPalette(calendarId: Long): List<EventColorOption> =
+        publishedEventColors(calendarId).curatedForPicker()
+
+    override fun publishedEventColors(calendarId: Long): List<EventColorOption> {
         val account = calendarAccount(calendarId) ?: return emptyList()
         return resolver.query(
             CalendarContract.Colors.CONTENT_URI,
@@ -755,7 +768,6 @@ class AndroidCalendarDataSource @Inject constructor(
             c.mapAll { EventColorOption(key = it.getString(0).orEmpty(), argb = it.getInt(1)) }
         }
             ?.filter { it.key.isNotEmpty() }
-            ?.curatedForPicker()
             ?: emptyList()
     }
 
@@ -835,16 +847,27 @@ class AndroidCalendarDataSource @Inject constructor(
         val uri = resolver.insert(CalendarContract.Events.CONTENT_URI, values.toContentValues())
             ?: throw WriteFailedException("import event into calendar id=$calendarId")
         val eventId = ContentUris.parseId(uri)
-        val startDate = Instant.ofEpochMilli(event.start.toEpochMilliseconds())
-            .atZone(ZoneOffset.UTC).toLocalDate()
+        val zone = ZoneId.systemDefault()
+        // An all-day reminder is stored the same way a hand-created one is, so it
+        // fires at the time the user picked rather than at UTC midnight — sampled
+        // where it will actually fire, not at an ancient recurrence anchor
+        // (see [importedAllDayReminderDate]).
+        val reminderDate = if (event.isAllDay) {
+            importedAllDayReminderDate(
+                startDate = Instant.ofEpochMilli(event.start.toEpochMilliseconds())
+                    .atZone(ZoneOffset.UTC).toLocalDate(),
+                recurrenceRule = event.recurrenceRule,
+                today = LocalDate.now(zone),
+            )
+        } else {
+            null
+        }
         event.semanticReminderMinutes().forEach { minutes ->
-            // An all-day reminder is stored the same way a hand-created one is,
-            // so it fires at the time the user picked rather than at UTC midnight.
-            val providerMinutes = if (event.isAllDay) {
+            val providerMinutes = if (reminderDate != null) {
                 toProviderAllDayMinutes(
                     semanticMinutes = minutes,
-                    startDate = startDate,
-                    zone = ZoneId.systemDefault(),
+                    startDate = reminderDate,
+                    zone = zone,
                     timeOfDayMinutes = allDayReminderTimeMinutes,
                 )
             } else {

app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt

@@ -220,8 +220,10 @@ class CalendarRepositoryImpl @Inject constructor(
     ): IcsImportSummary = withContext(io) {
         val existing = dataSource.existingUids(targetCalendarId)
         // Both are per-calendar, not per-event: looking them up once keeps a
-        // thousand-event restore to two extra queries.
-        val palette = dataSource.eventColorPalette(targetCalendarId)
+        // thousand-event restore to two extra queries. The palette is the
+        // uncurated one — an imported colour is matched against every key the
+        // account accepts, not the subset the picker shows.
+        val palette = dataSource.publishedEventColors(targetCalendarId)
         val allDayMinutes = allDayReminderTimeMinutes()
         var imported = 0
         var skipped = 0

app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt

@@ -186,6 +186,7 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
         var status = EventStatus.Confirmed
         var availability = Availability.Busy
         var calendarName = fileCalendarName
+        var category: String? = null
         var eventColor: Int? = null
         var calendarColor: Int? = null
         val reminders = mutableListOf<Int>()
@@ -223,10 +224,10 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
                 "X-CALENDULA-CALENDAR" ->
                     calendarName = unescapeText(line.value).trim().ifEmpty { calendarName }
                 // Fossify names the source calendar per event; it never writes
-                // an X-WR-CALNAME, so this is the only calendar label its files carry.
-                "CATEGORIES" -> calendarName = unescapeText(line.value).trim()
-                    .takeIf { it.isNotEmpty() && !it.equals("null", true) }
-                    ?: calendarName
+                // an X-WR-CALNAME, so this is the only calendar label its files
+                // carry. Elsewhere CATEGORIES is a plain tag list, so it only
+                // stands in where nothing named the calendar outright.
+                "CATEGORIES" -> category = singleCategory(line.value) ?: category
                 // Per-event colour, most specific encoding first (see parseIcsColorValue).
                 "X-FOSSIFY-EVENT-COLOR", "X-SMT-EVENT-COLOR" ->
                     eventColor = parseIcsColorValue(line.value) ?: eventColor
@@ -247,10 +248,8 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
             warnings.add(IcsParseWarning.EventWithoutStartSkipped)
             return null
         }
-        val cleanRrule = sanitizeRrule(rrule)
-        if (rrule != null && cleanRrule != rrule.removePrefix("RRULE:").trim()) {
-            warnings.add(IcsParseWarning.RecurrenceRuleRepaired)
-        }
+        val recurrence = sanitizeRrule(rrule)
+        if (recurrence.repaired) warnings.add(IcsParseWarning.RecurrenceRuleRepaired)
         val end = resolveEnd(start, dtEnd, duration)
         return ParsedIcsEvent(
             uid = uid,
@@ -259,15 +258,19 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
             end = end,
             isAllDay = start.isAllDay,
             zoneId = start.zoneId,
-            recurrenceRule = cleanRrule,
+            recurrenceRule = recurrence.rule,
             location = location,
             description = description,
             reminderMinutes = reminders.distinct(),
             status = status,
             availability = availability,
-            calendarName = calendarName,
+            calendarName = calendarName ?: category,
             color = eventColor ?: calendarColor,
-            exDates = if (cleanRrule == null) emptyList() else normalizeExDates(exDateLines, start),
+            exDates = if (recurrence.rule == null) {
+                emptyList()
+            } else {
+                normalizeExDates(exDateLines, start)
+            },
             isTask = isTask,
         )
     }
@@ -302,6 +305,11 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
      * out that way even for timed series, where the property is then ambiguous.
      * Such a value is resolved against the series' own time of day, which is
      * what it meant — its repetitions never move within the day.
+     *
+     * An all-day exclusion names a calendar day, so a zone-qualified DATE-TIME
+     * still means the day it spells out; resolving it to an instant first and
+     * reading *that* back in UTC would roll a midnight east of UTC onto the day
+     * before and exclude an occurrence that doesn't exist.
      */
     private fun normalizeExDates(lines: List<IcsContentLine>, start: IcsDateTime): List<String> {
         if (lines.isEmpty()) return emptyList()
@@ -310,17 +318,12 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
         return lines
             .flatMap { line -> line.value.split(',').map { line to it.trim() } }
             .mapNotNull { (line, token) ->
-                if (token.isEmpty()) return@mapNotNull null
-                if (token.contains('T')) {
-                    val instant = parseExDateTime(token, line, startZone) ?: return@mapNotNull null
-                    if (start.isAllDay) utcDayCode(instant) else utcStamp(instant)
-                } else {
-                    val date = parseBasicDate(token) ?: return@mapNotNull null
-                    if (start.isAllDay) {
-                        utcDayCode(date.atStartOfDayIn(TimeZone.UTC))
-                    } else {
-                        utcStamp(LocalDateTime(date, startTime).toInstant(startZone))
-                    }
+                when {
+                    token.isEmpty() -> null
+                    start.isAllDay -> parseBasicDate(token.substringBefore('T'))?.let(::dayCode)
+                    token.contains('T') -> parseExDateTime(token, line, startZone)?.let(::utcStamp)
+                    else -> parseBasicDate(token)
+                        ?.let { utcStamp(LocalDateTime(it, startTime).toInstant(startZone)) }
                 }
             }
             .distinct()
@@ -391,14 +394,31 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
         fun Instant.plusDays(days: Int): Instant =
             Instant.fromEpochMilliseconds(toEpochMilliseconds() + days * DAY_MILLIS)
 
-        fun utcDayCode(instant: Instant): String = with(instant.toLocalDateTime(TimeZone.UTC)) {
-            "%04d%02d%02d".format(year, month.number, day)
-        }
+        fun dayCode(date: LocalDate): String =
+            "%04d%02d%02d".format(date.year, date.month.number, date.day)
 
         fun utcStamp(instant: Instant): String = with(instant.toLocalDateTime(TimeZone.UTC)) {
             "%04d%02d%02dT%02d%02d%02dZ".format(year, month.number, day, hour, minute, second)
         }
 
+        /**
+         * A single-valued `CATEGORIES`, which is what the Fossify family writes
+         * for the source calendar's name. A multi-item list is a tag list and
+         * names no calendar, so it is ignored.
+         */
+        fun singleCategory(raw: String): String? {
+            var escaped = false
+            for (c in raw) {
+                when {
+                    escaped -> escaped = false
+                    c == '\\' -> escaped = true
+                    c == ',' -> return null
+                }
+            }
+            return unescapeText(raw).trim()
+                .takeIf { it.isNotEmpty() && !it.equals("null", true) }
+        }
+
         /** Index of the matching `END:<component>` at/after [from], or list end. */
         fun indexOfEnd(lines: List<String>, from: Int, component: String): Int {
             var i = from

app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsRecurrence.kt

@@ -4,15 +4,50 @@ private val VALID_FREQ = setOf(
     "SECONDLY", "MINUTELY", "HOURLY", "DAILY", "WEEKLY", "MONTHLY", "YEARLY",
 )
 
-/** RRULE parts whose value is a comma-separated list, so empty items can be dropped. */
-private val LIST_PARTS = setOf(
-    "BYSECOND", "BYMINUTE", "BYHOUR", "BYDAY", "BYMONTHDAY", "BYYEARDAY",
-    "BYWEEKNO", "BYMONTH", "BYSETPOS",
-)
+private val VALID_WEEKDAYS = setOf("SU", "MO", "TU", "WE", "TH", "FR", "SA")
 
 /**
- * Make a foreign RRULE safe to hand `CalendarContract`, or return null when it
- * can't be salvaged.
+ * What `EventRecurrence` accepts in each numeric list part: a value range, plus
+ * whether `0` is legal. Its `parseNumberList` throws on a non-number, on an item
+ * outside the range *and* on a zero in the by-position parts, so checking only
+ * that an item is a number would still hand the provider a rule it rejects.
+ *
+ * `BYSETPOS` is the one part it leaves unbounded; `0` is dropped from it anyway
+ * because RFC 5545 §3.3.10 gives it no meaning.
+ */
+private val NUMERIC_LIST_PARTS = mapOf(
+    "BYSECOND" to NumericPart(0..59, zeroAllowed = true),
+    "BYMINUTE" to NumericPart(0..59, zeroAllowed = true),
+    "BYHOUR" to NumericPart(0..23, zeroAllowed = true),
+    "BYMONTHDAY" to NumericPart(-31..31, zeroAllowed = false),
+    "BYYEARDAY" to NumericPart(-366..366, zeroAllowed = false),
+    "BYWEEKNO" to NumericPart(-53..53, zeroAllowed = false),
+    "BYMONTH" to NumericPart(1..12, zeroAllowed = false),
+    "BYSETPOS" to NumericPart(Int.MIN_VALUE..Int.MAX_VALUE, zeroAllowed = false),
+)
+
+private class NumericPart(private val range: IntRange, private val zeroAllowed: Boolean) {
+    fun accepts(item: String): Boolean {
+        val n = item.toIntOrNull() ?: return false
+        return n in range && (zeroAllowed || n != 0)
+    }
+}
+
+/** `UNTIL` in RFC 5545 basic format; anything else fails at expansion time. */
+private val UNTIL_SHAPE = Regex("""\d{8}(T\d{6}Z?)?""")
+
+/**
+ * A foreign RRULE made safe to hand `CalendarContract`.
+ *
+ * [rule] is what survived, or null when there is nothing left to salvage.
+ * [repaired] is true only when a part was actually dropped — pure normalisation
+ * (case, a stray separator) doesn't count, so a conformant file is never
+ * reported to the user as faulty.
+ */
+data class SanitizedRrule(val rule: String?, val repaired: Boolean)
+
+/**
+ * Salvage a foreign RRULE.
  *
  * The provider validates every RRULE through `EventRecurrence.parse`, which
  * **throws** on a malformed rule — and the throw surfaces from `insert`, not
@@ -22,34 +57,65 @@ private val LIST_PARTS = setOf(
  * (`Parser.getByDay` interpolates an empty day string unconditionally), which is
  * the state every weekly event it imported from elsewhere is left in.
  *
- * Empty and malformed parts are dropped rather than failing the rule; only a
- * missing or unrecognised `FREQ` — without which there is no rule at all —
- * returns null.
+ * That parser has a fixed table of parts and throws on any name outside it, so
+ * this is a whitelist: an unrecognised part (RFC 7529 `RSCALE`, a vendor `X-`
+ * extension) and an unusable value are dropped rather than passed on to fail the
+ * whole event. Only a missing or unrecognised `FREQ` — without which there is no
+ * rule at all — gives up entirely.
  */
-fun sanitizeRrule(raw: String?): String? {
+fun sanitizeRrule(raw: String?): SanitizedRrule {
     val rule = raw?.trim()?.removePrefix("RRULE:")?.trim().orEmpty()
-    if (rule.isEmpty()) return null
+    if (rule.isEmpty()) return SanitizedRrule(rule = null, repaired = false)
 
     var freqSeen = false
-    val parts = rule.split(';').mapNotNull { part ->
+    var repaired = false
+    val parts = mutableListOf<String>()
+
+    /** The items [valid] accepts, uppercased and comma-joined; null if none survive. */
+    fun keepItems(value: String, valid: (String) -> Boolean): String? {
+        val items = value.split(',').map { it.trim().uppercase() }
+        val kept = items.filter(valid)
+        if (kept.size != items.size) repaired = true
+        return kept.takeIf { it.isNotEmpty() }?.joinToString(",")
+    }
+
+    for (part in rule.split(';')) {
         val key = part.substringBefore('=', "").trim().uppercase()
         val value = if ('=' in part) part.substringAfter('=').trim() else ""
-        if (key.isEmpty() || value.isEmpty()) return@mapNotNull null
-        when {
-            key == "FREQ" -> {
-                if (value.uppercase() !in VALID_FREQ) return null
-                freqSeen = true
-                "FREQ=${value.uppercase()}"
-            }
-            // A non-positive or non-numeric INTERVAL is rejected by the provider.
+        // 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" ->
+                value.uppercase().takeIf { it in VALID_FREQ }
+                    ?.also { freqSeen = true }
+                    ?.let { "FREQ=$it" }
+            // A non-positive or non-numeric INTERVAL/COUNT is rejected by the
+            // provider, and a COUNT of zero would expand to no occurrences.
             key == "INTERVAL" -> value.toIntOrNull()?.takeIf { it > 0 }?.let { "INTERVAL=$it" }
-            key in LIST_PARTS -> value.split(',')
-                .map { it.trim() }
-                .filter { it.isNotEmpty() }
-                .takeIf { it.isNotEmpty() }
-                ?.let { "$key=${it.joinToString(",")}" }
-            else -> "$key=$value"
+            key == "COUNT" -> value.toIntOrNull()?.takeIf { it > 0 }?.let { "COUNT=$it" }
+            key == "UNTIL" ->
+                value.uppercase().takeIf { UNTIL_SHAPE.matches(it) }?.let { "UNTIL=$it" }
+            key == "WKST" ->
+                value.uppercase().takeIf { it in VALID_WEEKDAYS }?.let { "WKST=$it" }
+            key == "BYDAY" -> keepItems(value, ::isWeekdayItem)?.let { "BYDAY=$it" }
+            key in NUMERIC_LIST_PARTS ->
+                keepItems(value, NUMERIC_LIST_PARTS.getValue(key)::accepts)?.let { "$key=$it" }
+            else -> null
         }
+        if (kept == null) repaired = true else parts += kept
+    }
+    return if (freqSeen) {
+        SanitizedRrule(rule = parts.joinToString(";"), repaired = repaired)
+    } else {
+        SanitizedRrule(rule = null, repaired = true)
     }
-    return if (freqSeen) parts.joinToString(";") else null
+}
+
+/** A `BYDAY` item: a two-letter weekday, optionally preceded by an ordinal. */
+private fun isWeekdayItem(item: String): Boolean {
+    if (item.length < 2) return false
+    val ordinal = item.dropLast(2)
+    return item.takeLast(2) in VALID_WEEKDAYS &&
+        (ordinal.isEmpty() || ordinal.toIntOrNull() != null)
 }

app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt

@@ -44,9 +44,9 @@ sealed interface ImportUiState {
         val warnings: Set<IcsParseWarning>,
         val calendars: List<CalendarSource>,
         /**
-         * The calendar the file says its events came from, when it names exactly
-         * one — used to preselect a matching target. Null when the file names
-         * none or its events disagree.
+         * The calendar the file says its events came from, when every one of
+         * them names the same — used to preselect a matching target (see
+         * [fileCalendarName]).
          */
         val fileCalendarName: String? = null,
     ) : ImportUiState
@@ -98,10 +98,7 @@ class ImportViewModel @Inject constructor(
                         warnings = parsed.warnings,
                         calendars = repository.calendars().first()
                             .filter { it.isEventTarget },
-                        fileCalendarName = parsed.events
-                            .mapNotNull { it.calendarName }
-                            .distinct()
-                            .singleOrNull(),
+                        fileCalendarName = fileCalendarName(parsed.events),
                     )
                 }
             }
@@ -124,6 +121,17 @@ class ImportViewModel @Inject constructor(
     }
 }
 
+/**
+ * The one calendar [events] all came from, or null.
+ *
+ * Every event has to agree, unnamed ones included: a foreign file where a single
+ * `VEVENT` carries a `CATEGORIES` tag and the rest carry nothing names no
+ * calendar — it just has one tagged event, and letting it pick the target would
+ * land the whole file wherever that tag happens to match.
+ */
+internal fun fileCalendarName(events: List<ParsedIcsEvent>): String? =
+    events.map { it.calendarName }.distinct().singleOrNull()
+
 /**
  * The target calendar to preselect for a bulk import.
  *