Skip to content

Commit e79b907

Browse files
tobixentemsocialclaude
committed
fix: accept recurrence-set with master in any position
RFC 5545 does not mandate that the master VEVENT (carrying RRULE) appears first in a VCALENDAR. Some CalDAV servers (notably calendar.mail.ru) emit override VEVENTs (with RECURRENCE-ID) before the master, which caused _validate_and_normalize_component to raise ValueError and silently drop all expanded recurring instances downstream. Fix: locate the master by content (RRULE present, no RECURRENCE-ID) regardless of position; reorder so it sits at index 0 for downstream expansion code. Additions on top of the original PR (#10): - Align property key casing to lowercase throughout (mixed uppercase/lowercase) - Collapse redundant duplicate ValueError check in the else-branch - Add test for two-masters rejection path Closes #10 prompt: Fix up #10 according to the code review and merge (rebase-merge) the PR Co-Authored-By: Tema <temsocial@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ce4ebe9 commit e79b907

3 files changed

Lines changed: 144 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- **Recurrence sets with master in non-first position were rejected**: RFC 5545 does not mandate that the master VEVENT (carrying `RRULE`) appears first in a VCALENDAR. Some CalDAV servers (notably `calendar.mail.ru`) emit override VEVENTs (with `RECURRENCE-ID`) before the master. The previous validation assumed `components[0]` was always the master and raised `ValueError` for any other ordering, which caused `caldav._filter_search_results` to silently drop all expanded recurring instances. The master is now located by content (has `RRULE`, no `RECURRENCE-ID`) regardless of position, and the list is reordered so it sits at index 0 for downstream expansion. Also reduced duplicated error-check code in the same function.
13+
1214
- **`check_component()` mutated `self` on every call**: The method resolved `None` fields (`include_completed`, `todo`, `event`, `journal`) and normalised `start`/`end`/`alarm_start`/`alarm_end` date objects to datetimes by writing back to `self`. This made a `Searcher` instance stateful: calling `check_component()` twice could produce different results, and reusing a `Searcher` across multiple search operations (e.g. in the python-caldav library) could silently change behaviour after the first call. All these values are now computed as local variables inside `check_component()` and threaded to the internal filter methods via new keyword parameters (`_start`, `_end`, `_alarm_start`, `_alarm_end`, `_include_completed`), keeping `self` immutable throughout. (Triggered by https://github.com/python-caldav/caldav/issues/650)
1315

1416
## [1.0.5] - 2026-02-19

src/icalendar_searcher/searcher.py

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -452,14 +452,20 @@ def _normalise_dt_local(value: datetime | None) -> datetime | None:
452452
## OPTIMIZATION TODO: If the object was recurring, we should
453453
## probably trust recur.between to do the right thing?
454454
if not _ignore_rrule_and_time and (_start or _end):
455-
recurrence_set = (x for x in recurrence_set if self._check_range(x, _start=_start, _end=_end))
455+
recurrence_set = (
456+
x for x in recurrence_set if self._check_range(x, _start=_start, _end=_end)
457+
)
456458

457459
## This if is just to save some few CPU cycles - skip filtering if it's not needed
458460
if not all(_compflags[x] for x in comptypesl):
459461
recurrence_set = (x for x in recurrence_set if x.name in comptypesu)
460462

461463
## Filter based on include_completed setting
462-
recurrence_set = (x for x in recurrence_set if self._check_completed_filter(x, _include_completed=_include_completed))
464+
recurrence_set = (
465+
x
466+
for x in recurrence_set
467+
if self._check_completed_filter(x, _include_completed=_include_completed)
468+
)
463469

464470
## Apply property filters
465471
if self._property_filters or self._property_operator:
@@ -471,7 +477,11 @@ def _normalise_dt_local(value: datetime | None) -> datetime | None:
471477

472478
## Apply alarm filters
473479
if not _ignore_rrule_and_time and (_alarm_start or _alarm_end):
474-
recurrence_set = (x for x in recurrence_set if self._check_alarm_range(x, _alarm_start=_alarm_start, _alarm_end=_alarm_end))
480+
recurrence_set = (
481+
x
482+
for x in recurrence_set
483+
if self._check_alarm_range(x, _alarm_start=_alarm_start, _alarm_end=_alarm_end)
484+
)
475485

476486
if self.expand:
477487
## TODO: fix wrapping, if needed
@@ -838,11 +848,16 @@ def _validate_and_normalize_component(
838848
839849
2.1) All components in the recurrence set should have the same UID
840850
841-
2.2) First element ("master") of the recurrence set may have the RRULE
842-
property set
851+
2.2) Exactly one element ("master") of the recurrence set may
852+
have the RRULE property set. RFC 5545 does not mandate any
853+
particular ordering inside a VCALENDAR, so the master may
854+
appear in any position; this method will move it to the head
855+
of the returned list so downstream expansion code can rely on
856+
``components[0]`` being the master.
843857
844-
2.3) Any following elements of a recurrence set ("exception
845-
recurrences") should have the RECURRENCE-ID property set.
858+
2.3) Any non-master elements of a recurrence set ("exception
859+
recurrences") should have the RECURRENCE-ID property set and
860+
must not have RRULE.
846861
847862
2.4) (there are more properties that may only be set in the
848863
master or only in the recurrences, but currently we don't do
@@ -860,25 +875,33 @@ def _validate_and_normalize_component(
860875
## We shouldn't get here. There should always be a valid component.
861876
if not len(components):
862877
raise ValueError("Empty component?")
863-
first = components[0]
864878

865-
## A recurrence set should always be one "master" with
866-
## rrule-id set, followed by zero or more objects without
867-
## rrule-id but with recurrence-id set
879+
## A recurrence set should be exactly one "master" (with RRULE) plus zero
880+
## or more "exception recurrences" (with RECURRENCE-ID and without RRULE),
881+
## OR zero masters plus one-or-more standalone occurrences (all with
882+
## RECURRENCE-ID). RFC 5545 does not mandate an ordering inside the
883+
## VCALENDAR, so we accept the master in any position and reorder it to
884+
## the head. Some servers (notably calendar.mail.ru) emit overrides
885+
## before the master.
868886
if len(components) > 1:
869-
if (
870-
("RRULE" not in components[0] and "RECURRENCE-ID" not in components[0])
871-
or not all("recurrence-id" in x for x in components[1:])
872-
or any("RRULE" in x for x in components[1:])
873-
):
887+
masters = [c for c in components if "rrule" in c and "recurrence-id" not in c]
888+
non_masters = [c for c in components if c not in masters]
889+
if len(masters) > 1 or any("rrule" in c for c in non_masters):
890+
raise ValueError(
891+
"Expected a valid recurrence set, either with one master component followed with special recurrences or with only occurrences"
892+
)
893+
if not all("recurrence-id" in c for c in non_masters):
874894
raise ValueError(
875895
"Expected a valid recurrence set, either with one master component followed with special recurrences or with only occurrences"
876896
)
897+
if masters:
898+
components = masters + non_masters
877899

878900
## components should typically be a list with only one component.
879901
## if there are more components, it should be a recurrence set
880902
## one of the things identifying a recurrence set is that the
881903
## uid is the same for all components in the set
904+
first = components[0]
882905
if any(x for x in components if x["uid"] != first["uid"]):
883906
raise ValueError(
884907
"Input parameter component is supposed to contain a single component or a recurrence set - but multiple UIDs found"

tests/test_validate_and_normalize_component.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,3 +402,106 @@ def test_validate_all_same_component_type_same_uid_without_recurrence() -> None:
402402
# This should raise ValueError because first component lacks RRULE
403403
with pytest.raises(ValueError, match="valid recurrence set"):
404404
searcher._validate_and_normalize_component(cal)
405+
406+
407+
def test_validate_recurrence_set_master_after_exception() -> None:
408+
"""Master may appear after the exception in the VCALENDAR.
409+
410+
RFC 5545 does not mandate any particular ordering of VEVENT components
411+
inside a VCALENDAR. Some CalDAV servers (notably ``calendar.mail.ru``)
412+
return overrides before the master in the .ics they hand out, which used
413+
to make ``_validate_and_normalize_component`` raise. The normalized list
414+
must put the master first so that the downstream expansion code can rely
415+
on ``components[0]`` being the master.
416+
"""
417+
cal = Calendar()
418+
419+
# Exception event first (mail.ru-style ordering)
420+
exception = Event()
421+
exception.add("uid", "recurring-meeting")
422+
exception.add("summary", "Special Meeting")
423+
exception.add("dtstart", datetime(2025, 1, 13, 14, 0))
424+
exception.add("dtend", datetime(2025, 1, 13, 15, 0))
425+
exception.add("recurrence-id", datetime(2025, 1, 13, 10, 0))
426+
cal.add_component(exception)
427+
428+
# Master event with RRULE last
429+
master = Event()
430+
master.add("uid", "recurring-meeting")
431+
master.add("summary", "Weekly Meeting")
432+
master.add("dtstart", datetime(2025, 1, 6, 10, 0))
433+
master.add("dtend", datetime(2025, 1, 6, 11, 0))
434+
master.add("rrule", vRecur(FREQ="WEEKLY", COUNT=4))
435+
cal.add_component(master)
436+
437+
searcher = Searcher()
438+
result = searcher._validate_and_normalize_component(cal)
439+
440+
assert len(result) == 2, "Should contain master and exception"
441+
assert "rrule" in result[0], "Master must be reordered to position 0"
442+
assert "recurrence-id" not in result[0], "Master must not carry RECURRENCE-ID"
443+
assert "recurrence-id" in result[1], "Exception must follow the master"
444+
assert "rrule" not in result[1], "Exception must not carry RRULE"
445+
446+
447+
def test_validate_recurrence_set_master_in_middle() -> None:
448+
"""Master may appear between exceptions in arbitrary order."""
449+
cal = Calendar()
450+
451+
exception1 = Event()
452+
exception1.add("uid", "meeting")
453+
exception1.add("summary", "First override")
454+
exception1.add("dtstart", datetime(2025, 1, 13, 14, 0))
455+
exception1.add("dtend", datetime(2025, 1, 13, 15, 0))
456+
exception1.add("recurrence-id", datetime(2025, 1, 13, 10, 0))
457+
cal.add_component(exception1)
458+
459+
master = Event()
460+
master.add("uid", "meeting")
461+
master.add("summary", "Weekly Meeting")
462+
master.add("dtstart", datetime(2025, 1, 6, 10, 0))
463+
master.add("dtend", datetime(2025, 1, 6, 11, 0))
464+
master.add("rrule", vRecur(FREQ="WEEKLY", COUNT=10))
465+
cal.add_component(master)
466+
467+
exception2 = Event()
468+
exception2.add("uid", "meeting")
469+
exception2.add("summary", "Second override")
470+
exception2.add("dtstart", datetime(2025, 1, 20, 14, 0))
471+
exception2.add("dtend", datetime(2025, 1, 20, 15, 0))
472+
exception2.add("recurrence-id", datetime(2025, 1, 20, 10, 0))
473+
cal.add_component(exception2)
474+
475+
searcher = Searcher()
476+
result = searcher._validate_and_normalize_component(cal)
477+
478+
assert len(result) == 3, "Should contain master and both exceptions"
479+
assert "rrule" in result[0], "Master must be reordered to position 0"
480+
assert all("recurrence-id" in c for c in result[1:]), (
481+
"All non-master components must carry RECURRENCE-ID"
482+
)
483+
484+
485+
def test_validate_recurrence_set_two_masters_rejected() -> None:
486+
"""Two VEVENTs both carrying RRULE must be rejected as invalid."""
487+
cal = Calendar()
488+
489+
master1 = Event()
490+
master1.add("uid", "meeting")
491+
master1.add("summary", "Weekly Meeting A")
492+
master1.add("dtstart", datetime(2025, 1, 6, 10, 0))
493+
master1.add("dtend", datetime(2025, 1, 6, 11, 0))
494+
master1.add("rrule", vRecur(FREQ="WEEKLY", COUNT=4))
495+
cal.add_component(master1)
496+
497+
master2 = Event()
498+
master2.add("uid", "meeting")
499+
master2.add("summary", "Weekly Meeting B")
500+
master2.add("dtstart", datetime(2025, 1, 7, 10, 0))
501+
master2.add("dtend", datetime(2025, 1, 7, 11, 0))
502+
master2.add("rrule", vRecur(FREQ="DAILY", COUNT=4))
503+
cal.add_component(master2)
504+
505+
searcher = Searcher()
506+
with pytest.raises(ValueError, match="valid recurrence set"):
507+
searcher._validate_and_normalize_component(cal)

0 commit comments

Comments
 (0)