From 6abc243e5d40bd60174a3fb1affc9517b0ac5617 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 19 Aug 2026 09:09:36 +0200 Subject: feat(render): iCalendar emitter, RFC 5545 Not a template job: folding, escaping, exclusive DTEND and stable UIDs are rules a logic-less template cannot enforce, and each fails silently in a subscriber's client rather than loudly at generation. DTEND is EXCLUSIVE for an all-day event (section 3.6.1). Wrong here shows every event a day short, everywhere. UIDs are YYYYMMDD-@colitur and stable across regenerations (section 3.8.4.7). Wrong here duplicates the whole year in every subscriber's phone, months later. Every line is CRLF-terminated and folded at 75 octets (section 3.1). No RRULE: a liturgical calendar is not a recurrence rule. Asserted, so nobody optimises it later. DTSTAMP is a parameter, not a clock read. RFC 5545 requires it and the obvious implementation reads the wall clock -- which violates the kernel's determinism rule and would make two feeds from identical data differ byte-for-byte, defeating reproducible builds and any reviewable diff on a published tree. Corrected one test literal against real engine output: DTSTAMP is a per-VEVENT property (section 3.8.7.2), not calendar-level, so the default-value line count is 365 (every event), not 1. Mutation-tested: a non-exclusive DTEND reddens the suite. --- test/test_ics.ml | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 test/test_ics.ml (limited to 'test/test_ics.ml') diff --git a/test/test_ics.ml b/test/test_ics.ml new file mode 100644 index 0000000..ac0c9ee --- /dev/null +++ b/test/test_ics.ml @@ -0,0 +1,100 @@ +module Ics = Colitur_render.Emit_ics + +let out_2027 () = Ics.year (Test_view.view_of 2027) + +let lines s = + String.split_on_char '\n' s + |> List.map (fun l -> if l <> "" && l.[String.length l - 1] = '\r' then String.sub l 0 (String.length l - 1) else l) + |> List.filter (fun l -> l <> "") + +let count_prefix p ls = + List.length (List.filter (fun l -> String.length l >= String.length p && String.sub l 0 (String.length p) = p) ls) + +let test_envelope () = + let ls = lines (out_2027 ()) in + Alcotest.(check string) "first line" "BEGIN:VCALENDAR" (List.hd ls); + Alcotest.(check string) "last line" "END:VCALENDAR" (List.nth ls (List.length ls - 1)); + Alcotest.(check int) "version" 1 (count_prefix "VERSION:2.0" ls); + Alcotest.(check int) "prodid" 1 (count_prefix "PRODID:" ls); + Alcotest.(check int) "365 events" 365 (count_prefix "BEGIN:VEVENT" ls) + +(* RFC 5545 section 3.6.1: for an all-day event DTEND is EXCLUSIVE. Getting + this wrong shows every event a day short in every client. *) +let test_dtend_is_exclusive () = + let ls = lines (out_2027 ()) in + let starts = List.filter_map (fun l -> + if count_prefix "DTSTART;VALUE=DATE:" [l] = 1 then + Some (String.sub l (String.length "DTSTART;VALUE=DATE:") 8) else None) ls in + let ends = List.filter_map (fun l -> + if count_prefix "DTEND;VALUE=DATE:" [l] = 1 then + Some (String.sub l (String.length "DTEND;VALUE=DATE:") 8) else None) ls in + Alcotest.(check int) "same count" (List.length starts) (List.length ends); + Alcotest.(check string) "1 Jan starts" "20270101" (List.hd starts); + Alcotest.(check string) "1 Jan ends on the 2nd" "20270102" (List.hd ends); + Alcotest.(check string) "31 Dec ends on 1 Jan next" "20280101" + (List.nth ends (List.length ends - 1)) + +let test_uids_unique_and_shaped () = + let ls = lines (out_2027 ()) in + let uids = List.filter_map (fun l -> + if count_prefix "UID:" [l] = 1 then Some (String.sub l 4 (String.length l - 4)) else None) ls in + Alcotest.(check int) "365 uids" 365 (List.length uids); + let sorted = List.sort_uniq String.compare uids in + Alcotest.(check int) "all unique" 365 (List.length sorted); + Alcotest.(check string) "shape" "20270101-ef@colitur" (List.hd uids) + +(* The rule that punishes silently, months later, in someone else's phone: + regenerating a feed must not duplicate every event. *) +let test_output_is_byte_stable_across_runs () = + Alcotest.(check string) "two runs identical" (out_2027 ()) (out_2027 ()) + +let test_every_line_folded_and_crlf () = + let raw = out_2027 () in + List.iter + (fun l -> + if String.length l > 75 then Alcotest.failf "line of %d octets: %s" (String.length l) l) + (lines raw); + (* Every physical line ends CRLF (RFC 5545 section 3.1). *) + let n = String.length raw in + let rec check i = + if i >= n then () + else if raw.[i] = '\n' then ( + if i = 0 || raw.[i - 1] <> '\r' then Alcotest.failf "bare LF at offset %d" i; + check (i + 1)) + else check (i + 1) + in + check 0 + +let test_no_rrule () = + Alcotest.(check int) "no RRULE" 0 (count_prefix "RRULE" (lines (out_2027 ()))) + +let test_text_escaped () = + let ls = lines (Ics.year (Test_view.view_of 2035)) in + (* 2035-04-03 is St Joseph, whose English name contains a comma. *) + let joseph = List.find (fun l -> count_prefix "SUMMARY:St. Joseph" [l] = 1) ls in + Alcotest.(check bool) "comma escaped" true + (let re = "\\," in + let n = String.length re in + let rec f i = i + n <= String.length joseph && (String.sub joseph i n = re || f (i + 1)) in + f 0) + +let test_dtstamp_is_a_parameter () = + let a = Ics.year ~dtstamp:"20200101T000000Z" (Test_view.view_of 2027) in + let b = Ics.year ~dtstamp:"20210101T000000Z" (Test_view.view_of 2027) in + Alcotest.(check bool) "differs with the parameter" true (a <> b); + (* DTSTAMP is a per-VEVENT property (RFC 5545 section 3.8.7.2), not a + calendar-level one -- every one of the 365 events carries the same + default value, not just one line in the whole feed. *) + Alcotest.(check int) "default is deterministic" 365 + (count_prefix "DTSTAMP:20270101T000000Z" (lines (out_2027 ()))) + +let suite = + ( "Emit/ics", + [ Alcotest.test_case "envelope" `Quick test_envelope; + Alcotest.test_case "DTEND exclusive" `Quick test_dtend_is_exclusive; + Alcotest.test_case "uids unique and shaped" `Quick test_uids_unique_and_shaped; + Alcotest.test_case "byte-stable across runs" `Quick test_output_is_byte_stable_across_runs; + Alcotest.test_case "folded and CRLF" `Quick test_every_line_folded_and_crlf; + Alcotest.test_case "no RRULE" `Quick test_no_rrule; + Alcotest.test_case "text escaped" `Quick test_text_escaped; + Alcotest.test_case "DTSTAMP is a parameter" `Quick test_dtstamp_is_a_parameter ] ) -- cgit v1.3 From d71504ad9fc39b735689da32fa0be8a63c2cc7f8 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Wed, 19 Aug 2026 09:26:17 +0200 Subject: fix(render): omit DTEND at the domain's own last day, 9999-12-31 F1: Date.add_days is UNBOUNDED (date.mli) -- only Date.make enforces 1583..9999 -- and Date.to_iso8601 pads but never truncates, so 9999-12-31's naive successor formatted as "10000-01-01", and compact turned that into a 9-digit, non-conformant DATE on the last VEVENT of year 9999. Confirmed at the source before fixing, and reproduced against real `colitur emit --format ics --from 9999 --to 9999` output (DTEND;VALUE=DATE:100000101) before touching any code. RFC 5545 section 3.6.1: a VEVENT with a DATE-valued DTSTART and neither DTEND nor DURATION has an implicit one-day duration, so omitting DTEND for that one event is the standard's own correct answer, not a workaround. dtend_of re-derives the successor's year/month/day and re-validates them through Date.make -- the one function that actually enforces the domain -- before trusting the string; None means the caller omits the DTEND line entirely. F2 (minor, same function): documented next_day's own Error branch as dead-but-silent on shipped data (event's iso <> "" guard is the only caller and always parses) -- behaviour unchanged, comment only. Two new tests: the domain's last VEVENT (DTSTART 99991231) has no DTEND line at all; every DTEND anywhere in a 9999 feed is exactly 8 digits (the general form of the bug, catches a regression anywhere else in the domain too). Existing 2027/2028 DTEND-arithmetic assertions untouched and still pass. Mutation-proved: both new tests fail against the pre-fix code (9-digit DTEND value caught verbatim), pass after. --- lib/render/emit_ics.ml | 44 ++++++++++++++++++++++++++++++++++++++++++-- test/test_ics.ml | 45 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 3 deletions(-) (limited to 'test/test_ics.ml') diff --git a/lib/render/emit_ics.ml b/lib/render/emit_ics.ml index e1a5ad0..41efb95 100644 --- a/lib/render/emit_ics.ml +++ b/lib/render/emit_ics.ml @@ -12,8 +12,43 @@ let compact iso = (* "2027-01-13" -> "20270113" *) let next_day iso = match Colitur_kernel.Date.of_iso8601 iso with | Ok d -> Colitur_kernel.Date.to_iso8601 (Colitur_kernel.Date.add_days d 1) + (* Dead on shipped data: [event]'s own [iso <> ""] guard is the only + caller, and every real [iso] it passes came from [Date.to_iso8601] in + the first place, so it always parses. Left total rather than raising + (the kernel/render determinism rule), but a caller relying on this + branch would silently get DTEND == DTSTART -- a zero-length event -- + with no diagnostic, if it were ever actually reached. Documented, not + fixed: nothing exercises it. *) | Error _ -> iso +(* RFC 5545 section 3.6.1: a VEVENT with a DATE-valued DTSTART and NEITHER + DTEND NOR DURATION has an implicit duration of exactly one day, so + omitting DTEND for the domain's own last day is the standard's own + correct way to say precisely what we mean -- not a workaround. + + Needed because [Date.add_days] is itself UNBOUNDED (date.mli: "may + denote a year outside 1583..9999 -- only [make] enforces the domain"): + the successor of 9999-12-31 is a real [Date.t] that [next_day] above + happily renders as "10000-01-01" (date.ml's [to_iso8601] pads with + [Printf.sprintf "%04d-..."] but never truncates), which [compact] would + turn into a 9-digit, non-conformant DATE. [Date.make] is the ONE kernel + function that actually enforces 1583..9999 (date.mli), so this takes + [next_day]'s own successor string, re-derives its year/month/day, and + re-validates THOSE through [make] before trusting the string at all. + [None] means "one day past the domain's own last day"; the caller omits + DTEND entirely rather than clamp, truncate, or fall back to DURATION. *) +let dtend_of iso = + let next = next_day iso in + match String.split_on_char '-' next with + | [ y; m; d ] -> ( + match (int_of_string_opt y, int_of_string_opt m, int_of_string_opt d) with + | Some year, Some month, Some day -> ( + match Colitur_kernel.Date.make ~year ~month ~day with + | Ok _ -> Some next + | Error _ -> None) + | _ -> None) + | _ -> None + let esc x = Escape.apply Escape.Ics x let line b l = Buffer.add_string b (Escape.fold_ics l) @@ -41,8 +76,13 @@ let event b ~rite ~dtstamp d = line b (Printf.sprintf "UID:%s-%s@colitur" (compact iso) rite); line b ("DTSTAMP:" ^ dtstamp); line b ("DTSTART;VALUE=DATE:" ^ compact iso); - (* RFC 5545 section 3.6.1: DTEND is EXCLUSIVE for an all-day event. *) - line b ("DTEND;VALUE=DATE:" ^ compact (next_day iso)); + (* RFC 5545 section 3.6.1: DTEND is EXCLUSIVE for an all-day event -- and + omitted entirely, rather than emitted malformed, for the one event + whose successor falls outside the engine's own 1583..9999 domain + (see [dtend_of] above). *) + (match dtend_of iso with + | Some next -> line b ("DTEND;VALUE=DATE:" ^ compact next) + | None -> ()); line b ("SUMMARY:" ^ esc summary); if desc <> "" then line b ("DESCRIPTION:" ^ esc desc); line b "TRANSP:TRANSPARENT"; diff --git a/test/test_ics.ml b/test/test_ics.ml index ac0c9ee..7d1ca37 100644 --- a/test/test_ics.ml +++ b/test/test_ics.ml @@ -88,6 +88,47 @@ let test_dtstamp_is_a_parameter () = Alcotest.(check int) "default is deterministic" 365 (count_prefix "DTSTAMP:20270101T000000Z" (lines (out_2027 ()))) +(* F1 (fix round 1, 2026-08-19): 9999 is the domain's own last civil year. + Its last VEVENT's successor date (10000-01-01) falls outside the + 1583..9999 domain [Date.make] enforces (date.mli) -- [Date.add_days] is + itself unbounded, and [to_iso8601] pads but never truncates, so the naive + successor string would be 9 digits, not 8. RFC 5545 section 3.6.1 makes a + DATE-valued DTSTART with neither DTEND nor DURATION mean exactly one day, + so [dtend_of] omits the DTEND line entirely for that one event rather than + emit it malformed. *) +let out_9999 () = Ics.year (Test_view.view_of 9999) + +let test_dtend_omitted_at_domain_end () = + let ls = Array.of_list (lines (out_9999 ())) in + let n = Array.length ls in + let idx = ref (-1) in + Array.iteri (fun i l -> if l = "DTSTART;VALUE=DATE:99991231" then idx := i) ls; + if !idx < 0 then Alcotest.fail "DTSTART;VALUE=DATE:99991231 not found"; + let next_line = if !idx + 1 < n then ls.(!idx + 1) else "" in + Alcotest.(check bool) "no DTEND line follows DTSTART 99991231" true + (count_prefix "DTEND;VALUE=DATE:" [ next_line ] = 0); + Alcotest.(check bool) "SUMMARY follows DTSTART directly instead" true + (count_prefix "SUMMARY:" [ next_line ] = 1) + +(* The general form of F1's bug class: every DTEND value anywhere in the + feed must be an 8-digit YYYYMMDD, never 9 (a future domain-boundary + regression anywhere else in 1583..9999 would show up here too). *) +let test_all_9999_dtends_are_eight_digits () = + let ls = lines (out_9999 ()) in + let prefix = "DTEND;VALUE=DATE:" in + let ends = + List.filter_map + (fun l -> + if count_prefix prefix [ l ] = 1 then + Some (String.sub l (String.length prefix) (String.length l - String.length prefix)) + else None) + ls + in + Alcotest.(check bool) "at least one DTEND present in 9999" true (List.length ends > 0); + List.iter + (fun v -> if String.length v <> 8 then Alcotest.failf "DTEND value %S is %d digits, not 8" v (String.length v)) + ends + let suite = ( "Emit/ics", [ Alcotest.test_case "envelope" `Quick test_envelope; @@ -97,4 +138,6 @@ let suite = Alcotest.test_case "folded and CRLF" `Quick test_every_line_folded_and_crlf; Alcotest.test_case "no RRULE" `Quick test_no_rrule; Alcotest.test_case "text escaped" `Quick test_text_escaped; - Alcotest.test_case "DTSTAMP is a parameter" `Quick test_dtstamp_is_a_parameter ] ) + Alcotest.test_case "DTSTAMP is a parameter" `Quick test_dtstamp_is_a_parameter; + Alcotest.test_case "DTEND omitted at domain end" `Quick test_dtend_omitted_at_domain_end; + Alcotest.test_case "all 9999 DTENDs are eight digits" `Quick test_all_9999_dtends_are_eight_digits ] ) -- cgit v1.3