From 8f8401ae5509c344482b72424e79e44e519221a5 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Fri, 23 Oct 2026 21:59:20 +0200 Subject: [PATCH 01/12] feat: probe DELETE If-Match, floating, attendees CheckDeleteIfMatch, CheckFloatingEvent and CheckAttendeeParams, for three SOGo behaviours. The floating probe reads the server's raw text: icalendar 7.3.0 writes a TZID=Etc/UTC time back with a 'Z', so a parsed object looks broken where SOGo's stored text is valid. Baikal, Radicale and Xandikos come out full on all three. Prompt: work on /tmp-handover-sogo-hints-2026-10-23.md (sic) [a handover note asking for probes and caldav hints for three SOGo behaviours found by calendaring's conformance suite: DELETE ignoring a stale If-Match, floating event times stored as UTC, ATTENDEE parameters rewritten] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- CHANGELOG.md | 10 ++ src/caldav_server_tester/checks.py | 260 +++++++++++++++++++++++++++-- tests/test_attendee_params.py | 120 +++++++++++++ tests/test_delete_if_match.py | 140 ++++++++++++++++ tests/test_floating_event.py | 114 +++++++++++++ 5 files changed, 628 insertions(+), 16 deletions(-) create mode 100644 tests/test_attendee_params.py create mode 100644 tests/test_delete_if_match.py create mode 100644 tests/test_floating_event.py diff --git a/CHANGELOG.md b/CHANGELOG.md index cb2e55d..61fcaca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ This project should adhere to [Semantic Versioning](https://semver.org/spec/v2.0 This library is tightly dependent on the CalDAV-library, particularly the `compatibility_hints.py`-file. This file is not (yet) considered to be part of the "core" business logic in the CalDAV library and can be changed in patch-releases in the CalDAV library - so this library is usually released in lock-steps with the CalDAV-library. I've considered to bump the version number to be follow the CalDAV version number. +## Unreleased + +Needs a caldav with the three features below in its `compatibility_hints.py`. + +### Added + +- **`delete.if-match`** - whether a DELETE with an If-Match etag the object no longer has is refused with 412, and one with the current etag goes through. SOGo refuses a stale If-Match on PUT, but deletes the object on DELETE. +- **`save-load.event.floating`** - whether an event with a floating DTSTART and DTEND is stored floating. SOGo gives the times TZID=Etc/UTC (tasks it keeps floating). The probe reads the server's own text: the icalendar library writes a TZID=Etc/UTC time back with a `Z` after it, which RFC 5545 §3.3.5 forbids. +- **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. + ## 1.7.0 - 2026-10-23 This release works with caldav 3.6.0. 1.6.0 was accidentally released without two probes and a grading fix that caldav 3.6.0's server hints were based on. diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index 7c9becd..e3098c2 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -4252,8 +4252,11 @@ class _RawPut: def _url(self, name): return str(self.checker.calendar.url.join(f"{name}.ics")) - def _ical(self, uid): + def _ical(self, uid, summary=None, floating=False, extra=""): + """An event at the fixture year's 20 January 12:00 UTC, or at 12:00 + wherever the user is with ``floating``; ``extra`` is more lines.""" year = self.checker.fixture_base_year + utc = "" if floating else "Z" return ( "BEGIN:VCALENDAR\r\n" "VERSION:2.0\r\n" @@ -4261,31 +4264,31 @@ def _ical(self, uid): "BEGIN:VEVENT\r\n" f"UID:{uid}\r\n" f"DTSTAMP:{year}0120T120000Z\r\n" - f"DTSTART:{year}0120T120000Z\r\n" - f"DTEND:{year}0120T130000Z\r\n" - f"SUMMARY:{self.LABEL} probe\r\n" + f"DTSTART:{year}0120T120000{utc}\r\n" + f"DTEND:{year}0120T130000{utc}\r\n" + f"SUMMARY:{summary or self.LABEL + ' probe'}\r\n" + f"{extra}" "END:VEVENT\r\n" "END:VCALENDAR\r\n" ) - def _get_etag(self, url): - """The ETag header a GET answers with, or None.""" + def _get(self, url): + """The answer to a GET, or None where it failed.""" try: response = self.client.request(url) except (DAVError, OSError): return None - if not self._ok(response): - return None - return response.headers.get("Etag") + return response if self._ok(response) else None - def _put(self, url, ical, if_match=None, if_none_match=None): - headers = {"Content-Type": "text/calendar; charset=utf-8"} - if if_match is not None: - headers["If-Match"] = if_match - if if_none_match is not None: - headers["If-None-Match"] = if_none_match + def _get_etag(self, url): + """The ETag header a GET answers with, or None.""" + response = self._get(url) + return response.headers.get("Etag") if response is not None else None + + @staticmethod + def _answer(request, *args): try: - return self.client.put(url, ical, headers) + return request(*args) except AuthorizationError as exc: ## 401/403 are raised before a response exists, but they are answers. return _StatusOnly(dav_error_status(exc) or _Unauthorized(), tuple(getattr(exc, "preconditions", ()))) @@ -4293,6 +4296,17 @@ def _put(self, url, ical, if_match=None, if_none_match=None): ## Rate limited for longer than the client waits, or no answer at all. return None + def _put(self, url, ical, if_match=None, if_none_match=None): + headers = {"Content-Type": "text/calendar; charset=utf-8"} + if if_match is not None: + headers["If-Match"] = if_match + if if_none_match is not None: + headers["If-None-Match"] = if_none_match + return self._answer(self.client.put, url, ical, headers) + + def _delete_if_match(self, url, etag): + return self._answer(self.client.delete, url, {"If-Match": etag}) + def _delete(self, url): try: self.client.delete(url) @@ -4565,6 +4579,220 @@ def _grade(self, response): return self._graded("ungraceful", f"a second object with the same UID was answered with {status}") +class CheckDeleteIfMatch(_RawPut, Check): + """ + Checks whether a DELETE carrying an If-Match etag the object no longer + has is refused, and one carrying the current etag goes through. + + RFC 9110 section 13.1.1 has the server refuse it with 412. SOGo refuses + a stale If-Match on PUT, but on DELETE it deletes the object anyway. + The etags are read with GET, as delete(if_match=True) sends the one the + object was loaded with. + """ + + depends_on = {PrepareCalendar} + LABEL = "delete-if-match" + FEATURE = "delete.if-match" + features_to_be_checked = {FEATURE} + + def _run_check(self) -> None: + url = self._url("csc_delete_if_match") + ical = self._ical("csc_delete_if_match") + try: + if not self._ok(self._put(url, ical)): + self.set_feature(self.FEATURE, None) + return + stale = self._get_etag(url) + changed = self._put(url, self._ical("csc_delete_if_match", summary=f"{self.LABEL} probe changed")) + current = self._get_etag(url) if self._ok(changed) else None + self.set_feature(self.FEATURE, self._grade(url, stale, current)) + finally: + self._delete(url) + + def _grade(self, url, stale, current): + for etag in (stale, current): + if not etag or not ENTITY_TAG.fullmatch(etag) or etag.startswith("W/"): + return self._graded("unknown", "cannot test, no strong etag for the object") + if stale == current: + return self._graded("unknown", "cannot test, the etag did not change when the object did") + status = self._status(self._delete_if_match(url, stale)) + if status is None: + return self._graded("unknown", "cannot test, a DELETE with a stale If-Match got no answer") + if 200 <= status < 300: + if self._get(url) is None: + return self._graded("broken", f"a DELETE with a stale If-Match deleted the object ({status})") + return self._graded("ungraceful", f"a DELETE with a stale If-Match answered {status}, but kept the object") + if status != 412: + return self._graded("ungraceful", f"a DELETE with a stale If-Match answered {status} rather than 412") + status = self._status(self._delete_if_match(url, current)) + if status is None: + return self._graded("unknown", "cannot test, a DELETE with the current If-Match got no answer") + if status == 412: + return self._graded("unsupported", "a DELETE with the current If-Match is refused with 412 too") + if not 200 <= status < 300: + return self._graded("ungraceful", f"a DELETE with the current If-Match answered {status}") + return "full" + + +def unfolded_lines(ical): + """The content lines of an iCalendar text, unfolded (RFC 5545 section 3.1).""" + return re.sub(r"\r?\n[ \t]", "", ical).splitlines() + + +def component_lines(ical, name): + """The content lines of the first ``name`` component, BEGIN and END left out.""" + lines = unfolded_lines(ical) + try: + start = lines.index(f"BEGIN:{name}") + return lines[start + 1 : lines.index(f"END:{name}", start)] + except ValueError: + return [] + + +class CheckFloatingEvent(_RawPut, Check): + """ + Checks whether an event with a floating DTSTART and DTEND (RFC 5545 + section 3.3.5: neither TZID nor 'Z') is stored floating. + + SOGo adds a VTIMEZONE for Etc/UTC and gives the times TZID=Etc/UTC. The + server's own text is read rather than a parsed object: the icalendar + library writes a TZID=Etc/UTC time back with a 'Z', which the section + forbids, so a re-serialised object would look broken where SOGo's is not. + """ + + depends_on = {PrepareCalendar} + LABEL = "floating" + FEATURE = "save-load.event.floating" + features_to_be_checked = {FEATURE} + PROPERTY = re.compile(r"(DTSTART|DTEND)((?:;[^:]*)?):(.*)") + + def _run_check(self) -> None: + url = self._url("csc_floating_event") + try: + stored = self._put(url, self._ical("csc_floating_event", floating=True)) + status = self._status(stored) + if status is None: + self.set_feature(self.FEATURE, self._graded("unknown", "cannot test, a floating event got no answer")) + elif not 200 <= status < 300: + self.set_feature( + self.FEATURE, self._graded("ungraceful", f"an event with a floating time is refused ({status})") + ) + else: + self.set_feature(self.FEATURE, self._grade(self._get(url))) + finally: + self._delete(url) + + def _grade(self, response): + if response is None: + return self._graded("unknown", "cannot test, the floating event could not be read back") + times = [ + match.groups() for match in map(self.PROPERTY.fullmatch, component_lines(response.raw, "VEVENT")) if match + ] + if not times: + return self._graded("unknown", "cannot test, the event came back without DTSTART") + for _, params, value in times: + tzid = re.search(r";TZID=([^;]*)", params, re.IGNORECASE) + if tzid and value.endswith("Z"): + return self._graded( + "broken", + f"a floating time is stored with TZID={tzid[1]} and a 'Z' both, which RFC 5545 section 3.3.5 forbids", + ) + if tzid: + return self._graded("unsupported", f"a floating time is stored with TZID={tzid[1]}") + if value.endswith("Z"): + return self._graded("unsupported", "a floating time is stored as UTC") + return "full" + + +class CheckAttendeeParams(_RawPut, Check): + """ + Checks whether the parameters of an ATTENDEE are stored as sent. + + A parameter added with RFC 5545's default value changes nothing, nor does + SCHEDULE-STATUS, which RFC 6638 has the server set. SOGo adds RSVP=TRUE, + where the default is FALSE. The event has no ORGANIZER, so it is not a + scheduling object resource and the server has no iTIP reason to touch it. + """ + + depends_on = {PrepareCalendar} + LABEL = "attendee-params" + FEATURE = "save-load.icalendar.attendee-params" + features_to_be_checked = {FEATURE} + ## RFC 5545 section 3.2 + DEFAULTS = {"PARTSTAT": "NEEDS-ACTION", "ROLE": "REQ-PARTICIPANT", "CUTYPE": "INDIVIDUAL", "RSVP": "FALSE"} + ## RFC 6638 section 7.3 + SERVER_SET = {"SCHEDULE-STATUS"} + ATTENDEES = { + "mailto:csc-bob@attendee.example.com": {"CN": "Bob"}, + "mailto:csc-ase@attendee.example.com": {"CN": "Åse", "PARTSTAT": "ACCEPTED"}, + } + + def _run_check(self) -> None: + url = self._url("csc_attendee_params") + lines = "".join( + "ATTENDEE" + "".join(f";{name}={value}" for name, value in params.items()) + f":{address}\r\n" + for address, params in self.ATTENDEES.items() + ) + try: + stored = self._put(url, self._ical("csc_attendee_params", extra=lines)) + status = self._status(stored) + if status is None: + self.set_feature( + self.FEATURE, self._graded("unknown", "cannot test, an event with attendees got no answer") + ) + elif not 200 <= status < 300: + self.set_feature( + self.FEATURE, self._graded("ungraceful", f"an event with attendees is refused ({status})") + ) + else: + self.set_feature(self.FEATURE, self._grade(self._get(url))) + finally: + self._delete(url) + + @staticmethod + def _parse(line): + """The address and parameters of an ATTENDEE line.""" + match = re.fullmatch(r"ATTENDEE((?:;(?:[^:;\"]|\"[^\"]*\")*)*):(.*)", line, re.IGNORECASE) + if not match: + return None, {} + params = {} + for param in re.findall(r';((?:[^;"]|"[^"]*")*)', match[1]): + name, _, value = param.partition("=") + params[name.upper()] = value.strip('"') + return match[2].lower(), params + + def _grade(self, response): + if response is None: + return self._graded("unknown", "cannot test, the event with attendees could not be read back") + stored = dict( + self._parse(line) for line in component_lines(response.raw, "VEVENT") if line.upper().startswith("ATTENDEE") + ) + changes, defaults = [], [] + for address, sent in self.ATTENDEES.items(): + if address not in stored: + changes.append(f"ATTENDEE {address} is dropped") + continue + got = stored[address] + for name, value in sent.items(): + if name not in got: + changes.append(f"{name}={value} is dropped") + elif got[name] != value and not (name != "CN" and got[name].upper() == value.upper()): + changes.append(f"{name}={value} is changed to {got[name]}") + for name, value in got.items(): + if name in sent: + continue + note = f"{name}={value} is added" + if name in self.SERVER_SET or self.DEFAULTS.get(name) == value.upper(): + defaults.append(f"{name}={value}" if name in self.SERVER_SET else note) + else: + changes.append(note) + ## Each attendee may get the same addition; say it once + changes, defaults = list(dict.fromkeys(changes)), list(dict.fromkeys(defaults)) + if changes: + return self._graded("unsupported", *changes, *defaults) + return self._graded("full", *defaults) + + class CheckAttendeePartstat(Check): """ Checks whether an attendee's PARTSTAT can be modified via a direct PUT. diff --git a/tests/test_attendee_params.py b/tests/test_attendee_params.py new file mode 100644 index 0000000..d6280c1 --- /dev/null +++ b/tests/test_attendee_params.py @@ -0,0 +1,120 @@ +"""Whether the parameters of an ATTENDEE are stored as sent. + +SOGo adds PARTSTAT=NEEDS-ACTION, which is RFC 5545's default and changes +nothing, and RSVP=TRUE, which is not: the default is FALSE. +""" + +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet + +from caldav_server_tester.checks import CheckAttendeeParams + +BASE = "http://example.com/cal/" +FEATURE = "save-load.icalendar.attendee-params" + + +class RewritingServer: + def __init__(self, rewrite=lambda line: line, put_status=201): + self.rewrite = rewrite + self.put_status = put_status + self.objects = {} + + def put(self, url, body, headers=None): + if self.put_status >= 300: + return Mock(status=self.put_status, headers={}) + lines = [self.rewrite(line) if line.startswith("ATTENDEE") else line for line in body.split("\r\n")] + self.objects[url] = "\r\n".join(line for line in lines if line is not None) + return Mock(status=201, headers={}) + + def request(self, url, method="GET"): + if url not in self.objects: + return Mock(status=404, headers={}, raw="") + return Mock(status=200, headers={}, raw=self.objects[url]) + + def delete(self, url, headers=None): + self.objects.pop(url, None) + return Mock(status=204) + + +def _sogo(line): + params, address = line.split(":", 1) + if "PARTSTAT" not in params: + params += ";PARTSTAT=NEEDS-ACTION" + return f"{params};RSVP=TRUE:{address}" + + +def _run(server): + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url.join.side_effect = lambda name: BASE + name + CheckAttendeeParams(checker)._run_check() + return checker._features_checked.is_supported(FEATURE, return_type=dict) + + +def test_stored_as_sent_is_full(): + server = RewritingServer() + assert _run(server) == {"support": "full"} + assert not server.objects + + +def test_the_probe_sends_no_organizer(): + """Attendees without an ORGANIZER: not a scheduling object resource.""" + sent = [] + server = RewritingServer() + original = server.put + server.put = lambda url, body, headers=None: sent.append(body) or original(url, body, headers) + _run(server) + assert "ATTENDEE" in sent[0] + assert "ORGANIZER" not in sent[0] + + +def test_sogo_is_unsupported(): + verdict = _run(RewritingServer(rewrite=_sogo)) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "RSVP=TRUE is added; PARTSTAT=NEEDS-ACTION is added" + + +@pytest.mark.parametrize( + "added", ["PARTSTAT=NEEDS-ACTION", "ROLE=REQ-PARTICIPANT", "CUTYPE=INDIVIDUAL", "RSVP=FALSE", "SCHEDULE-STATUS=1.2"] +) +def test_an_added_default_is_full(added): + def add(line): + params, address = line.split(":", 1) + return line if added.split("=")[0] in params else f"{params};{added}:{address}" + + verdict = _run(RewritingServer(rewrite=add)) + assert verdict["support"] == "full" + assert added in verdict["behaviour"] + + +def test_a_dropped_parameter_is_unsupported(): + verdict = _run(RewritingServer(rewrite=lambda line: line.replace(";PARTSTAT=ACCEPTED", ""))) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "PARTSTAT=ACCEPTED is dropped" + + +def test_a_changed_parameter_is_unsupported(): + verdict = _run(RewritingServer(rewrite=lambda line: line.replace("PARTSTAT=ACCEPTED", "PARTSTAT=NEEDS-ACTION"))) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "PARTSTAT=ACCEPTED is changed to NEEDS-ACTION" + + +def test_a_mangled_name_is_unsupported(): + verdict = _run(RewritingServer(rewrite=lambda line: line.replace("Åse", "?se"))) + assert verdict["support"] == "unsupported" + assert "CN=Åse is changed" in verdict["behaviour"] + + +def test_a_dropped_attendee_is_unsupported(): + verdict = _run(RewritingServer(rewrite=lambda line: None if "PARTSTAT" in line else line)) + assert verdict["support"] == "unsupported" + assert "is dropped" in verdict["behaviour"] + + +def test_refused_is_ungraceful(): + assert _run(RewritingServer(put_status=400))["support"] == "ungraceful" diff --git a/tests/test_delete_if_match.py b/tests/test_delete_if_match.py new file mode 100644 index 0000000..ee5929b --- /dev/null +++ b/tests/test_delete_if_match.py @@ -0,0 +1,140 @@ +"""Whether a DELETE honours If-Match. + +RFC 9110 section 13.1.1 has the server refuse a request whose If-Match etag +the object no longer has. SOGo does so on PUT (412), but a DELETE with a +stale etag goes through and the object is gone. +""" + +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import AuthorizationError + +from caldav_server_tester.checks import CheckDeleteIfMatch + +BASE = "http://example.com/cal/" +FEATURE = "delete.if-match" + + +class VersionedServer: + """A collection giving each stored version of an object a new etag. + + ``delete_if_match`` is how a conditional DELETE is evaluated: "rfc" + (holds for the current etag only), "ignored" (always holds) or "never". + ``stale_answer`` overrides the answer to a stale DELETE with a status, or + raises it where it is an exception. ``etags`` False leaves the ETag + header out; ``bump`` False keeps the etag across versions. + """ + + def __init__(self, delete_if_match="rfc", stale_answer=None, etags=True, bump=True, create_status=201): + self.delete_if_match = delete_if_match + self.stale_answer = stale_answer + self.etags = etags + self.bump = bump + self.create_status = create_status + self.versions = {} + self.deletes = [] + + def _etag(self, url): + return f'"v{self.versions[url]}"' + + def put(self, url, body, headers=None): + if self.create_status >= 300: + return self._response(self.create_status) + if url in self.versions: + self.versions[url] += 1 if self.bump else 0 + else: + self.versions[url] = 0 + return self._response(201, url) + + def request(self, url, method="GET"): + if url not in self.versions: + return self._response(404) + return self._response(200, url) + + def delete(self, url, headers=None): + etag = (headers or {}).get("If-Match") + self.deletes.append(etag) + if url not in self.versions: + return self._response(404) + if etag is not None: + current = etag == self._etag(url) + if not current and self.stale_answer is not None: + if isinstance(self.stale_answer, Exception): + raise self.stale_answer + return self._response(self.stale_answer) + holds = {"rfc": current, "ignored": True, "never": False}[self.delete_if_match] + if not holds: + return self._response(412) + del self.versions[url] + return self._response(204) + + def _response(self, status, url=None): + response = Mock() + response.status = status + response.headers = {"Etag": self._etag(url)} if url and self.etags else {} + return response + + +def _run(server): + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url.join.side_effect = lambda name: BASE + name + CheckDeleteIfMatch(checker)._run_check() + return checker._features_checked.is_supported(FEATURE, return_type=dict) + + +def test_stale_refused_and_current_taken_is_full(): + server = VersionedServer() + assert _run(server)["support"] == "full" + ## The stale etag first, then the current one + assert server.deletes[:2] == ['"v0"', '"v1"'] + assert not server.versions + + +def test_stale_if_match_deleting_is_broken(): + """SOGo""" + server = VersionedServer(delete_if_match="ignored") + verdict = _run(server) + assert verdict["support"] == "broken" + assert "stale If-Match deleted the object (204)" in verdict["behaviour"] + + +def test_current_if_match_refused_too_is_unsupported(): + server = VersionedServer(delete_if_match="never") + verdict = _run(server) + assert verdict["support"] == "unsupported" + + +@pytest.mark.parametrize("status", [400, 409, 500]) +def test_stale_if_match_refused_without_412_is_ungraceful(status): + verdict = _run(VersionedServer(stale_answer=status)) + assert verdict["support"] == "ungraceful" + assert str(status) in verdict["behaviour"] + + +def test_stale_if_match_answered_403_is_ungraceful(): + verdict = _run(VersionedServer(stale_answer=AuthorizationError("403 Forbidden"))) + assert verdict["support"] == "ungraceful" + + +def test_no_etag_is_unknown(): + assert _run(VersionedServer(etags=False))["support"] == "unknown" + + +def test_an_etag_unchanged_by_an_update_is_unknown(): + assert _run(VersionedServer(bump=False))["support"] == "unknown" + + +def test_an_object_that_cannot_be_stored_is_unknown(): + assert _run(VersionedServer(create_status=403))["support"] == "unknown" + + +def test_the_object_is_deleted_whatever_the_verdict(): + server = VersionedServer(delete_if_match="never") + _run(server) + assert not server.versions diff --git a/tests/test_floating_event.py b/tests/test_floating_event.py new file mode 100644 index 0000000..7f504d4 --- /dev/null +++ b/tests/test_floating_event.py @@ -0,0 +1,114 @@ +"""Whether an event with a floating time is stored floating. + +SOGo adds a VTIMEZONE for Etc/UTC and gives DTSTART and DTEND TZID=Etc/UTC. +The probe reads the server's own text: the icalendar library writes such a +time back with a 'Z' after the TZID, which RFC 5545 section 3.3.5 forbids. +""" + +import re +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import AuthorizationError + +from caldav_server_tester.checks import CheckFloatingEvent + +BASE = "http://example.com/cal/" +FEATURE = "save-load.event.floating" +VTIMEZONE = ( + "BEGIN:VTIMEZONE\r\nTZID:Etc/UTC\r\nBEGIN:STANDARD\r\nTZOFFSETFROM:+0000\r\n" + "TZOFFSETTO:+0000\r\nDTSTART:19700101T000000\r\nEND:STANDARD\r\nEND:VTIMEZONE\r\n" +) + + +class RewritingServer: + """Stores what ``rewrite`` makes of the body; ``put_answer`` overrides the + answer to the PUT (raised where it is an exception).""" + + def __init__(self, rewrite=lambda body: body, put_answer=None, get_status=200): + self.rewrite = rewrite + self.put_answer = put_answer + self.get_status = get_status + self.objects = {} + + def put(self, url, body, headers=None): + if isinstance(self.put_answer, Exception): + raise self.put_answer + if self.put_answer is not None: + return Mock(status=self.put_answer, headers={}) + self.objects[url] = self.rewrite(body) + return Mock(status=201, headers={}) + + def request(self, url, method="GET"): + if url not in self.objects: + return Mock(status=404, headers={}, raw="") + return Mock(status=self.get_status, headers={}, raw=self.objects[url]) + + def delete(self, url, headers=None): + self.objects.pop(url, None) + return Mock(status=204) + + +def _pin_to_utc(body, z=""): + body = re.sub(r"(DTSTART|DTEND):(\d{8}T\d{6})\r\n", rf"\1;TZID=Etc/UTC:\2{z}\r\n", body) + return body.replace("BEGIN:VEVENT", VTIMEZONE + "BEGIN:VEVENT") + + +def _run(server): + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url.join.side_effect = lambda name: BASE + name + CheckFloatingEvent(checker)._run_check() + return checker._features_checked.is_supported(FEATURE, return_type=dict) + + +def test_stored_floating_is_full(): + server = RewritingServer() + assert _run(server)["support"] == "full" + assert not server.objects + + +def test_the_probe_sends_floating_times(): + sent = [] + _run(RewritingServer(rewrite=lambda body: sent.append(body) or body)) + assert re.search(r"\r\nDTSTART:\d{8}T\d{6}\r\n", sent[0]) + assert re.search(r"\r\nDTEND:\d{8}T\d{6}\r\n", sent[0]) + + +def test_pinned_to_a_time_zone_is_unsupported(): + """SOGo; the VTIMEZONE's own DTSTART is not mistaken for the event's.""" + verdict = _run(RewritingServer(rewrite=_pin_to_utc)) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "a floating time is stored with TZID=Etc/UTC" + + +def test_tzid_and_z_both_is_broken(): + verdict = _run(RewritingServer(rewrite=lambda body: _pin_to_utc(body, "Z"))) + assert verdict["support"] == "broken" + assert "5545" in verdict["behaviour"] + + +def test_converted_to_utc_is_unsupported(): + verdict = _run(RewritingServer(rewrite=lambda body: re.sub(r"(DT(?:START|END):\d{8}T\d{6})\r\n", r"\1Z\r\n", body))) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "a floating time is stored as UTC" + + +def test_a_folded_line_is_read_whole(): + def fold(body): + return re.sub(r"DTSTART:(\d{8})", r"DTSTART;TZID=Etc/UTC:\r\n \1", body) + + assert _run(RewritingServer(rewrite=fold))["support"] == "unsupported" + + +@pytest.mark.parametrize("answer", [400, 403, AuthorizationError("403 Forbidden")]) +def test_refused_is_ungraceful(answer): + assert _run(RewritingServer(put_answer=answer))["support"] == "ungraceful" + + +def test_unreadable_is_unknown(): + assert _run(RewritingServer(get_status=500))["support"] == "unknown" From db6a774ecbe46913f57812101322a96c8500ca76 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sat, 24 Oct 2026 02:08:21 +0200 Subject: [PATCH 02/12] feat: probe sync, multiget and DELETE quirks CheckSyncMembers (what a sync report lists), CheckMultiget (a missing href, and limits on big multiget and PROPFIND answers, probed only with CALDAV_SERVER_TESTER_LIMIT_OBJECTS set), and two extensions: CheckDeleteIfMatch tries a DELETE of a missing object, CheckPutEtag compares the stored bytes with what was sent (broken where a rewritten object gets a strong ETag). The created-and-deleted member gets a sync interval of its own: Stalwart reports it only beside a change to a member it already had. The raw sync report helpers move to a _RawSync mixin shared with CheckSyncTokenLimit. Prompt: Have a look into https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst and see if there is anything we should add to the tests from that page Followup-Prompt: All of them should be probed. Please consider what feature tags we should put the results under and open up suggestions in my editor. Prompts (comments made inline in the document while reviewing it): > (on the DELETE with If-Match on a missing object) We don't have any other features under delete.* yet. There is a sync-token.delete and delete-calendarxo (sic) ... but I have no better ideas than the recommended. It's Robur that is the special case with non-existing-raises-not-found.object - does robur return 404 when trying to delete the object? If it returns 403, then use the alternative. > (on calendar-multiget) The check for stalwarts (sic) page problem should also be added > (on the initial sync) We have a sync-token.limit.initial-sync, should it be renamed sync-token.initial-sync.limit ? > (on the order of work) ok > check the external servers too, defined under ~/caldav-server-tester Followup-Prompt: (answer to a clarifying question) Keep the name [sync-token.limit.initial-sync] Followup-Prompt: (answer to a clarifying question) The checker code should be generic. If testing with 60 items and the limit was set down to 50 in the stalwart image, then we're confirming the problem with stalwart, but we're not checking anything on any other servers. I think we lower it in docker and have an environment variable to set the number of items probed. (wasn't there also a limit header or xml attribute that could be sent?) Followup-Prompt: if foo.bar is unsupported, but foo.bar.zoo is suppored (sic), then that may be a hint that the hierarchy is not well-enough thought through. foo.bar.zoo should typically be dependent on foo.bar Followup-Prompt: (answer to a clarifying question) save.etag.matches-stored Followup-Prompt: (summary: open the handover document about the sync probes in the editor; inline comments follow) Prompts (comments made inline in the document while reviewing it): > (on dropping the child feature) This makes sense - except, when too many different behaviours gets (sic) packed into one feature it often makes sense to split it up into children. The problem here is that we're potentially dealing with a child that is "Not Applicable" if the parent is unsupported, can be "Not Applicable" if the parent is supported and can never be unsupported, only broken. This simply does not fit into the feature hiearchy (sic) as it was designed. There is no grading "Not Applicable". Maybe the solution is to add such a grading. > matches-stored ... that may make sense. "supported" if stored is not rewritten. "supported" if stored is rewritten and the etag is weak or not given. "broken" if stored is rewritten and etag is strong. Am I right? > (on servers rewriting with a strong ETag) So they would get "broken" > (on servers rewriting without a strong ETag) And they would get "supported". > (on servers storing as sent) And they would also get "supported". > (on Stalwart reporting created-and-deleted correctly, unlike calendaring-sync's page) Do more research, maybe? Report back in the pull request, tag sashank Followup-Prompt: yes [go ahead with broken for a rewrite under a strong ETag, then the rest of the handover] Prompts (comments made inline in the document while reviewing it): > (on grading delete.missing.if-match against the plain DELETE's answer) Make sure both `delete.missing` and `delete.missing.if-match` is checked. Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- CHANGELOG.md | 9 +- src/caldav_server_tester/checks.py | 484 +++++++++++++++++++++++++---- tests/test_delete_if_match.py | 80 ++++- tests/test_multiget.py | 206 ++++++++++++ tests/test_put_etag.py | 98 ++++++ tests/test_sync_members.py | 232 ++++++++++++++ 6 files changed, 1051 insertions(+), 58 deletions(-) create mode 100644 tests/test_multiget.py create mode 100644 tests/test_sync_members.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 61fcaca..d1aa12e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,20 @@ This library is tightly dependent on the CalDAV-library, particularly the `compa ## Unreleased -Needs a caldav with the three features below in its `compatibility_hints.py`. +Needs a caldav with the features below in its `compatibility_hints.py`. ### Added - **`delete.if-match`** - whether a DELETE with an If-Match etag the object no longer has is refused with 412, and one with the current etag goes through. SOGo refuses a stale If-Match on PUT, but deletes the object on DELETE. - **`save-load.event.floating`** - whether an event with a floating DTSTART and DTEND is stored floating. SOGo gives the times TZID=Etc/UTC (tasks it keeps floating). The probe reads the server's own text: the icalendar library writes a TZID=Etc/UTC time back with a `Z` after it, which RFC 5545 §3.3.5 forbids. - **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. +- **`delete.missing`** and **`delete.missing.if-match`** - whether a DELETE of an object that does not exist answers 404, also with If-Match, as RFC 9110 §13.2.1 has it. Baïkal, Davis, Nextcloud, CCS, OX and Robur answer 412 with If-Match, so a delete of an object someone else already deleted reads as an etag mismatch. +- **`save.etag.matches-stored`** - whether a strong ETag in a PUT answer belongs to what was stored: RFC 4791 §5.3.4 lets a server rewrite the object, but not answer with a strong ETag then. The behaviour says what the server changed. +- **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice. +- **`multiget.missing-href`** - whether a calendar-multiget answers a missing href with a 404 entry; Baïkal leaves it out. +- **`multiget.limit`** and **`propfind.limit`** - whether a calendar-multiget or a Depth: 1 PROPFIND answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. + +The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst ## 1.7.0 - 2026-10-23 diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index e3098c2..d847bae 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -1,6 +1,7 @@ import contextlib import logging import math +import os import re import time import uuid @@ -4361,7 +4362,8 @@ class CheckPutEtag(_RawPut, Check): depends_on = {PrepareCalendar} WILDCARD = "save-load.mutable.if-match-wildcard" IF_NONE_MATCH = "save.if-none-match" - features_to_be_checked = {"save.etag", WILDCARD, IF_NONE_MATCH} + STORED = "save.etag.matches-stored" + features_to_be_checked = {"save.etag", STORED, WILDCARD, IF_NONE_MATCH} ## One missing object per condition: an If-Match: * that is ignored or ## backwards creates its object, which would then exist for the other. MISSING = ("csc_put_etag_missing_1", "csc_put_etag_missing_2") @@ -4375,7 +4377,14 @@ def _run_check(self) -> None: self.set_feature(feature, None) return try: + ## Read back before _grade_etag PUTs again: what an update adds + ## (LAST-MODIFIED, SEQUENCE) says nothing about the create's ETag + stored = self._get(url) self._grade_etag(url, ical, created.headers.get("Etag")) + ## Whether an unusable ETag matches the stored object is moot: + ## left to derive from save.etag + if self.checker._features_checked.is_supported("save.etag", str) != "broken": + self.set_feature(self.STORED, self._grade_stored(stored, ical, created.headers.get("Etag"))) ## A run that died half-way may have left a "missing" object behind. for name in self.MISSING: self._delete(self._url(name)) @@ -4385,6 +4394,53 @@ def _run_check(self) -> None: for probe_url in (url, *map(self._url, self.MISSING)): self._delete(probe_url) + def _grade_stored(self, response, ical, etag): + """RFC 4791 section 5.3.4 lets a server store an object otherwise than + sent, but then it MUST NOT answer the PUT with a strong ETag.""" + if response is None: + return self._graded("unknown", "cannot test, the object could not be read back") + ## The library turns CRLF into LF in .raw, and keeps the bytes as + ## received beside it; without them, line endings can't be compared + body = getattr(response, "_raw_body", None) + if isinstance(body, bytes) and body: + stored = body.decode("utf-8", "replace") + else: + stored = response.raw if isinstance(response.raw, str) else response.raw.decode("utf-8", "replace") + ical = ical.replace("\r\n", "\n") + stored = stored.replace("\r\n", "\n") + if stored == ical: + return "full" + what = self._difference(ical, stored) + if etag and not etag.startswith("W/"): + return self._graded("broken", f"stored with {what}, yet the PUT answered with a strong ETag") + return self._graded("full", f"stored with {what}, and the PUT answered without a strong ETag") + + @staticmethod + def _difference(sent, stored): + """What the server changed, in a few words.""" + + def names(lines): + return [re.split(r"[;:]", line, maxsplit=1)[0].upper() for line in lines] + + if sent.replace("\r\n", "\n") == stored.replace("\r\n", "\n"): + return "other line endings" + notes = ["other line endings"] if ("\r\n" in sent) != ("\r\n" in stored) else [] + sent_lines, stored_lines = unfolded_lines(sent), unfolded_lines(stored) + if sent_lines == stored_lines: + return ", ".join([*notes, "other folding"]) + added = [n for n in dict.fromkeys(names(stored_lines)) if n not in names(sent_lines)] + removed = [n for n in dict.fromkeys(names(sent_lines)) if n not in names(stored_lines)] + changed = [n for line in sent_lines if line not in stored_lines for n in names([line]) if n not in removed] + if added: + notes.append(f"{' and '.join(added)} added") + if removed: + notes.append(f"{' and '.join(removed)} removed") + if changed: + notes.append(f"{' and '.join(dict.fromkeys(changed))} changed") + if not (added or removed or changed): + notes.append("its properties reordered") + return ", ".join(notes) + def _grade_wildcard(self, url, ical): """``If-Match: *`` on the existing object grades; the missing object tells "never holds" from "holds backwards".""" @@ -4588,14 +4644,23 @@ class CheckDeleteIfMatch(_RawPut, Check): a stale If-Match on PUT, but on DELETE it deletes the object anyway. The etags are read with GET, as delete(if_match=True) sends the one the object was loaded with. + + On an object that does not exist, a DELETE answers 404, and RFC 9110 + section 13.2.1 has the server ignore an If-Match there, since the DELETE + fails anyway: 404, not 412. + Baikal and Robur answer 412, so delete(if_match=True) on an object + someone else deleted reads as an etag mismatch. """ depends_on = {PrepareCalendar} LABEL = "delete-if-match" FEATURE = "delete.if-match" - features_to_be_checked = {FEATURE} + PLAIN = "delete.missing" + MISSING = "delete.missing.if-match" + features_to_be_checked = {FEATURE, PLAIN, MISSING} def _run_check(self) -> None: + self._grade_missing() url = self._url("csc_delete_if_match") ical = self._ical("csc_delete_if_match") try: @@ -4633,6 +4698,37 @@ def _grade(self, url, stale, current): return self._graded("ungraceful", f"a DELETE with the current If-Match answered {status}") return "full" + def _grade_missing(self) -> None: + """A DELETE of a missing object, plain and with If-Match.""" + url = self._url("csc_delete_if_match_missing") + status = self._status(self._answer(self.client.delete, url)) + if status is None: + self.set_feature( + self.PLAIN, self._graded("unknown", "cannot test, a DELETE of a missing object got no answer") + ) + elif status == 404: + self.set_feature(self.PLAIN) + elif 200 <= status < 300: + self.set_feature(self.PLAIN, self._graded("unsupported", f"a DELETE of a missing object answered {status}")) + else: + self.set_feature(self.PLAIN, self._graded("ungraceful", f"a DELETE of a missing object answered {status}")) + ## RFC 9110 section 13.2.1 has the condition ignored where the request + ## would fail without it, so the plain DELETE's failure is the answer + plain, status = status, self._status(self._delete_if_match(url, '"csc-never-issued"')) + if status is None: + verdict = self._graded("unknown", "cannot test, a DELETE with If-Match on a missing object got no answer") + elif plain is not None and 200 <= plain < 300: + verdict = self._graded("unknown", f"cannot test, a plain DELETE of a missing object answered {plain}") + elif status == 404: + verdict = "full" + elif status == plain: + verdict = self._graded("full", f"answered {status}, as a plain DELETE of a missing object is") + elif status == 412: + verdict = self._graded("unsupported", "a DELETE with If-Match on a missing object answered 412") + else: + verdict = self._graded("ungraceful", f"a DELETE with If-Match on a missing object answered {status}") + self.set_feature(self.MISSING, verdict) + def unfolded_lines(ical): """The content lines of an iCalendar text, unfolded (RFC 5545 section 3.1).""" @@ -6787,12 +6883,102 @@ def _run_check(self) -> None: self._check_invalid_token(cal) +class _RawSync: + """Raw sync-collection REPORTs on the probe calendar, for probes that + need what the library's parser leaves out: statuses, the 507 marker, + and a member listed twice. Set ``cal_url``, ``collection_path`` and + ``time_based`` before use.""" + + ## _entries' stand-in for the collection listed as one of its own members + COLLECTION = "collection" + + def _setup_sync(self) -> bool: + """False where sync-collection is not supported well enough to probe.""" + sync = self.checker.features_checked.is_supported("sync-token", return_type=dict) + if sync.get("support") not in ("full", "fragile"): + return False + self.time_based = sync.get("behaviour") == "time-based" + self.cal_url = self.checker.calendar.url.canonical() + self.collection_path = unquote(self.cal_url.path).rstrip("/") + return True + + def _tick(self) -> None: + """Time-based tokens tell apart only changes a second apart.""" + if self.time_based: + time.sleep(1) + + def _name(self, url): + return unquote(url.path).rstrip("/").rsplit("/", 1)[-1] + + def _is_collection(self, url): + ## The request-URI may come back without its trailing slash, and on + ## OX by another path; members of a calendar are never collections + path = unquote(url.path) + return path.endswith("/") or path.rstrip("/") == self.collection_path + + def _report(self, token, limit): + """One raw sync-collection REPORT on the calendar. + + Returns the names of the members it names (changed or removed, and + COLLECTION for the collection itself), whether it carries the section + 3.6 507 marker for the collection, and its token. + """ + (entries, truncated, new_token) = self._entries(token, limit) + return ({name for (name, _) in entries}, truncated, new_token) + + def _entries(self, token, limit=None): + """Like _report, but every entry in the order given, with its status: + the number of a response-level DAV:status (404 for a removed member), + or None for an entry with properties (a changed member).""" + body = ( + '' + '' + f"{xml_escape(token)}" + "1" + + (f"{limit}" if limit else "") + + "" + ) + response = self.client.report(str(self.cal_url), body, depth=0) + if limit and response.status == 507 and "number-of-matches-within-limits" in str(response.raw): + raise LimitRefused() + if response.status not in (200, 207) or response.tree is None: + raise ReportError(f"sync-collection answered {response.status}") + (entries, truncated) = self._parse_entries(response) + new_token = response.tree.find(f".//{dav.SyncToken.tag}") + return (entries, truncated, new_token.text if new_token is not None else None) + + def _parse_entries(self, response, propstat_status=False): + """The members a multistatus names, each with its response-level + status (None where it has properties), and whether it carries a 507 + for the collection. With ``propstat_status``, an entry whose + propstats all fail takes the first propstat's status instead.""" + entries = [] + truncated = False + for elem in response.tree.iter(dav.Response.tag): + href = elem.find(dav.Href.tag) + if href is None or not href.text: + continue + url = self.cal_url.join(href.text) + is_collection = self._is_collection(url) + status = elem.find(dav.Status.tag) + if status is not None and " 507" in (status.text or ""): + truncated |= is_collection + continue + if status is None and propstat_status: + failed = elem.findall(f"{dav.PropStat.tag}/{dav.Status.tag}") + if failed and not any(re.search(r" 2\d\d\b", f.text or "") for f in failed): + status = failed[0] + code = re.search(r" (\d{3})", status.text or "") if status is not None else None + entries.append((self.COLLECTION if is_collection else self._name(url), int(code[1]) if code else None)) + return (entries, truncated) + + class LimitRefused(Exception): """A sync-collection with DAV:limit failed as RFC 6578 section 3.7 says a server that cannot truncate must: DAV:number-of-matches-within-limits.""" -class CheckSyncTokenLimit(Check): +class CheckSyncTokenLimit(_RawSync, Check): """ Checks DAV:limit in a sync-collection report (RFC 6578 section 3.7), and following the checkpoint token of a truncated reply (section 3.6). @@ -6819,8 +7005,6 @@ class CheckSyncTokenLimit(Check): NRESULTS = 2 MAX_PAGES = 20 UIDS = ("csc_sync_limit_1", "csc_sync_limit_2", "csc_sync_limit_3") - ## _report's stand-in for the collection listed as one of its own members - COLLECTION = "collection" COLLECTION_NOTE = "lists the collection itself as a member" CUT = "the reply is cut at DAV:limit without a 507" STUCK = "a 507 without a new token" @@ -6828,13 +7012,9 @@ class CheckSyncTokenLimit(Check): REFUSED = "refused with 507 number-of-matches-within-limits" def _run_check(self) -> None: - sync = self.checker.features_checked.is_supported("sync-token", return_type=dict) - if sync.get("support") not in ("full", "fragile"): + if not self._setup_sync(): return - self.time_based = sync.get("behaviour") == "time-based" cal = self.checker.calendar - self.cal_url = cal.url.canonical() - self.collection_path = unquote(self.cal_url.path).rstrip("/") created = [] try: try: @@ -6867,11 +7047,6 @@ def _run_check(self) -> None: except Exception: pass - def _tick(self) -> None: - """Time-based tokens tell apart only changes a second apart.""" - if self.time_based: - time.sleep(1) - def _grade(self, token, expected, what): limit = max(self.NRESULTS, math.ceil(len(expected) / (self.MAX_PAGES // 2))) try: @@ -6935,50 +7110,255 @@ def _follow(self, token, limit): token = new_token return (seen, truncated_any, notes) - def _name(self, url): - return unquote(url.path).rstrip("/").rsplit("/", 1)[-1] - def _is_collection(self, url): - ## The request-URI may come back without its trailing slash, and on - ## OX by another path; members of a calendar are never collections - path = unquote(url.path) - return path.endswith("/") or path.rstrip("/") == self.collection_path +class CheckSyncMembers(_RawPut, _RawSync, Check): + """ + Checks which members a sync-collection report lists. + + RFC 6578 section 3.4: an initial sync (empty token) lists the current + members and MUST NOT list removed ones; Radicale lists deleted members + with a 404, Stalwart also lists the collection itself. Section 3.5.2: a + member created and deleted between two reports MUST be reported as + removed; Xandikos leaves it out, so a client never learns what became of + its own create, and so does Stalwart unless a member it had at the token + changed in the same interval. Sections 3.5.1 and 3.2: a member deleted + and re-created at one href is reported once, as changed; Stalwart reports + it as removed and then changed. + https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst + """ - def _report(self, token, limit): - """One raw sync-collection REPORT on the calendar. + depends_on = {CheckSyncToken} + LABEL = "sync-members" + INITIAL = "sync-token.initial-sync" + TRANSIENT = "sync-token.delete.created-and-deleted" + RECREATED = "sync-token.recreated" + features_to_be_checked = {INITIAL, TRANSIENT, RECREATED} + KEEP = "csc_sync_members_keep" + GONE = "csc_sync_members_gone" + SHORT_LIVED = "csc_sync_members_transient" + AGAIN = "csc_sync_members_recreated" - Returns the names of the members it names (changed or removed, and - COLLECTION for the collection itself), whether it carries the section - 3.6 507 marker for the collection, and its token. - """ + def _run_check(self) -> None: + self._graded_here = set() + if not self._setup_sync(): + return + names = (self.KEEP, self.GONE, self.SHORT_LIVED, self.AGAIN) + for name in names: + self._delete(self._url(name)) + try: + self._probe() + except (DAVError, ReportError) as e: + ## A report failing later leaves the verdicts the earlier ones gave + for feature in self.features_to_be_checked - self._graded_here: + self.set_feature(feature, {"support": "ungraceful", "behaviour": f"sync-collection failed: {e}"}) + finally: + for name in names: + self._delete(self._url(name)) + + def set_feature(self, feature, value=True): + self._graded_here.add(feature) + super().set_feature(feature, value) + + def _store(self, name, summary=None) -> bool: + return self._ok(self._put(self._url(name), self._ical(name, summary=summary))) + + def _probe(self) -> None: + if not all(self._store(name) for name in (self.KEEP, self.AGAIN, self.GONE)): + for feature in self.features_to_be_checked: + self.set_feature(feature, self._graded("unknown", "cannot test, the probe objects could not be stored")) + return + self._tick() + self._delete(self._url(self.GONE)) + self._tick() + (entries, _, token) = self._entries("") + self.set_feature(self.INITIAL, self._grade_initial(entries)) + self._tick() + short_lived = self._store(self.SHORT_LIVED) + self._tick() + self._delete(self._url(self.SHORT_LIVED)) + self._tick() + ## An interval of its own: Stalwart reports the short-lived member + ## only where a member it already had changed in the same interval + (entries, _, token) = self._entries(token) + if short_lived: + self.set_feature(self.TRANSIENT, self._grade_transient(self._statuses(entries, self.SHORT_LIVED))) + else: + self.set_feature(self.TRANSIENT, self._graded("unknown", "cannot test, the object could not be stored")) + self._tick() + self._delete(self._url(self.AGAIN)) + self._tick() + ## Nextcloud's trashbin keeps a deleted object's UID and refuses it + recreated = self._store(self.AGAIN, summary=f"{self.LABEL} probe again") + self._tick() + (entries, _, _) = self._entries(token) + if recreated: + self.set_feature(self.RECREATED, self._grade_recreated(self._statuses(entries, self.AGAIN))) + else: + self.set_feature( + self.RECREATED, self._graded("unknown", "cannot test, a deleted object could not be re-created") + ) + + @staticmethod + def _statuses(entries, name): + """The statuses a report lists the member ``name`` with, in order.""" + return [status for href, status in entries if href == f"{name}.ics"] + + def _grade_initial(self, entries): + notes = [] + names = {name for name, _ in entries} + if any(status == 404 for name, status in entries if name != self.COLLECTION): + notes.append("lists deleted members with a 404") + elif f"{self.GONE}.ics" in names: + notes.append("lists a deleted member") + if self.COLLECTION in names: + notes.append("lists the collection itself") + missing = [name for name in (self.KEEP, self.AGAIN) if f"{name}.ics" not in names] + if missing: + return self._graded("broken", f"{len(missing)} current member(s) not listed", *notes) + return self._graded("quirk", *notes) if notes else "full" + + def _grade_transient(self, statuses): + if not statuses: + return self._graded("unsupported", "a member created and deleted between two reports is not reported") + if statuses[-1] != 404: + return self._graded("broken", "a member created and deleted between two reports is reported as changed") + return "full" + + def _grade_recreated(self, statuses): + if not statuses: + return self._graded("unsupported", "a member deleted and re-created is not reported") + changed = [status for status in statuses if status != 404] + if not changed: + return self._graded("broken", "a member deleted and re-created is reported as removed only") + if statuses == [404, *changed] and len(changed) == 1: + return self._graded("quirk", "reported twice: as removed, then as changed") + if len(statuses) > 1: + return self._graded("quirk", f"reported {len(statuses)} times") + return "full" + + +class CheckMultiget(_RawPut, _RawSync, Check): + """ + Checks calendar-multiget for a missing href, and limits on how many + members a multiget or a Depth: 1 PROPFIND answers. + + RFC 4791 section 7.9 gives every requested href an entry, a 404 for a + missing one; Baikal leaves it out. Stalwart answers at most its + WebDav.maxResults (2000 by default) members with a 507 for the collection, + even to a multiget that asked for fewer than it holds. Neither request has + a client-side limit to provoke that with, so the probe has to store more + objects than the server's limit: CALDAV_SERVER_TESTER_LIMIT_OBJECTS says + how many, and without it the limits are not probed. + """ + + depends_on = {PrepareCalendar} + LABEL = "multiget" + MISSING_HREF = "multiget.missing-href" + LIMIT = "multiget.limit" + PROPFIND_LIMIT = "propfind.limit" + features_to_be_checked = {MISSING_HREF, LIMIT, PROPFIND_LIMIT} + ENV = "CALDAV_SERVER_TESTER_LIMIT_OBJECTS" + PRESENT = "csc_multiget_present" + ABSENT = "csc_multiget_missing" + PROPFIND = ( + '' + ) + + def _run_check(self) -> None: + self.cal_url = self.checker.calendar.url.canonical() + self.collection_path = unquote(self.cal_url.path).rstrip("/") + present = self._url(self.PRESENT) + self._delete(self._url(self.ABSENT)) + try: + if self._ok(self._put(present, self._ical(self.PRESENT))): + self.set_feature(self.MISSING_HREF, self._grade_missing()) + else: + self.set_feature(self.MISSING_HREF, None) + finally: + self._delete(present) + count = self._object_count() + if not count: + for feature in (self.LIMIT, self.PROPFIND_LIMIT): + self.set_feature( + feature, {"support": "unknown", "behaviour": f"not probed: set {self.ENV} to a number of objects"} + ) + return + self._probe_limits(count) + + def _object_count(self) -> int: + try: + return max(0, int(os.environ.get(self.ENV, "0"))) + except ValueError: + return 0 + + def _multiget(self, names): + hrefs = "".join(f"{xml_escape(urlsplit(self._url(name)).path)}" for name in names) body = ( '' - '' - f"{xml_escape(token)}" - "1" - + (f"{limit}" if limit else "") - + "" + '' + f"{hrefs}" ) - response = self.client.report(str(self.cal_url), body, depth=0) - if limit and response.status == 507 and "number-of-matches-within-limits" in str(response.raw): - raise LimitRefused() + response = self.client.report(str(self.cal_url), body, depth=1) if response.status not in (200, 207) or response.tree is None: - raise ReportError(f"sync-collection answered {response.status}") - members = set() - truncated = False - for elem in response.tree.iter(dav.Response.tag): - href = elem.find(dav.Href.tag) - if href is None or not href.text: - continue - url = self.cal_url.join(href.text) - is_collection = self._is_collection(url) - status = elem.find(dav.Status.tag) - if status is not None and " 507" in (status.text or ""): - truncated |= is_collection - continue - members.add(self.COLLECTION if is_collection else self._name(url)) - new_token = response.tree.find(f".//{dav.SyncToken.tag}") - return (members, truncated, new_token.text if new_token is not None else None) + raise ReportError(f"calendar-multiget answered {response.status}") + return self._parse_entries(response, propstat_status=True) + + def _grade_missing(self): + try: + (entries, _) = self._multiget((self.PRESENT, self.ABSENT)) + except (DAVError, ReportError) as e: + return self._graded("ungraceful", f"a multiget naming a missing object failed: {e}") + statuses = [status for name, status in entries if name == f"{self.ABSENT}.ics"] + if not statuses: + return self._graded("unsupported", "a missing href is left out of the multistatus") + if statuses[0] is None or 200 <= statuses[0] < 300: + answer = "properties" if statuses[0] is None else statuses[0] + return self._graded("broken", f"a missing href is answered with {answer}, as if it existed") + if statuses[0] != 404: + return self._graded("full", f"a missing href is answered with {statuses[0]}") + return "full" + + def _probe_limits(self, count) -> None: + names = [f"csc_limit_{i:05d}" for i in range(count)] + try: + stored = sum(self._ok(self._put(self._url(name), self._ical(name))) for name in names) + if stored < count: + for feature in (self.LIMIT, self.PROPFIND_LIMIT): + self.set_feature( + feature, self._graded("unknown", f"cannot test, only {stored} of {count} objects were stored") + ) + return + mine = {f"{name}.ics" for name in names} + requests = ((self.LIMIT, lambda: self._multiget(names)), (self.PROPFIND_LIMIT, self._propfind)) + for feature, request in requests: + try: + (entries, truncated) = request() + except (DAVError, ReportError) as e: + ## Stalwart answers a PROPFIND past its limit with a bare 507 + if re.search(r"HTTP/1\.\d 507\b", str(e)) or dav_error_status(e) == 507: + verdict = self._graded("unsupported", f"answered 507 Insufficient Storage with {count} objects") + else: + verdict = self._graded("ungraceful", f"failed with {count} objects: {e}") + self.set_feature(feature, verdict) + continue + seen = len({name for name, status in entries if name in mine and status is None}) + self.set_feature(feature, self._grade_limit(seen, count, truncated)) + finally: + for name in names: + self._delete(self._url(name)) + + def _propfind(self): + response = self.client.propfind(str(self.cal_url), self.PROPFIND, depth=1) + if response.status not in (200, 207) or response.tree is None: + raise ReportError(f"PROPFIND answered {response.status}") + return self._parse_entries(response) + + def _grade_limit(self, seen, count, truncated): + if seen >= count: + return self._graded("full", f"no limit up to {count} objects") + if truncated: + return self._graded("unsupported", f"stops at {seen} of {count} objects, with a 507") + return self._graded("broken", f"lists {seen} of {count} objects, without a 507") class CheckFreeBusyQuery(Check): diff --git a/tests/test_delete_if_match.py b/tests/test_delete_if_match.py index ee5929b..30bc83d 100644 --- a/tests/test_delete_if_match.py +++ b/tests/test_delete_if_match.py @@ -27,7 +27,18 @@ class VersionedServer: header out; ``bump`` False keeps the etag across versions. """ - def __init__(self, delete_if_match="rfc", stale_answer=None, etags=True, bump=True, create_status=201): + def __init__( + self, + delete_if_match="rfc", + stale_answer=None, + etags=True, + bump=True, + create_status=201, + missing_answer=404, + missing_plain_answer=404, + ): + self.missing_answer = missing_answer + self.missing_plain_answer = missing_plain_answer self.delete_if_match = delete_if_match self.stale_answer = stale_answer self.etags = etags @@ -57,7 +68,9 @@ def delete(self, url, headers=None): etag = (headers or {}).get("If-Match") self.deletes.append(etag) if url not in self.versions: - return self._response(404) + if etag is not None and isinstance(self.missing_answer, Exception): + raise self.missing_answer + return self._response(self.missing_answer if etag is not None else self.missing_plain_answer) if etag is not None: current = etag == self._etag(url) if not current and self.stale_answer is not None: @@ -77,7 +90,7 @@ def _response(self, status, url=None): return response -def _run(server): +def _run(server, feature=FEATURE): checker = Mock() checker._features_checked = FeatureSet() checker.debug_mode = None @@ -85,14 +98,15 @@ def _run(server): checker.fixture_base_year = 2027 checker.calendar.url.join.side_effect = lambda name: BASE + name CheckDeleteIfMatch(checker)._run_check() - return checker._features_checked.is_supported(FEATURE, return_type=dict) + return checker._features_checked.is_supported(feature, return_type=dict) def test_stale_refused_and_current_taken_is_full(): server = VersionedServer() assert _run(server)["support"] == "full" ## The stale etag first, then the current one - assert server.deletes[:2] == ['"v0"', '"v1"'] + stale = server.deletes.index('"v0"') + assert server.deletes[stale : stale + 2] == ['"v0"', '"v1"'] assert not server.versions @@ -138,3 +152,59 @@ def test_the_object_is_deleted_whatever_the_verdict(): server = VersionedServer(delete_if_match="never") _run(server) assert not server.versions + + +MISSING = "delete.missing.if-match" +PLAIN = "delete.missing" + + +def test_a_conditional_delete_of_a_missing_object_answered_404_is_full(): + assert _run(VersionedServer(), MISSING) == {"support": "full"} + + +def test_a_conditional_delete_of_a_missing_object_answered_412_is_unsupported(): + """Baikal, Robur""" + verdict = _run(VersionedServer(missing_answer=412), MISSING) + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "a DELETE with If-Match on a missing object answered 412" + + +def test_a_conditional_delete_of_a_missing_object_answered_otherwise_is_ungraceful(): + assert _run(VersionedServer(missing_answer=500), MISSING)["support"] == "ungraceful" + + +def test_the_missing_object_is_probed_whatever_the_stale_verdict(): + """It needs no etag, so a server without one still gets a verdict.""" + assert _run(VersionedServer(etags=False, missing_answer=412), MISSING)["support"] == "unsupported" + + +def test_a_conditional_delete_answered_as_the_plain_one_is_full(): + """RFC 9110 section 13.2.1: the precondition is ignored where the request + would fail without it, so a 403 for both is right""" + verdict = _run(VersionedServer(missing_plain_answer=403, missing_answer=403), MISSING) + assert verdict == {"support": "full", "behaviour": "answered 403, as a plain DELETE of a missing object is"} + + +def test_a_conditional_delete_answered_412_where_the_plain_one_is_refused_is_unsupported(): + verdict = _run(VersionedServer(missing_plain_answer=403, missing_answer=412), MISSING) + assert verdict["support"] == "unsupported" + + +def test_a_conditional_delete_is_not_graded_where_the_plain_one_succeeds(): + """With nothing to fail, the condition decides, and a 412 is right""" + verdict = _run(VersionedServer(missing_plain_answer=204, missing_answer=412), MISSING) + assert verdict["support"] == "unknown" + + +def test_a_plain_delete_of_a_missing_object_answered_404_is_full(): + assert _run(VersionedServer(missing_answer=412), PLAIN) == {"support": "full"} + + +@pytest.mark.parametrize("status", [204, 200]) +def test_a_plain_delete_of_a_missing_object_succeeding_is_unsupported(status): + verdict = _run(VersionedServer(missing_plain_answer=status), PLAIN) + assert verdict == {"support": "unsupported", "behaviour": f"a DELETE of a missing object answered {status}"} + + +def test_a_plain_delete_of_a_missing_object_answered_otherwise_is_ungraceful(): + assert _run(VersionedServer(missing_plain_answer=403), PLAIN)["support"] == "ungraceful" diff --git a/tests/test_multiget.py b/tests/test_multiget.py new file mode 100644 index 0000000..f67086c --- /dev/null +++ b/tests/test_multiget.py @@ -0,0 +1,206 @@ +"""calendar-multiget for a missing href, and limits on big listings. + +RFC 4791 section 7.9 gives every requested href an entry in the multistatus, +a 404 for a missing one; Baikal leaves it out. Stalwart answers a multiget +or a Depth: 1 PROPFIND with at most WebDav.maxResults members and a 507 for +the collection. The limit is only probed with +CALDAV_SERVER_TESTER_LIMIT_OBJECTS set, since it takes that many objects. +""" + +import re +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import ResponseError +from caldav.lib.url import URL +from lxml import etree + +from caldav_server_tester.checks import CheckMultiget + +BASE = "http://example.com/cal/" +MISSING = "multiget.missing-href" +LIMIT = "multiget.limit" +PROPFIND = "propfind.limit" +ENV = "CALDAV_SERVER_TESTER_LIMIT_OBJECTS" + + +class MultigetServer: + """``omit_missing`` leaves a missing href out (Baikal); ``cap`` answers at + most that many members, with a 507 for the collection unless + ``cap_marker`` is False. ``missing_as`` answers a missing href with that + entry instead of a response-level 404: "propstat 404", "propstat 200" or a + response-level status code.""" + + def __init__( + self, omit_missing=False, cap=None, cap_marker=True, report_status=207, propfind_raises=None, missing_as=None + ): + self.missing_as = missing_as + self.propfind_raises = propfind_raises + self.omit_missing = omit_missing + self.cap = cap + self.cap_marker = cap_marker + self.report_status = report_status + self.objects = set() + self.puts = 0 + + @staticmethod + def _name(url): + return url.rstrip("/").rsplit("/", 1)[-1] + + def put(self, url, body, headers=None): + self.puts += 1 + self.objects.add(self._name(url)) + return Mock(status=201, headers={}) + + def delete(self, url, headers=None): + self.objects.discard(self._name(url)) + return Mock(status=204) + + def report(self, url, body, depth=0): + if self.report_status != 207: + return Mock(status=self.report_status, tree=None, raw="") + names = [self._name(h) for h in re.findall(r"(.*?)", body)] + entries = [(n, None if n in self.objects else (self.missing_as or 404)) for n in names] + if self.omit_missing: + entries = [e for e in entries if e[1] is None] + return self._answer(entries) + + def propfind(self, url, props=None, depth=0): + if self.propfind_raises: + raise self.propfind_raises + return self._answer([("", None)] + [(n, None) for n in sorted(self.objects)]) + + def _answer(self, entries): + truncated = self.cap is not None and len(entries) > self.cap + if truncated: + entries = entries[: self.cap] + parts = [] + for name, status in entries: + href = f"/cal/{name}" + if isinstance(status, str): + code = status.split()[1] + parts.append( + f"{href}" + f"HTTP/1.1 {code} X" + ) + elif status: + parts.append(f"{href}HTTP/1.1 {status} Not Found") + else: + parts.append( + f'{href}"x"' + "HTTP/1.1 200 OK" + ) + if truncated and self.cap_marker: + parts.append( + "/cal/HTTP/1.1 507 Insufficient Storage" + ) + xml = '' + "".join(parts) + "" + return Mock(status=207, tree=etree.fromstring(xml.encode()), raw=xml) + + +def _run(server, monkeypatch, objects=None): + if objects is None: + monkeypatch.delenv(ENV, raising=False) + else: + monkeypatch.setenv(ENV, str(objects)) + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url = URL.objectify(BASE) + CheckMultiget(checker)._run_check() + fs = checker._features_checked + return {f: fs.is_supported(f, return_type=dict) for f in (MISSING, LIMIT, PROPFIND)} + + +def test_a_404_entry_for_the_missing_href_is_full(monkeypatch): + server = MultigetServer() + assert _run(server, monkeypatch)[MISSING]["support"] == "full" + assert not server.objects + + +def test_the_missing_href_left_out_is_unsupported(monkeypatch): + """Baikal""" + verdict = _run(MultigetServer(omit_missing=True), monkeypatch)[MISSING] + assert verdict["support"] == "unsupported" + assert verdict["behaviour"] == "a missing href is left out of the multistatus" + + +def test_a_404_inside_the_propstat_is_full(monkeypatch): + assert _run(MultigetServer(missing_as="propstat 404"), monkeypatch)[MISSING] == {"support": "full"} + + +@pytest.mark.parametrize("answer", ["propstat 200", 200]) +def test_a_missing_href_answered_as_if_it_existed_is_broken(monkeypatch, answer): + verdict = _run(MultigetServer(missing_as=answer), monkeypatch)[MISSING] + assert verdict["support"] == "broken" + assert "as if it existed" in verdict["behaviour"] + + +def test_a_missing_href_answered_otherwise_names_the_status(monkeypatch): + assert _run(MultigetServer(missing_as=410), monkeypatch)[MISSING] == { + "support": "full", + "behaviour": "a missing href is answered with 410", + } + + +def test_a_failing_multiget_is_ungraceful(monkeypatch): + assert _run(MultigetServer(report_status=400), monkeypatch)[MISSING]["support"] == "ungraceful" + + +def test_the_limits_are_not_probed_by_default(monkeypatch): + server = MultigetServer() + verdicts = _run(server, monkeypatch) + assert server.puts == 1 + for feature in (LIMIT, PROPFIND): + assert verdicts[feature]["support"] == "unknown" + assert ENV in verdicts[feature]["behaviour"] + + +def test_no_limit_up_to_the_objects_stored(monkeypatch): + server = MultigetServer() + verdicts = _run(server, monkeypatch, objects=12) + for feature in (LIMIT, PROPFIND): + assert verdicts[feature] == {"support": "full", "behaviour": "no limit up to 12 objects"} + assert not server.objects + + +def test_a_cap_with_a_507_is_unsupported(monkeypatch): + """Stalwart""" + verdicts = _run(MultigetServer(cap=5), monkeypatch, objects=12) + assert verdicts[LIMIT] == {"support": "unsupported", "behaviour": "stops at 5 of 12 objects, with a 507"} + assert verdicts[PROPFIND]["support"] == "unsupported" + + +def test_a_cap_without_a_507_is_broken(monkeypatch): + verdicts = _run(MultigetServer(cap=5, cap_marker=False), monkeypatch, objects=12) + assert verdicts[LIMIT]["support"] == "broken" + assert verdicts[LIMIT]["behaviour"] == "lists 5 of 12 objects, without a 507" + + +@pytest.mark.parametrize("value", ["0", "", "many"]) +def test_an_unusable_object_count_does_not_probe(monkeypatch, value): + monkeypatch.setenv(ENV, value) + server = MultigetServer() + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url = URL.objectify(BASE) + CheckMultiget(checker)._run_check() + assert server.puts == 1 + + +def test_a_propfind_answered_507_as_a_whole_is_unsupported(monkeypatch): + """Stalwart answers a PROPFIND past its maxResults with a bare 507.""" + server = MultigetServer(propfind_raises=ResponseError("HTTP/1.1 507 Insufficient Storage")) + verdict = _run(server, monkeypatch, objects=12)[PROPFIND] + assert verdict == {"support": "unsupported", "behaviour": "answered 507 Insufficient Storage with 12 objects"} + + +def test_another_failure_is_ungraceful(monkeypatch): + server = MultigetServer(propfind_raises=ResponseError("HTTP/1.1 500 Internal Server Error")) + assert _run(server, monkeypatch, objects=12)[PROPFIND]["support"] == "ungraceful" diff --git a/tests/test_put_etag.py b/tests/test_put_etag.py index 6074404..2817f68 100644 --- a/tests/test_put_etag.py +++ b/tests/test_put_etag.py @@ -57,7 +57,14 @@ def __init__( create_status=201, create_raises=None, answers=None, + rewrite=None, + rewrite_updates=None, + raw_body=True, ): + self.rewrite = rewrite + self.rewrite_updates = rewrite_updates + self.raw_body = raw_body + self.bodies = {} self.put_etag = put_etag self.get_etag = put_etag if get_etag is None else get_etag self.accepts = {put_etag} if accepts is None else set(accepts) @@ -95,6 +102,8 @@ def put(self, url, body, headers=None): if not holds: return self._response(412) self.objects.add(url) + rewrite = self.rewrite_updates if exists and self.rewrite_updates else self.rewrite + self.bodies[url] = rewrite(body) if rewrite else body return self._response(204 if exists else 201) def _response(self, status): @@ -107,6 +116,11 @@ def request(self, url, method="GET"): response = Mock() response.status = 200 if url in self.objects else 404 response.headers = {"Etag": self.get_etag} if response.status == 200 and self.get_etag else {} + ## DAVResponse keeps the bytes as received, and turns CRLF into LF in .raw + body = self.bodies.get(url, "") + if self.raw_body: + response._raw_body = body.encode() + response.raw = body.replace("\r\n", "\n") return response def delete(self, url): @@ -403,3 +417,87 @@ def test_object_that_cannot_be_created_grades_nothing(server): for feature in ("save.etag", "save-load.mutable.if-match-wildcard", "save.if-none-match"): assert fs.is_supported(feature, return_type=dict)["support"] == "unknown" assert server.requests == [None] + + +STORED = "save.etag.matches-stored" + + +def _add_dtstamp_last(body): + """Radicale and Baikal add or move DTSTAMP; here it goes to the end.""" + lines = body.split("\r\n") + dtstamp = next(line for line in lines if line.startswith("DTSTAMP")) + lines.remove(dtstamp) + lines.insert(lines.index("END:VEVENT"), dtstamp) + return "\r\n".join(lines) + + +def test_stored_as_sent_is_full(): + assert _run_all(EtagServer()).is_supported(STORED, return_type=dict) == {"support": "full"} + + +def test_stored_differently_with_a_strong_etag_is_broken(): + """Radicale, Xandikos""" + verdict = _run_all(EtagServer(rewrite=_add_dtstamp_last)).is_supported(STORED, return_type=dict) + assert verdict == { + "support": "broken", + "behaviour": "stored with its properties reordered, yet the PUT answered with a strong ETag", + } + + +def test_line_endings_rewritten_with_a_strong_etag_is_broken(): + verdict = _run_all(EtagServer(rewrite=lambda body: body.replace("\r\n", "\n"))).is_supported( + STORED, return_type=dict + ) + assert verdict["support"] == "broken" + assert verdict["behaviour"].startswith("stored with other line endings") + + +def test_stored_with_lines_added_names_them(): + verdict = _run_all( + EtagServer(rewrite=lambda body: body.replace("END:VEVENT", "CLASS:PUBLIC\r\nEND:VEVENT")) + ).is_supported(STORED, return_type=dict) + assert verdict["behaviour"].startswith("stored with CLASS added") + + +def test_stored_differently_without_an_etag_is_full(): + """Baikal leaves the ETag out when it rewrites.""" + verdict = _run_all(EtagServer(put_etag=None, get_etag='"g"', rewrite=_add_dtstamp_last)).is_supported( + STORED, return_type=dict + ) + assert verdict["support"] == "full" + + +def test_stored_differently_with_a_weak_etag_is_full(): + verdict = _run_all(EtagServer(put_etag='W/"w"', rewrite=_add_dtstamp_last)).is_supported(STORED, return_type=dict) + assert verdict["support"] == "full" + + +def test_line_endings_are_compared_on_the_bytes_received(): + """caldav's .raw has LF where the server sent CRLF; that is not a rewrite.""" + server = EtagServer(rewrite=lambda body: body.replace("\r\n", "\n")) + assert _run_all(server).is_supported(STORED, return_type=dict)["support"] == "broken" + + +def test_without_the_bytes_received_line_endings_are_not_compared(): + """An older caldav keeps only .raw, with CRLF turned into LF""" + server = EtagServer(raw_body=False, rewrite=lambda body: body.replace("\r\n", "\n")) + assert _run_all(server).is_supported(STORED, return_type=dict) == {"support": "full"} + + +def test_without_the_bytes_received_a_rewrite_is_still_seen(): + verdict = _run_all(EtagServer(raw_body=False, rewrite=_add_dtstamp_last)).is_supported(STORED, return_type=dict) + assert verdict["support"] == "broken" + + +def test_a_rewrite_on_update_only_is_not_blamed_on_the_create(): + """The probe PUTs the object again for the If-Match checks; what an update + adds (LAST-MODIFIED, SEQUENCE) says nothing about the create's ETag""" + server = EtagServer(rewrite_updates=_add_dtstamp_last) + assert _run_all(server).is_supported(STORED, return_type=dict) == {"support": "full"} + + +def test_an_unusable_etag_is_not_asked_whether_it_matches(): + """Robur: the ETag is malformed, so whether it matches the stored object is moot.""" + fs = _run_all(EtagServer(put_etag="a22ccfb2985aed13f50b4991a110d32253fd99aa")) + assert fs.is_supported(STORED, return_type=dict) == fs.is_supported("save.etag", return_type=dict) + assert fs.is_supported(STORED, return_type=dict)["support"] == "broken" diff --git a/tests/test_sync_members.py b/tests/test_sync_members.py new file mode 100644 index 0000000..d07079a --- /dev/null +++ b/tests/test_sync_members.py @@ -0,0 +1,232 @@ +"""What a sync-collection report lists, member by member. + +RFC 6578 section 3.4: an initial sync (empty token) lists the current members +and no removed ones. Radicale lists deleted members with a 404, Stalwart also +lists the collection itself. Section 3.5.2: a member created and deleted +between two reports is reported as removed; Xandikos leaves it out, and so +does Stalwart where nothing else it had changed. Sections 3.5.1 and 3.2: a member deleted and re-created at one href is +reported once, as changed; Stalwart reports it as removed and then changed. +""" + +import re +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import DAVError +from caldav.lib.url import URL +from lxml import etree + +from caldav_server_tester.checks import CheckSyncMembers + +BASE = "http://example.com/cal/" +INITIAL = "sync-token.initial-sync" +TRANSIENT = "sync-token.delete.created-and-deleted" +RECREATED = "sync-token.recreated" + + +class SyncServer: + """A calendar keeping a change log; a token is the log's length. + + ``removed_in_initial`` lists deleted members with a 404 in an initial + sync (Radicale), ``collection_in_initial`` lists the collection itself + (Stalwart), ``skip_transient`` leaves out a member created and deleted + since the token (Xandikos), ``skip_lone_transient`` does so only where no + member that existed at the token changed too (Stalwart), and + ``recreate_twice`` reports a re-created member as removed and then + changed (Stalwart). + """ + + def __init__( + self, + removed_in_initial=False, + collection_in_initial=False, + skip_transient=False, + skip_lone_transient=False, + recreate_twice=False, + recreate_as_removed=False, + report_raises=None, + reports_before_raising=0, + refuse_recreate=False, + ): + self.refuse_recreate = refuse_recreate + self.removed_in_initial = removed_in_initial + self.collection_in_initial = collection_in_initial + self.skip_transient = skip_transient + self.skip_lone_transient = skip_lone_transient + self.recreate_twice = recreate_twice + self.recreate_as_removed = recreate_as_removed + self.report_raises = report_raises + self.reports_before_raising = reports_before_raising + self.log = [] + self.objects = set() + self.deleted = set() + + @staticmethod + def _name(url): + return url.rstrip("/").rsplit("/", 1)[-1] + + def put(self, url, body, headers=None): + name = self._name(url) + if self.refuse_recreate and name in self.deleted: + return Mock(status=409, headers={}) + self.log.append((name, "put")) + self.objects.add(name) + self.deleted.discard(name) + return Mock(status=201, headers={}) + + def delete(self, url, headers=None): + name = self._name(url) + if name in self.objects: + self.log.append((name, "delete")) + self.objects.discard(name) + self.deleted.add(name) + return Mock(status=204) + return Mock(status=404) + + def _state_at(self, seq): + state = set() + for name, action in self.log[:seq]: + (state.add if action == "put" else state.discard)(name) + return state + + def report(self, url, body, depth=0): + if self.report_raises: + if not self.reports_before_raising: + raise self.report_raises + self.reports_before_raising -= 1 + token = re.search(r"(.*?)", body)[1] + entries = [] + if not token: + entries = [(name, None) for name in sorted(self.objects)] + if self.removed_in_initial: + entries += [(name, 404) for name in sorted(self.deleted)] + if self.collection_in_initial: + entries.append(("", None)) + else: + since = int(token) + before = self._state_at(since) + window = self.log[since:] + ## Stalwart reports a transient member only beside a change to one it had + skip_transient = self.skip_transient or ( + self.skip_lone_transient and not any(n in before for n, _ in window) + ) + for name in dict.fromkeys(n for n, _ in window): + deleted_in_window = any(n == name and a == "delete" for n, a in window) + if name in self.objects: + if deleted_in_window and name in before and self.recreate_as_removed: + entries.append((name, 404)) + elif deleted_in_window and name in before and self.recreate_twice: + entries += [(name, 404), (name, None)] + else: + entries.append((name, None)) + elif name in before or not skip_transient: + entries.append((name, 404)) + return self._multistatus(entries, str(len(self.log))) + + @staticmethod + def _multistatus(entries, token): + parts = [] + for name, status in entries: + href = f"/cal/{name}" + if status: + parts.append(f"{href}HTTP/1.1 {status} Not Found") + else: + parts.append( + f'{href}"x"' + "HTTP/1.1 200 OK" + ) + xml = ( + '' + + "".join(parts) + + f"{token}" + ) + return Mock(status=207, tree=etree.fromstring(xml.encode()), raw=xml) + + +def _run(server, sync=None): + checker = Mock() + checker._features_checked = FeatureSet({"sync-token": sync or {"support": "full"}}) + checker.features_checked = checker._features_checked + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url = URL.objectify(BASE) + CheckSyncMembers(checker)._run_check() + fs = checker._features_checked + return {f: fs.is_supported(f, return_type=dict) for f in (INITIAL, TRANSIENT, RECREATED)} + + +def test_an_rfc_server_is_full_on_all_three(): + server = SyncServer() + verdicts = _run(server) + assert {v["support"] for v in verdicts.values()} == {"full"} + assert not server.objects + + +def test_deleted_members_in_an_initial_sync(): + """Radicale""" + verdict = _run(SyncServer(removed_in_initial=True))[INITIAL] + assert verdict["support"] == "quirk" + assert verdict["behaviour"] == "lists deleted members with a 404" + + +def test_the_collection_in_an_initial_sync(): + """Stalwart""" + verdict = _run(SyncServer(collection_in_initial=True))[INITIAL] + assert verdict["support"] == "quirk" + assert verdict["behaviour"] == "lists the collection itself" + + +def test_a_created_and_deleted_member_left_out(): + """Xandikos, Stalwart""" + verdict = _run(SyncServer(skip_transient=True))[TRANSIENT] + assert verdict["support"] == "unsupported" + assert "not reported" in verdict["behaviour"] + + +def test_a_created_and_deleted_member_alone_is_not_reported(): + """Stalwart 0.16: reported only where a member it had changed in the same interval""" + verdict = _run(SyncServer(skip_lone_transient=True))[TRANSIENT] + assert verdict["support"] == "unsupported" + + +def test_a_recreated_member_reported_twice(): + """Stalwart""" + verdict = _run(SyncServer(recreate_twice=True))[RECREATED] + assert verdict["support"] == "quirk" + assert verdict["behaviour"] == "reported twice: as removed, then as changed" + + +def test_a_recreated_member_reported_as_removed_only(): + verdict = _run(SyncServer(recreate_as_removed=True))[RECREATED] + assert verdict["support"] == "broken" + + +def test_a_later_failing_report_keeps_the_verdicts_already_given(): + verdicts = _run(SyncServer(skip_transient=True, report_raises=DAVError("500"), reports_before_raising=2)) + assert verdicts[INITIAL] == {"support": "full"} + assert verdicts[TRANSIENT]["support"] == "unsupported" + assert verdicts[RECREATED]["support"] == "ungraceful" + + +def test_a_failing_report_is_ungraceful(): + verdicts = _run(SyncServer(report_raises=DAVError("500"))) + assert {v["support"] for v in verdicts.values()} == {"ungraceful"} + + +@pytest.mark.parametrize("support", ["unsupported", "broken"]) +def test_nothing_is_probed_without_sync_support(support): + server = SyncServer() + verdicts = _run(server, sync={"support": support}) + assert not server.log + ## Not probed, so derived from sync-token + assert {v["support"] for v in verdicts.values()} == {support} + + +def test_a_member_that_cannot_be_recreated_is_not_graded(): + """Nextcloud keeps a deleted object's UID in its trashbin and refuses it.""" + verdicts = _run(SyncServer(refuse_recreate=True)) + assert verdicts[RECREATED]["support"] == "unknown" + assert "re-created" in verdicts[RECREATED]["behaviour"] + assert verdicts[TRANSIENT]["support"] == "full" From 7726ab597ebc2820a832a6bf176aea768708e4a5 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sat, 24 Oct 2026 14:28:38 +0200 Subject: [PATCH 03/12] docs: CHANGELOG follows the probe review fixes The entries for delete.missing.if-match, the sync members and multiget.missing-href now say what the probes grade after the review: the If-Match answer is compared with the plain DELETE's, a re-created member reported only as removed is named, and a missing href answered as if it existed is broken. Purelymail and Infomaniak join the 412 list. Prompt: (...) Add that commit to feat/sync-multiget-probes. (...) [the CHANGELOG catch-up the end-of-session audit proposed] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1aa12e..6fa700f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,10 +17,10 @@ Needs a caldav with the features below in its `compatibility_hints.py`. - **`delete.if-match`** - whether a DELETE with an If-Match etag the object no longer has is refused with 412, and one with the current etag goes through. SOGo refuses a stale If-Match on PUT, but deletes the object on DELETE. - **`save-load.event.floating`** - whether an event with a floating DTSTART and DTEND is stored floating. SOGo gives the times TZID=Etc/UTC (tasks it keeps floating). The probe reads the server's own text: the icalendar library writes a TZID=Etc/UTC time back with a `Z` after it, which RFC 5545 §3.3.5 forbids. - **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. -- **`delete.missing`** and **`delete.missing.if-match`** - whether a DELETE of an object that does not exist answers 404, also with If-Match, as RFC 9110 §13.2.1 has it. Baïkal, Davis, Nextcloud, CCS, OX and Robur answer 412 with If-Match, so a delete of an object someone else already deleted reads as an etag mismatch. +- **`delete.missing`** and **`delete.missing.if-match`** - whether a DELETE of an object that does not exist answers 404, and one with If-Match answers as the plain one does, as RFC 9110 §13.2.1 has it. Baïkal, Davis, Nextcloud, CCS, OX, Robur, Purelymail and Infomaniak answer 412 with If-Match, so a delete of an object someone else already deleted reads as an etag mismatch. - **`save.etag.matches-stored`** - whether a strong ETag in a PUT answer belongs to what was stored: RFC 4791 §5.3.4 lets a server rewrite the object, but not answer with a strong ETag then. The behaviour says what the server changed. -- **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice. -- **`multiget.missing-href`** - whether a calendar-multiget answers a missing href with a 404 entry; Baïkal leaves it out. +- **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice, or only as removed (Nextcloud, Synology). +- **`multiget.missing-href`** - whether a calendar-multiget answers a missing href with a 404 entry; Baïkal, Nextcloud, DAViCal, Davis and Infomaniak leave it out, Posteo fails the whole multiget with a 404, and an entry claiming the object exists is broken. - **`multiget.limit`** and **`propfind.limit`** - whether a calendar-multiget or a Depth: 1 PROPFIND answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst From f8cd221b472f07208a5531dd2494afd24d78ca09 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sat, 24 Oct 2026 14:29:24 +0200 Subject: [PATCH 04/12] docs: TODO on probing expired sync tokens Whether to probe a sync token whose history the server discarded. Stalwart can be made to do it, but only through an admin purge the checker cannot make as a client, so it is left as an open question with the arguments against it. Prompt: (...) File the Stalwart problem in such a way that I can consider it later - we shouldn't have too many server-special quirks in the checker though. (...) [expired sync tokens via Stalwart's DataRetention.maxChangesHistory] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- docs/TODO.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/TODO.md b/docs/TODO.md index af0ea9e..8f359a2 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -497,3 +497,29 @@ a production Zimbra, not the podman one): xandikos. Reproduce: is it the number of UIDs, Zimbra's UID search (it ignores text-match, so each lookup fetches the whole list), or plann's handling of several UIDs? + +# Expired sync tokens: worth a probe at all? + +(Noted 2026-10-24, from calendaring-sync's evidence branch. Undecided.) + +RFC 6578 section 3.2 has a server refuse a token whose change history it +discarded, with `DAV:valid-sync-token`. `sync-token.invalid-token` only +covers tokens the server never issued; a token that was valid and expired is +not probed, and a client that keeps tokens for days is the one that meets it. + +Stalwart can be made to expire tokens: calendaring-sync shrank +`DataRetention.maxChangesHistory` to 1 and then ran an `AccountMaintenance` +purge (https://github.com/PyCalendar/calendaring-sync/tree/evidence, under +`sync-design/servers/stalwart_admin.py`). + +Against it: + +* The purge is an admin JMAP call. The checker is a client; it would need a + server-side hook, i.e. a Stalwart special case, and the checker shouldn't + grow many of those. +* A low `maxChangesHistory` in caldav's shared Stalwart container expires + tokens under caldav's own sync tests whenever a purge runs. + +If it is done, the generic shape is a hook in the test-server definition +("expire the change history now"), with the probe `unknown` wherever no hook +is defined; Stalwart would be the only server with one. From 21671b6842f71f5575ff56a9c8a5e7e3ec3c2a36 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sat, 24 Oct 2026 15:25:03 +0200 Subject: [PATCH 05/12] fix: read the PROPFIND limit from the raw reply caldav's propfind() parses the reply at once and raises on the 507 Stalwart adds for the collection, so the probe saw an exception where there was a 207 listing 50 members and graded it as a 507 for the whole PROPFIND, a description that then reached the caldav profile. The PROPFIND now goes out raw, as the multiget does, and the cap is reported as the number of members listed, since it counts the calendar's other members too. A 507 for the whole answer still reads as unsupported. Prompt: See [the caldav review findings document] - the other session claims that the actual probing of Stalwart is inconsistently described now. Is there anything in the context or in the files in this project that can shed a light on it? [it found the propfind.limit description and the Stalwart profile disagreeing] Followup-Prompt: yes [fix the probe here; pass the profile wording and a possible caldav issue to the other session] Assisted-By: Claude Opus 5.5 (1M context) Reviewed-by: Tobias Brox --- src/caldav_server_tester/checks.py | 19 +++++++----- tests/test_multiget.py | 48 +++++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index d847bae..9f278f1 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -7334,30 +7334,35 @@ def _probe_limits(self, count) -> None: try: (entries, truncated) = request() except (DAVError, ReportError) as e: - ## Stalwart answers a PROPFIND past its limit with a bare 507 - if re.search(r"HTTP/1\.\d 507\b", str(e)) or dav_error_status(e) == 507: - verdict = self._graded("unsupported", f"answered 507 Insufficient Storage with {count} objects") + ## A server refusing the whole listing; Stalwart lists up + ## to its limit and adds a 507 for the collection instead + if re.search(r"answered 507\b", str(e)) or dav_error_status(e) == 507: + verdict = self._graded("unsupported", f"the whole answer was a 507, with {count} objects") else: verdict = self._graded("ungraceful", f"failed with {count} objects: {e}") self.set_feature(feature, verdict) continue seen = len({name for name, status in entries if name in mine and status is None}) - self.set_feature(feature, self._grade_limit(seen, count, truncated)) + ## A cap counts the calendar's other members too + listed = len({name for name, status in entries if name != self.COLLECTION and status is None}) + self.set_feature(feature, self._grade_limit(seen, count, truncated, listed)) finally: for name in names: self._delete(self._url(name)) def _propfind(self): - response = self.client.propfind(str(self.cal_url), self.PROPFIND, depth=1) + ## Raw, as for the multiget: caldav's propfind() parses at once and + ## raises on the collection's 507, hiding the members listed before it + response = self.client.request(str(self.cal_url), "PROPFIND", self.PROPFIND, {"Depth": "1"}) if response.status not in (200, 207) or response.tree is None: raise ReportError(f"PROPFIND answered {response.status}") return self._parse_entries(response) - def _grade_limit(self, seen, count, truncated): + def _grade_limit(self, seen, count, truncated, listed): if seen >= count: return self._graded("full", f"no limit up to {count} objects") if truncated: - return self._graded("unsupported", f"stops at {seen} of {count} objects, with a 507") + return self._graded("unsupported", f"stops at {listed} members, with a 507") return self._graded("broken", f"lists {seen} of {count} objects, without a 507") diff --git a/tests/test_multiget.py b/tests/test_multiget.py index f67086c..37f5cd7 100644 --- a/tests/test_multiget.py +++ b/tests/test_multiget.py @@ -33,8 +33,16 @@ class MultigetServer: response-level status code.""" def __init__( - self, omit_missing=False, cap=None, cap_marker=True, report_status=207, propfind_raises=None, missing_as=None + self, + omit_missing=False, + cap=None, + cap_marker=True, + report_status=207, + propfind_raises=None, + missing_as=None, + propfind_status=None, ): + self.propfind_status = propfind_status self.missing_as = missing_as self.propfind_raises = propfind_raises self.omit_missing = omit_missing @@ -67,9 +75,21 @@ def report(self, url, body, depth=0): return self._answer(entries) def propfind(self, url, props=None, depth=0): + """caldav parses the answer at once, and raises on the collection's 507""" + answer = self.request(url, "PROPFIND") + if b"507" in etree.tostring(answer.tree): + raise ResponseError("HTTP/1.1 507 Insufficient Storage") + return answer + + def request(self, url, method="GET", body="", headers=None): + """The raw answer, left unparsed""" + assert method == "PROPFIND" if self.propfind_raises: raise self.propfind_raises - return self._answer([("", None)] + [(n, None) for n in sorted(self.objects)]) + if self.propfind_status: + return Mock(status=self.propfind_status, tree=None, raw="") + ## Last, so that a cap drops it, as on Stalwart: its members, then a 507 + return self._answer([(n, None) for n in sorted(self.objects)] + [("", None)]) def _answer(self, entries): truncated = self.cap is not None and len(entries) > self.cap @@ -170,8 +190,16 @@ def test_no_limit_up_to_the_objects_stored(monkeypatch): def test_a_cap_with_a_507_is_unsupported(monkeypatch): """Stalwart""" verdicts = _run(MultigetServer(cap=5), monkeypatch, objects=12) - assert verdicts[LIMIT] == {"support": "unsupported", "behaviour": "stops at 5 of 12 objects, with a 507"} - assert verdicts[PROPFIND]["support"] == "unsupported" + assert verdicts[LIMIT] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} + assert verdicts[PROPFIND] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} + + +def test_a_propfind_cap_counts_every_member_listed(monkeypatch): + """A server's cap counts the calendar's other members too""" + server = MultigetServer(cap=5) + server.objects.update({"aa_other_1.ics", "aa_other_2.ics"}) # listed first + verdicts = _run(server, monkeypatch, objects=12) + assert verdicts[PROPFIND] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} def test_a_cap_without_a_507_is_broken(monkeypatch): @@ -195,10 +223,14 @@ def test_an_unusable_object_count_does_not_probe(monkeypatch, value): def test_a_propfind_answered_507_as_a_whole_is_unsupported(monkeypatch): - """Stalwart answers a PROPFIND past its maxResults with a bare 507.""" - server = MultigetServer(propfind_raises=ResponseError("HTTP/1.1 507 Insufficient Storage")) - verdict = _run(server, monkeypatch, objects=12)[PROPFIND] - assert verdict == {"support": "unsupported", "behaviour": "answered 507 Insufficient Storage with 12 objects"} + """No member listed at all, the HTTP status itself a 507""" + verdict = _run(MultigetServer(propfind_status=507), monkeypatch, objects=12)[PROPFIND] + assert verdict == {"support": "unsupported", "behaviour": "the whole answer was a 507, with 12 objects"} + + +def test_a_multiget_answered_507_as_a_whole_is_unsupported(monkeypatch): + verdict = _run(MultigetServer(report_status=507), monkeypatch, objects=12)[LIMIT] + assert verdict == {"support": "unsupported", "behaviour": "the whole answer was a 507, with 12 objects"} def test_another_failure_is_ungraceful(monkeypatch): From 0c246fc9d51b07e992c376ecece4da1fad9803b1 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 26 Oct 2026 01:40:24 +0100 Subject: [PATCH 06/12] feat: probe PARTICIPANT and VTIMEZONE round trips save-load.icalendar.participant: an event carrying an RFC 9073 PARTICIPANT is stored (CCS and Bedework 3 answer 403, robur 400, GMX and Ox 500). save-load.icalendar.vtimezone: the VTIMEZONE sent with an event comes back (CCS drops it, keeping the TZID). The PUT-and-read- back skeleton the floating and attendee-params probes each had is now one base class, _RoundTrip, shared by all four. Fixes https://github.com/python-caldav/caldav-server-tester/issues/38 Fixes https://github.com/python-caldav/caldav-server-tester/issues/39 Prompt: (...) But those issues you just filed seems like things that are needed for the conformance tests, so please work on them. [https://github.com/python-caldav/caldav-server-tester/issues/38, /39, /40, /41 and https://github.com/python-caldav/caldav/issues/829] Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 2 + src/caldav_server_tester/checks.py | 192 +++++++++++++++++++++-------- tests/test_icalendar_roundtrip.py | 101 +++++++++++++++ 3 files changed, 242 insertions(+), 53 deletions(-) create mode 100644 tests/test_icalendar_roundtrip.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fa700f..1f8cd17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Needs a caldav with the features below in its `compatibility_hints.py`. - **`delete.if-match`** - whether a DELETE with an If-Match etag the object no longer has is refused with 412, and one with the current etag goes through. SOGo refuses a stale If-Match on PUT, but deletes the object on DELETE. - **`save-load.event.floating`** - whether an event with a floating DTSTART and DTEND is stored floating. SOGo gives the times TZID=Etc/UTC (tasks it keeps floating). The probe reads the server's own text: the icalendar library writes a TZID=Etc/UTC time back with a `Z` after it, which RFC 5545 §3.3.5 forbids. - **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. +- **`save-load.icalendar.participant`** - whether an RFC 9073 PARTICIPANT inside a VEVENT is stored. CCS and Bedework 3 refuse the event with 403, robur with 400, GMX and Ox with 500. See https://github.com/python-caldav/caldav-server-tester/issues/38. +- **`save-load.icalendar.vtimezone`** - whether the VTIMEZONE sent with an event comes back with it. CCS returns the event's TZID without the VTIMEZONE. See https://github.com/python-caldav/caldav-server-tester/issues/39. - **`delete.missing`** and **`delete.missing.if-match`** - whether a DELETE of an object that does not exist answers 404, and one with If-Match answers as the plain one does, as RFC 9110 §13.2.1 has it. Baïkal, Davis, Nextcloud, CCS, OX, Robur, Purelymail and Infomaniak answer 412 with If-Match, so a delete of an object someone else already deleted reads as an etag mismatch. - **`save.etag.matches-stored`** - whether a strong ETag in a PUT answer belongs to what was stored: RFC 4791 §5.3.4 lets a server rewrite the object, but not answer with a strong ETag then. The behaviour says what the server changed. - **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice, or only as removed (Nextcloud, Synology). diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index 9f278f1..4afe7d2 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -4253,20 +4253,23 @@ class _RawPut: def _url(self, name): return str(self.checker.calendar.url.join(f"{name}.ics")) - def _ical(self, uid, summary=None, floating=False, extra=""): + def _ical(self, uid, summary=None, floating=False, extra="", vtimezone=None): """An event at the fixture year's 20 January 12:00 UTC, or at 12:00 - wherever the user is with ``floating``; ``extra`` is more lines.""" + wherever the user is with ``floating``, or in the time zone of the + ``vtimezone`` text, which goes along; ``extra`` is more lines.""" year = self.checker.fixture_base_year - utc = "" if floating else "Z" + utc = "" if floating or vtimezone else "Z" + tzid = f";TZID={re.search(r'^TZID:(.*)$', vtimezone, re.MULTILINE)[1].strip()}" if vtimezone else "" return ( "BEGIN:VCALENDAR\r\n" "VERSION:2.0\r\n" f"PRODID:-//caldav-server-tester//{self.LABEL} probe//EN\r\n" + f"{vtimezone or ''}" "BEGIN:VEVENT\r\n" f"UID:{uid}\r\n" f"DTSTAMP:{year}0120T120000Z\r\n" - f"DTSTART:{year}0120T120000{utc}\r\n" - f"DTEND:{year}0120T130000{utc}\r\n" + f"DTSTART{tzid}:{year}0120T120000{utc}\r\n" + f"DTEND{tzid}:{year}0120T130000{utc}\r\n" f"SUMMARY:{summary or self.LABEL + ' probe'}\r\n" f"{extra}" "END:VEVENT\r\n" @@ -4745,7 +4748,39 @@ def component_lines(ical, name): return [] -class CheckFloatingEvent(_RawPut, Check): +class _RoundTrip(_RawPut): + """Stores the event ``_body()`` gives, reads it back, and has ``_grade()`` + judge the server's own text. Refused is "ungraceful".""" + + ## The probe object's UID and name, and what it is, for the behaviour notes + UID = "csc_round_trip" + WHAT = "the event" + ## More lines for the VEVENT + EXTRA = "" + + def _body(self): + return self._ical(self.UID, extra=self.EXTRA) + + def _run_check(self) -> None: + url = self._url(self.UID) + try: + status = self._status(self._put(url, self._body())) + if status is None: + verdict = self._graded("unknown", f"cannot test, {self.WHAT} got no answer") + elif not 200 <= status < 300: + verdict = self._graded("ungraceful", f"{self.WHAT} is refused ({status})") + else: + response = self._get(url) + if response is None: + verdict = self._graded("unknown", f"cannot test, {self.WHAT} could not be read back") + else: + verdict = self._grade(response.raw) + self.set_feature(self.FEATURE, verdict) + finally: + self._delete(url) + + +class CheckFloatingEvent(_RoundTrip, Check): """ Checks whether an event with a floating DTSTART and DTEND (RFC 5545 section 3.3.5: neither TZID nor 'Z') is stored floating. @@ -4760,30 +4795,15 @@ class CheckFloatingEvent(_RawPut, Check): LABEL = "floating" FEATURE = "save-load.event.floating" features_to_be_checked = {FEATURE} + UID = "csc_floating_event" + WHAT = "an event with a floating time" PROPERTY = re.compile(r"(DTSTART|DTEND)((?:;[^:]*)?):(.*)") - def _run_check(self) -> None: - url = self._url("csc_floating_event") - try: - stored = self._put(url, self._ical("csc_floating_event", floating=True)) - status = self._status(stored) - if status is None: - self.set_feature(self.FEATURE, self._graded("unknown", "cannot test, a floating event got no answer")) - elif not 200 <= status < 300: - self.set_feature( - self.FEATURE, self._graded("ungraceful", f"an event with a floating time is refused ({status})") - ) - else: - self.set_feature(self.FEATURE, self._grade(self._get(url))) - finally: - self._delete(url) + def _body(self): + return self._ical(self.UID, floating=True) - def _grade(self, response): - if response is None: - return self._graded("unknown", "cannot test, the floating event could not be read back") - times = [ - match.groups() for match in map(self.PROPERTY.fullmatch, component_lines(response.raw, "VEVENT")) if match - ] + def _grade(self, raw): + times = [match.groups() for match in map(self.PROPERTY.fullmatch, component_lines(raw, "VEVENT")) if match] if not times: return self._graded("unknown", "cannot test, the event came back without DTSTART") for _, params, value in times: @@ -4800,7 +4820,7 @@ def _grade(self, response): return "full" -class CheckAttendeeParams(_RawPut, Check): +class CheckAttendeeParams(_RoundTrip, Check): """ Checks whether the parameters of an ATTENDEE are stored as sent. @@ -4814,6 +4834,8 @@ class CheckAttendeeParams(_RawPut, Check): LABEL = "attendee-params" FEATURE = "save-load.icalendar.attendee-params" features_to_be_checked = {FEATURE} + UID = "csc_attendee_params" + WHAT = "an event with attendees" ## RFC 5545 section 3.2 DEFAULTS = {"PARTSTAT": "NEEDS-ACTION", "ROLE": "REQ-PARTICIPANT", "CUTYPE": "INDIVIDUAL", "RSVP": "FALSE"} ## RFC 6638 section 7.3 @@ -4823,27 +4845,10 @@ class CheckAttendeeParams(_RawPut, Check): "mailto:csc-ase@attendee.example.com": {"CN": "Åse", "PARTSTAT": "ACCEPTED"}, } - def _run_check(self) -> None: - url = self._url("csc_attendee_params") - lines = "".join( - "ATTENDEE" + "".join(f";{name}={value}" for name, value in params.items()) + f":{address}\r\n" - for address, params in self.ATTENDEES.items() - ) - try: - stored = self._put(url, self._ical("csc_attendee_params", extra=lines)) - status = self._status(stored) - if status is None: - self.set_feature( - self.FEATURE, self._graded("unknown", "cannot test, an event with attendees got no answer") - ) - elif not 200 <= status < 300: - self.set_feature( - self.FEATURE, self._graded("ungraceful", f"an event with attendees is refused ({status})") - ) - else: - self.set_feature(self.FEATURE, self._grade(self._get(url))) - finally: - self._delete(url) + EXTRA = "".join( + "ATTENDEE" + "".join(f";{name}={value}" for name, value in params.items()) + f":{address}\r\n" + for address, params in ATTENDEES.items() + ) @staticmethod def _parse(line): @@ -4857,11 +4862,9 @@ def _parse(line): params[name.upper()] = value.strip('"') return match[2].lower(), params - def _grade(self, response): - if response is None: - return self._graded("unknown", "cannot test, the event with attendees could not be read back") + def _grade(self, raw): stored = dict( - self._parse(line) for line in component_lines(response.raw, "VEVENT") if line.upper().startswith("ATTENDEE") + self._parse(line) for line in component_lines(raw, "VEVENT") if line.upper().startswith("ATTENDEE") ) changes, defaults = [], [] for address, sent in self.ATTENDEES.items(): @@ -4889,6 +4892,89 @@ def _grade(self, response): return self._graded("full", *defaults) +class CheckParticipant(_RoundTrip, Check): + """ + Checks whether a PARTICIPANT component (RFC 9073 section 7.1) inside a + VEVENT is stored. CCS and Bedework 3 refuse the event with 403, robur + with 400, GMX and Ox with 500 (and Zimbra, reportedly). + """ + + depends_on = {PrepareCalendar} + LABEL = "participant" + FEATURE = "save-load.icalendar.participant" + features_to_be_checked = {FEATURE} + UID = "csc_participant_event" + WHAT = "an event with a PARTICIPANT" + ## UID and PARTICIPANT-TYPE are the properties section 7.1 requires + EXTRA = ( + "BEGIN:PARTICIPANT\r\n" + "UID:csc_participant_1\r\n" + "PARTICIPANT-TYPE:ACTIVE\r\n" + "CALENDAR-ADDRESS:mailto:csc-participant@attendee.example.com\r\n" + "END:PARTICIPANT\r\n" + ) + + def _grade(self, raw): + if "BEGIN:PARTICIPANT" in (line.upper() for line in component_lines(raw, "VEVENT")): + return "full" + return self._graded("unsupported", "the event is stored without its PARTICIPANT") + + +class CheckVtimezone(_RoundTrip, Check): + """ + Checks whether the VTIMEZONE sent with an event comes back with it. CCS + returns the event's TZID without the VTIMEZONE that defines it. + + Europe/Berlin rather than a zone such as Europe/Oslo: Bedework refuses + TZIDs that are tzdata link names, which would hide what is asked here. + """ + + depends_on = {PrepareCalendar} + LABEL = "vtimezone" + FEATURE = "save-load.icalendar.vtimezone" + features_to_be_checked = {FEATURE} + UID = "csc_vtimezone_event" + WHAT = "an event with a VTIMEZONE" + VTIMEZONE = ( + "BEGIN:VTIMEZONE\r\n" + "TZID:Europe/Berlin\r\n" + "BEGIN:DAYLIGHT\r\n" + "TZOFFSETFROM:+0100\r\n" + "TZOFFSETTO:+0200\r\n" + "TZNAME:CEST\r\n" + "DTSTART:19700329T020000\r\n" + "RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU\r\n" + "END:DAYLIGHT\r\n" + "BEGIN:STANDARD\r\n" + "TZOFFSETFROM:+0200\r\n" + "TZOFFSETTO:+0100\r\n" + "TZNAME:CET\r\n" + "DTSTART:19701025T030000\r\n" + "RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU\r\n" + "END:STANDARD\r\n" + "END:VTIMEZONE\r\n" + ) + + def _body(self): + return self._ical(self.UID, vtimezone=self.VTIMEZONE) + + def _grade(self, raw): + dtstart = next((line for line in component_lines(raw, "VEVENT") if line.upper().startswith("DTSTART")), "") + tzid = re.search(r";TZID=([^;:]*)", dtstart, re.IGNORECASE) + if not tzid: + return self._graded("unknown", "cannot test, the event came back without a TZID") + tzid = tzid[1].strip('"') + defined, inside = set(), False + for line in unfolded_lines(raw): + if line.upper() in ("BEGIN:VTIMEZONE", "END:VTIMEZONE"): + inside = line.upper() == "BEGIN:VTIMEZONE" + elif inside and line.upper().startswith("TZID:"): + defined.add(line[len("TZID:") :]) + if tzid in defined: + return "full" + return self._graded("unsupported", f"the event comes back with TZID={tzid} but no VTIMEZONE for it") + + class CheckAttendeePartstat(Check): """ Checks whether an attendee's PARTSTAT can be modified via a direct PUT. diff --git a/tests/test_icalendar_roundtrip.py b/tests/test_icalendar_roundtrip.py new file mode 100644 index 0000000..ce29de2 --- /dev/null +++ b/tests/test_icalendar_roundtrip.py @@ -0,0 +1,101 @@ +"""Whether a PARTICIPANT component and a VTIMEZONE survive a round trip. + +CCS, Bedework 3, robur, GMX and Ox (and Zimbra, reportedly) refuse an event +carrying an RFC 9073 PARTICIPANT; CCS was seen to return an object without the VTIMEZONE it was +sent with. +""" + +import re +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import AuthorizationError + +from caldav_server_tester.checks import CheckParticipant, CheckVtimezone + +from .test_floating_event import RewritingServer + +BASE = "http://example.com/cal/" + + +def _run(check, server): + checker = Mock() + checker._features_checked = FeatureSet() + checker.debug_mode = None + checker._client_obj = server + checker.fixture_base_year = 2027 + checker.calendar.url.join.side_effect = lambda name: BASE + name + check(checker)._run_check() + return checker._features_checked.is_supported(check.FEATURE, return_type=dict) + + +def _drop(component): + return lambda body: re.sub(rf"BEGIN:{component}\r\n.*?END:{component}\r\n", "", body, flags=re.DOTALL) + + +@pytest.mark.parametrize("check", [CheckParticipant, CheckVtimezone]) +def test_kept_is_full(check): + server = RewritingServer() + assert _run(check, server)["support"] == "full" + assert not server.objects + + +def test_the_participant_probe_sends_a_participant_inside_the_event(): + sent = [] + _run(CheckParticipant, RewritingServer(rewrite=lambda body: sent.append(body) or body)) + event = sent[0][sent[0].index("BEGIN:VEVENT") : sent[0].index("END:VEVENT")] + assert "BEGIN:PARTICIPANT\r\n" in event + assert "\r\nPARTICIPANT-TYPE:" in event + + +def test_the_vtimezone_probe_sends_a_tzid_and_its_vtimezone(): + sent = [] + _run(CheckVtimezone, RewritingServer(rewrite=lambda body: sent.append(body) or body)) + tzid = re.search(r"\r\nDTSTART;TZID=([^:]+):", sent[0])[1] + assert f"BEGIN:VTIMEZONE\r\nTZID:{tzid}\r\n" in sent[0] + + +def test_participant_dropped_is_unsupported(): + assert _run(CheckParticipant, RewritingServer(rewrite=_drop("PARTICIPANT")))["support"] == "unsupported" + + +def test_vtimezone_dropped_is_unsupported(): + """CCS: DTSTART keeps its TZID, the VTIMEZONE defining it is gone.""" + verdict = _run(CheckVtimezone, RewritingServer(rewrite=_drop("VTIMEZONE"))) + assert verdict["support"] == "unsupported" + assert "Europe/Berlin" in verdict["behaviour"] + + +def test_vtimezone_for_another_tzid_is_unsupported(): + def rename(body): + return body.replace("BEGIN:VTIMEZONE\r\nTZID:Europe/Berlin", "BEGIN:VTIMEZONE\r\nTZID:Etc/UTC") + + assert _run(CheckVtimezone, RewritingServer(rewrite=rename))["support"] == "unsupported" + + +def test_vtimezone_tzid_need_not_be_its_first_property(): + def push_down(body): + return body.replace("BEGIN:VTIMEZONE\r\n", "BEGIN:VTIMEZONE\r\nLAST-MODIFIED:20200101T000000Z\r\n") + + assert _run(CheckVtimezone, RewritingServer(rewrite=push_down))["support"] == "full" + + +def test_times_converted_to_utc_say_nothing_about_the_vtimezone(): + def to_utc(body): + body = _drop("VTIMEZONE")(body) + return re.sub(r"(DT(?:START|END));TZID=[^:]*:(\d{8}T\d{6})", r"\1:\2Z", body) + + assert _run(CheckVtimezone, RewritingServer(rewrite=to_utc))["support"] == "unknown" + + +@pytest.mark.parametrize("check", [CheckParticipant, CheckVtimezone]) +@pytest.mark.parametrize("answer", [400, 403, 500, AuthorizationError("403 Forbidden")]) +def test_refused_is_ungraceful(check, answer): + assert _run(check, RewritingServer(put_answer=answer))["support"] == "ungraceful" + + +@pytest.mark.parametrize("check", [CheckParticipant, CheckVtimezone]) +def test_unanswered_or_unreadable_is_unknown(check): + assert _run(check, RewritingServer(put_answer=503))["support"] == "unknown" + assert _run(check, RewritingServer(get_status=500))["support"] == "unknown" From 536d943abdd342843099727bf089648f0ca681eb Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 26 Oct 2026 01:43:16 +0100 Subject: [PATCH 07/12] feat: probe a result limit on calendar-query search.limit: a calendar-query for every event, sent along with the multiget and PROPFIND limit probes, so it shares their objects and CALDAV_SERVER_TESTER_LIMIT_OBJECTS. Stalwart stops at its maxResults with a 507; Baikal and CCS answered all 60. Fixes https://github.com/python-caldav/caldav-server-tester/issues/41 Prompt: (...) But those issues you just filed seems like things that are needed for the conformance tests, so please work on them. [https://github.com/python-caldav/caldav-server-tester/issues/38, /39, /40, /41 and https://github.com/python-caldav/caldav/issues/829] Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 2 +- src/caldav_server_tester/checks.py | 34 ++++++++++++++++++++++++------ tests/test_multiget.py | 29 ++++++++++++++++++++----- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8cd17..452e9f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ Needs a caldav with the features below in its `compatibility_hints.py`. - **`save.etag.matches-stored`** - whether a strong ETag in a PUT answer belongs to what was stored: RFC 4791 §5.3.4 lets a server rewrite the object, but not answer with a strong ETag then. The behaviour says what the server changed. - **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice, or only as removed (Nextcloud, Synology). - **`multiget.missing-href`** - whether a calendar-multiget answers a missing href with a 404 entry; Baïkal, Nextcloud, DAViCal, Davis and Infomaniak leave it out, Posteo fails the whole multiget with a 404, and an entry claiming the object exists is broken. -- **`multiget.limit`** and **`propfind.limit`** - whether a calendar-multiget or a Depth: 1 PROPFIND answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. +- **`multiget.limit`**, **`propfind.limit`** and **`search.limit`** - whether a calendar-multiget, a Depth: 1 PROPFIND or a calendar-query answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index 4afe7d2..e55b067 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -7326,13 +7326,14 @@ def _grade_recreated(self, statuses): class CheckMultiget(_RawPut, _RawSync, Check): """ Checks calendar-multiget for a missing href, and limits on how many - members a multiget or a Depth: 1 PROPFIND answers. + members a multiget, a Depth: 1 PROPFIND or a calendar-query answers. RFC 4791 section 7.9 gives every requested href an entry, a 404 for a missing one; Baikal leaves it out. Stalwart answers at most its WebDav.maxResults (2000 by default) members with a 507 for the collection, - even to a multiget that asked for fewer than it holds. Neither request has - a client-side limit to provoke that with, so the probe has to store more + even to a multiget that asked for fewer than it holds, and to a + calendar-query matching more. None of the requests has a + client-side limit to provoke that with, so the probe has to store more objects than the server's limit: CALDAV_SERVER_TESTER_LIMIT_OBJECTS says how many, and without it the limits are not probed. """ @@ -7342,7 +7343,8 @@ class CheckMultiget(_RawPut, _RawSync, Check): MISSING_HREF = "multiget.missing-href" LIMIT = "multiget.limit" PROPFIND_LIMIT = "propfind.limit" - features_to_be_checked = {MISSING_HREF, LIMIT, PROPFIND_LIMIT} + SEARCH_LIMIT = "search.limit" + features_to_be_checked = {MISSING_HREF, LIMIT, PROPFIND_LIMIT, SEARCH_LIMIT} ENV = "CALDAV_SERVER_TESTER_LIMIT_OBJECTS" PRESENT = "csc_multiget_present" ABSENT = "csc_multiget_missing" @@ -7364,7 +7366,7 @@ def _run_check(self) -> None: self._delete(present) count = self._object_count() if not count: - for feature in (self.LIMIT, self.PROPFIND_LIMIT): + for feature in (self.LIMIT, self.PROPFIND_LIMIT, self.SEARCH_LIMIT): self.set_feature( feature, {"support": "unknown", "behaviour": f"not probed: set {self.ENV} to a number of objects"} ) @@ -7409,13 +7411,17 @@ def _probe_limits(self, count) -> None: try: stored = sum(self._ok(self._put(self._url(name), self._ical(name))) for name in names) if stored < count: - for feature in (self.LIMIT, self.PROPFIND_LIMIT): + for feature in (self.LIMIT, self.PROPFIND_LIMIT, self.SEARCH_LIMIT): self.set_feature( feature, self._graded("unknown", f"cannot test, only {stored} of {count} objects were stored") ) return mine = {f"{name}.ics" for name in names} - requests = ((self.LIMIT, lambda: self._multiget(names)), (self.PROPFIND_LIMIT, self._propfind)) + requests = ( + (self.LIMIT, lambda: self._multiget(names)), + (self.PROPFIND_LIMIT, self._propfind), + (self.SEARCH_LIMIT, self._calendar_query), + ) for feature, request in requests: try: (entries, truncated) = request() @@ -7436,6 +7442,20 @@ def _probe_limits(self, count) -> None: for name in names: self._delete(self._url(name)) + def _calendar_query(self): + """Every event in the calendar, raw for the same reason as the multiget""" + body = ( + '' + '' + "" + '' + "" + ) + response = self.client.report(str(self.cal_url), body, depth=1) + if response.status not in (200, 207) or response.tree is None: + raise ReportError(f"calendar-query answered {response.status}") + return self._parse_entries(response, propstat_status=True) + def _propfind(self): ## Raw, as for the multiget: caldav's propfind() parses at once and ## raises on the collection's 507, hiding the members listed before it diff --git a/tests/test_multiget.py b/tests/test_multiget.py index 37f5cd7..c022f48 100644 --- a/tests/test_multiget.py +++ b/tests/test_multiget.py @@ -3,7 +3,8 @@ RFC 4791 section 7.9 gives every requested href an entry in the multistatus, a 404 for a missing one; Baikal leaves it out. Stalwart answers a multiget or a Depth: 1 PROPFIND with at most WebDav.maxResults members and a 507 for -the collection. The limit is only probed with +the collection, and a calendar-query matching more the same way. The limits +are only probed with CALDAV_SERVER_TESTER_LIMIT_OBJECTS set, since it takes that many objects. """ @@ -22,6 +23,7 @@ MISSING = "multiget.missing-href" LIMIT = "multiget.limit" PROPFIND = "propfind.limit" +SEARCH = "search.limit" ENV = "CALDAV_SERVER_TESTER_LIMIT_OBJECTS" @@ -30,7 +32,8 @@ class MultigetServer: most that many members, with a 507 for the collection unless ``cap_marker`` is False. ``missing_as`` answers a missing href with that entry instead of a response-level 404: "propstat 404", "propstat 200" or a - response-level status code.""" + response-level status code. ``query_status`` is the status of every + calendar-query answer.""" def __init__( self, @@ -41,7 +44,9 @@ def __init__( propfind_raises=None, missing_as=None, propfind_status=None, + query_status=207, ): + self.query_status = query_status self.propfind_status = propfind_status self.missing_as = missing_as self.propfind_raises = propfind_raises @@ -66,6 +71,10 @@ def delete(self, url, headers=None): return Mock(status=204) def report(self, url, body, depth=0): + if "calendar-query" in body: + if self.query_status != 207: + return Mock(status=self.query_status, tree=None, raw="") + return self._answer([(n, None) for n in sorted(self.objects)]) if self.report_status != 207: return Mock(status=self.report_status, tree=None, raw="") names = [self._name(h) for h in re.findall(r"(.*?)", body)] @@ -132,7 +141,7 @@ def _run(server, monkeypatch, objects=None): checker.calendar.url = URL.objectify(BASE) CheckMultiget(checker)._run_check() fs = checker._features_checked - return {f: fs.is_supported(f, return_type=dict) for f in (MISSING, LIMIT, PROPFIND)} + return {f: fs.is_supported(f, return_type=dict) for f in (MISSING, LIMIT, PROPFIND, SEARCH)} def test_a_404_entry_for_the_missing_href_is_full(monkeypatch): @@ -174,7 +183,7 @@ def test_the_limits_are_not_probed_by_default(monkeypatch): server = MultigetServer() verdicts = _run(server, monkeypatch) assert server.puts == 1 - for feature in (LIMIT, PROPFIND): + for feature in (LIMIT, PROPFIND, SEARCH): assert verdicts[feature]["support"] == "unknown" assert ENV in verdicts[feature]["behaviour"] @@ -182,7 +191,7 @@ def test_the_limits_are_not_probed_by_default(monkeypatch): def test_no_limit_up_to_the_objects_stored(monkeypatch): server = MultigetServer() verdicts = _run(server, monkeypatch, objects=12) - for feature in (LIMIT, PROPFIND): + for feature in (LIMIT, PROPFIND, SEARCH): assert verdicts[feature] == {"support": "full", "behaviour": "no limit up to 12 objects"} assert not server.objects @@ -192,6 +201,7 @@ def test_a_cap_with_a_507_is_unsupported(monkeypatch): verdicts = _run(MultigetServer(cap=5), monkeypatch, objects=12) assert verdicts[LIMIT] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} assert verdicts[PROPFIND] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} + assert verdicts[SEARCH] == {"support": "unsupported", "behaviour": "stops at 5 members, with a 507"} def test_a_propfind_cap_counts_every_member_listed(monkeypatch): @@ -236,3 +246,12 @@ def test_a_multiget_answered_507_as_a_whole_is_unsupported(monkeypatch): def test_another_failure_is_ungraceful(monkeypatch): server = MultigetServer(propfind_raises=ResponseError("HTTP/1.1 500 Internal Server Error")) assert _run(server, monkeypatch, objects=12)[PROPFIND]["support"] == "ungraceful" + + +def test_a_calendar_query_answered_507_as_a_whole_is_unsupported(monkeypatch): + verdict = _run(MultigetServer(query_status=507), monkeypatch, objects=12)[SEARCH] + assert verdict == {"support": "unsupported", "behaviour": "the whole answer was a 507, with 12 objects"} + + +def test_a_failing_calendar_query_is_ungraceful(monkeypatch): + assert _run(MultigetServer(query_status=500), monkeypatch, objects=12)[SEARCH]["support"] == "ungraceful" From 5f6c271044daa14bd6c4d39d8d4af6c7d528d306 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 26 Oct 2026 01:51:31 +0100 Subject: [PATCH 08/12] feat: probe data dropped silently from objects Four round-trip probes, for an X- property (in an event and in a task), an unknown property (ESTIMATED-DURATION), an ATTENDEE on a task, and COMPLETED on a task without STATUS. Ox fails all four; the other docker servers keep everything. A task Ox answers with 201 and then 404 is graded broken, but only where the same task without the ATTENDEE reads back. _RoundTrip now stores a task in the task calendar, and where tasks cannot be stored grades a task-only feature outside save-load.todo unknown. Fixes https://github.com/python-caldav/caldav-server-tester/issues/40 Prompt: (...) But those issues you just filed seems like things that are needed for the conformance tests, so please work on them. [https://github.com/python-caldav/caldav-server-tester/issues/38, /39, /40, /41 and https://github.com/python-caldav/caldav/issues/829] Prompts (comments made inline in the document while reviewing it): > (on whether a task that vanishes after a 201 is 'unsupported' or 'broken') broken Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 1 + src/caldav_server_tester/checks.py | 206 +++++++++++++++++++++++++---- tests/test_icalendar_roundtrip.py | 55 +++++++- tests/test_todo_roundtrip.py | 154 +++++++++++++++++++++ 4 files changed, 390 insertions(+), 26 deletions(-) create mode 100644 tests/test_todo_roundtrip.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 452e9f8..d33c8d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Needs a caldav with the features below in its `compatibility_hints.py`. - **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. - **`save-load.icalendar.participant`** - whether an RFC 9073 PARTICIPANT inside a VEVENT is stored. CCS and Bedework 3 refuse the event with 403, robur with 400, GMX and Ox with 500. See https://github.com/python-caldav/caldav-server-tester/issues/38. - **`save-load.icalendar.vtimezone`** - whether the VTIMEZONE sent with an event comes back with it. CCS returns the event's TZID without the VTIMEZONE. See https://github.com/python-caldav/caldav-server-tester/issues/39. +- **`save-load.icalendar.x-property`**, **`save-load.icalendar.unknown-property`**, **`save-load.todo.attendee`** and **`save-load.todo.completed-without-status`** - whether an X- property (in an event and in a task), a property RFC 5545 does not define (ESTIMATED-DURATION), an ATTENDEE on a task, and COMPLETED on a task without STATUS are kept. Ox drops all of them (the X- property only from a task); a task with an ATTENDEE is answered 201, then 404. See https://github.com/python-caldav/caldav-server-tester/issues/40. - **`delete.missing`** and **`delete.missing.if-match`** - whether a DELETE of an object that does not exist answers 404, and one with If-Match answers as the plain one does, as RFC 9110 §13.2.1 has it. Baïkal, Davis, Nextcloud, CCS, OX, Robur, Purelymail and Infomaniak answer 412 with If-Match, so a delete of an object someone else already deleted reads as an etag mismatch. - **`save.etag.matches-stored`** - whether a strong ETag in a PUT answer belongs to what was stored: RFC 4791 §5.3.4 lets a server rewrite the object, but not answer with a strong ETag then. The behaviour says what the server changed. - **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice, or only as removed (Nextcloud, Synology). diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index e55b067..3221486 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -4249,30 +4249,34 @@ class _RawPut: ## Names the probe in the objects it stores, should one be left behind LABEL = "put-etag" + ## The checker attribute holding the calendar the probe stores in + CALENDAR = "calendar" def _url(self, name): - return str(self.checker.calendar.url.join(f"{name}.ics")) + return str(getattr(self.checker, self.CALENDAR).url.join(f"{name}.ics")) - def _ical(self, uid, summary=None, floating=False, extra="", vtimezone=None): + def _ical(self, uid, summary=None, floating=False, extra="", vtimezone=None, todo=False): """An event at the fixture year's 20 January 12:00 UTC, or at 12:00 wherever the user is with ``floating``, or in the time zone of the - ``vtimezone`` text, which goes along; ``extra`` is more lines.""" + ``vtimezone`` text, which goes along; ``extra`` is more lines. With + ``todo``, a task starting then and due an hour later.""" year = self.checker.fixture_base_year utc = "" if floating or vtimezone else "Z" tzid = f";TZID={re.search(r'^TZID:(.*)$', vtimezone, re.MULTILINE)[1].strip()}" if vtimezone else "" + component = "VTODO" if todo else "VEVENT" return ( "BEGIN:VCALENDAR\r\n" "VERSION:2.0\r\n" f"PRODID:-//caldav-server-tester//{self.LABEL} probe//EN\r\n" f"{vtimezone or ''}" - "BEGIN:VEVENT\r\n" + f"BEGIN:{component}\r\n" f"UID:{uid}\r\n" f"DTSTAMP:{year}0120T120000Z\r\n" f"DTSTART{tzid}:{year}0120T120000{utc}\r\n" - f"DTEND{tzid}:{year}0120T130000{utc}\r\n" + f"{'DUE' if todo else 'DTEND'}{tzid}:{year}0120T130000{utc}\r\n" f"SUMMARY:{summary or self.LABEL + ' probe'}\r\n" f"{extra}" - "END:VEVENT\r\n" + f"END:{component}\r\n" "END:VCALENDAR\r\n" ) @@ -4749,36 +4753,94 @@ def component_lines(ical, name): class _RoundTrip(_RawPut): - """Stores the event ``_body()`` gives, reads it back, and has ``_grade()`` - judge the server's own text. Refused is "ungraceful".""" + """Stores the object ``_body()`` gives, reads it back, and has ``_grade()`` + judge the server's own text. Refused is "ungraceful". + + One object for each of ``COMPONENTS``, the first verdict that is not + "full" being the feature's. A task goes to the task calendar, which is + the event calendar where the two mix, and is not probed where tasks cannot + be stored: a save-load.todo feature probing tasks only is then left unset, + so save-load.todo's status propagates to it, and any other is "unknown", + as is every one where there is no task calendar at all.""" ## The probe object's UID and name, and what it is, for the behaviour notes UID = "csc_round_trip" WHAT = "the event" - ## More lines for the VEVENT + ## More lines for the VEVENT or VTODO EXTRA = "" + COMPONENTS = ("VEVENT",) + ## The one being probed + component = "VEVENT" + CANNOT_STORE = "tasks cannot be stored" + + @property + def uid(self): + """The UID and name of the object probed now: one per component, so a + task never lands where the event was just deleted""" + return self.UID if len(self.COMPONENTS) == 1 else f"{self.UID}_{self.component.lower()}" def _body(self): - return self._ical(self.UID, extra=self.EXTRA) + return self._ical(self.uid, extra=self.EXTRA, todo=self.component == "VTODO") def _run_check(self) -> None: - url = self._url(self.UID) + verdict, skipped = None, None + for component in self.COMPONENTS: + if component == "VTODO": + skipped = self._tasks_unstorable() + if skipped: + continue + self.component = component + verdict = self._round_trip() + if verdict != "full": + if len(self.COMPONENTS) > 1 and isinstance(verdict, dict): + kind = "a task" if component == "VTODO" else "an event" + verdict = {**verdict, "behaviour": f"{verdict['behaviour']}, in {kind}"} + break + if verdict == "full" and skipped: + verdict = self._graded("full", f"probed in an event only, {skipped}") + elif verdict is None and (skipped != self.CANNOT_STORE or not self.FEATURE.startswith("save-load.todo.")): + verdict = self._graded("unknown", f"cannot test, {skipped}") + if verdict is not None: + self.set_feature(self.FEATURE, verdict) + + def _tasks_unstorable(self): + """Why no task can be probed, or None where one can""" + if getattr(self.checker, "tasklist", None) is None: + return "there is no task calendar" + if not self.checker.features_checked.is_supported("save-load.todo"): + return self.CANNOT_STORE + return None + + def _round_trip(self): + url = self._url(self.uid) try: status = self._status(self._put(url, self._body())) if status is None: - verdict = self._graded("unknown", f"cannot test, {self.WHAT} got no answer") - elif not 200 <= status < 300: - verdict = self._graded("ungraceful", f"{self.WHAT} is refused ({status})") - else: - response = self._get(url) - if response is None: - verdict = self._graded("unknown", f"cannot test, {self.WHAT} could not be read back") - else: - verdict = self._grade(response.raw) - self.set_feature(self.FEATURE, verdict) + return self._graded("unknown", f"cannot test, {self.WHAT} got no answer") + if not 200 <= status < 300: + return self._graded("ungraceful", f"{self.WHAT} is refused ({status})") + response = self._answer(self.client.request, url) + if self._ok(response): + return self._grade(response.raw) + return self._unreadable(self._status(response)) finally: self._delete(url) + @property + def CALENDAR(self): + return "tasklist" if self.component == "VTODO" else "calendar" + + def _unreadable(self, status): + """The verdict where the stored object cannot be read back""" + answer = f" ({status})" if status else "" + return self._graded("unknown", f"cannot test, {self.WHAT} could not be read back{answer}") + + def _kept(self, raw, name): + """``_grade()`` for a probe asking whether property ``name`` is kept""" + if any(re.match(rf"{name}[;:]", line, re.IGNORECASE) for line in component_lines(raw, self.component)): + return "full" + return self._graded("unsupported", f"{name} is dropped") + class CheckFloatingEvent(_RoundTrip, Check): """ @@ -4800,7 +4862,7 @@ class CheckFloatingEvent(_RoundTrip, Check): PROPERTY = re.compile(r"(DTSTART|DTEND)((?:;[^:]*)?):(.*)") def _body(self): - return self._ical(self.UID, floating=True) + return self._ical(self.uid, floating=True) def _grade(self, raw): times = [match.groups() for match in map(self.PROPERTY.fullmatch, component_lines(raw, "VEVENT")) if match] @@ -4956,7 +5018,7 @@ class CheckVtimezone(_RoundTrip, Check): ) def _body(self): - return self._ical(self.UID, vtimezone=self.VTIMEZONE) + return self._ical(self.uid, vtimezone=self.VTIMEZONE) def _grade(self, raw): dtstart = next((line for line in component_lines(raw, "VEVENT") if line.upper().startswith("DTSTART")), "") @@ -4975,6 +5037,104 @@ def _grade(self, raw): return self._graded("unsupported", f"the event comes back with TZID={tzid} but no VTIMEZONE for it") +class CheckXProperty(_RoundTrip, Check): + """ + Checks whether an X- property (RFC 5545 section 3.8.8.2) is stored, in an + event and in a task. Ox keeps it in an event and drops it from a task. + """ + + depends_on = {PrepareCalendar} + LABEL = "x-property" + FEATURE = "save-load.icalendar.x-property" + features_to_be_checked = {FEATURE} + UID = "csc_x_property_event" + WHAT = "an object with an X- property" + EXTRA = "X-CALDAV-SERVER-TESTER:kept\r\n" + COMPONENTS = ("VEVENT", "VTODO") + + def _grade(self, raw): + return self._kept(raw, "X-CALDAV-SERVER-TESTER") + + +class CheckUnknownProperty(_RoundTrip, Check): + """ + Checks whether a property that is neither in RFC 5545 nor an X- one is + stored, using ESTIMATED-DURATION on a task. Ox drops it. + """ + + depends_on = {PrepareCalendar} + LABEL = "unknown-property" + FEATURE = "save-load.icalendar.unknown-property" + features_to_be_checked = {FEATURE} + UID = "csc_unknown_property_task" + WHAT = "a task with ESTIMATED-DURATION" + COMPONENTS = ("VTODO",) + EXTRA = "ESTIMATED-DURATION:PT2H\r\n" + + def _grade(self, raw): + return self._kept(raw, "ESTIMATED-DURATION") + + +class CheckTodoAttendee(_RoundTrip, Check): + """ + Checks whether a task with an ATTENDEE is stored with it. Ox answers the + PUT with 201 and the GET with 404: the task is gone, which is "broken", + since the server said it was stored. A 404 counts only where the same + task without the ATTENDEE reads back. There is no ORGANIZER, so the + server has no scheduling reason to touch it. + """ + + depends_on = {PrepareCalendar} + LABEL = "todo-attendee" + FEATURE = "save-load.todo.attendee" + features_to_be_checked = {FEATURE} + UID = "csc_attendee_task" + WHAT = "a task with an ATTENDEE" + COMPONENTS = ("VTODO",) + EXTRA = "ATTENDEE;CN=Bob:mailto:csc-bob@attendee.example.com\r\n" + + def _grade(self, raw): + return self._kept(raw, "ATTENDEE") + + def _unreadable(self, status): + if status == 404 and self._plain_task_kept(): + return self._graded("broken", "the task is accepted, then gone (404)") + return super()._unreadable(status) + + def _plain_task_kept(self): + """Whether the same task without the ATTENDEE can be read back""" + uid = f"{self.UID}_control" + url = self._url(uid) + try: + if not self._ok(self._put(url, self._ical(uid, todo=True))): + return False + return self._ok(self._answer(self.client.request, url)) + finally: + self._delete(url) + + +class CheckCompletedWithoutStatus(_RoundTrip, Check): + """ + Checks whether a task with COMPLETED but no STATUS keeps its COMPLETED. + Ox drops it. + """ + + depends_on = {PrepareCalendar} + LABEL = "completed-without-status" + FEATURE = "save-load.todo.completed-without-status" + features_to_be_checked = {FEATURE} + UID = "csc_completed_task" + WHAT = "a task with COMPLETED but no STATUS" + COMPONENTS = ("VTODO",) + + @property + def EXTRA(self): + return f"COMPLETED:{self.checker.fixture_base_year}0120T130000Z\r\n" + + def _grade(self, raw): + return self._kept(raw, "COMPLETED") + + class CheckAttendeePartstat(Check): """ Checks whether an attendee's PARTSTAT can be modified via a direct PUT. diff --git a/tests/test_icalendar_roundtrip.py b/tests/test_icalendar_roundtrip.py index ce29de2..106697c 100644 --- a/tests/test_icalendar_roundtrip.py +++ b/tests/test_icalendar_roundtrip.py @@ -12,20 +12,23 @@ from caldav.compatibility_hints import FeatureSet from caldav.lib.error import AuthorizationError -from caldav_server_tester.checks import CheckParticipant, CheckVtimezone +from caldav_server_tester.checks import CheckParticipant, CheckVtimezone, CheckXProperty from .test_floating_event import RewritingServer BASE = "http://example.com/cal/" +TASKS = "http://example.com/tasks/" -def _run(check, server): +def _run(check, server, todo="full"): checker = Mock() - checker._features_checked = FeatureSet() + checker._features_checked = FeatureSet({"save-load.todo": {"support": todo}}) + checker.features_checked = checker._features_checked checker.debug_mode = None checker._client_obj = server checker.fixture_base_year = 2027 checker.calendar.url.join.side_effect = lambda name: BASE + name + checker.tasklist.url.join.side_effect = lambda name: TASKS + name check(checker)._run_check() return checker._features_checked.is_supported(check.FEATURE, return_type=dict) @@ -99,3 +102,49 @@ def test_refused_is_ungraceful(check, answer): def test_unanswered_or_unreadable_is_unknown(check): assert _run(check, RewritingServer(put_answer=503))["support"] == "unknown" assert _run(check, RewritingServer(get_status=500))["support"] == "unknown" + + +def test_x_property_kept_is_full_in_an_event_and_a_task(): + sent = [] + assert _run(CheckXProperty, RewritingServer(rewrite=lambda body: sent.append(body) or body))["support"] == "full" + assert [re.search(r"BEGIN:(VEVENT|VTODO)", body)[1] for body in sent] == ["VEVENT", "VTODO"] + assert all(re.search(r"\r\nX-[A-Z-]+:", body) for body in sent) + + +def test_x_property_dropped_is_unsupported(): + verdict = _run(CheckXProperty, RewritingServer(rewrite=lambda body: re.sub(r"X-[^\r]*\r\n", "", body))) + assert verdict == {"support": "unsupported", "behaviour": "X-CALDAV-SERVER-TESTER is dropped, in an event"} + + +def test_x_property_dropped_from_a_task_only_is_unsupported(): + """Ox""" + + def from_tasks(body): + return re.sub(r"X-[^\r]*\r\n", "", body) if "BEGIN:VTODO" in body else body + + verdict = _run(CheckXProperty, RewritingServer(rewrite=from_tasks)) + assert verdict == {"support": "unsupported", "behaviour": "X-CALDAV-SERVER-TESTER is dropped, in a task"} + + +def test_x_property_in_events_only_where_tasks_cannot_be_stored(): + sent = [] + verdict = _run(CheckXProperty, RewritingServer(rewrite=lambda body: sent.append(body) or body), todo="unsupported") + assert verdict == {"support": "full", "behaviour": "probed in an event only, tasks cannot be stored"} + assert len(sent) == 1 and "BEGIN:VEVENT" in sent[0] + + +def test_x_property_event_and_task_go_to_their_own_calendars_and_urls(): + server = RewritingServer() + urls = [] + put = server.put + server.put = lambda url, body, headers=None: urls.append(url) or put(url, body, headers) + _run(CheckXProperty, server) + assert urls[0].startswith(BASE) and urls[1].startswith(TASKS) + assert urls[0].rsplit("/", 1)[1] != urls[1].rsplit("/", 1)[1] + + +@pytest.mark.parametrize("check", [CheckParticipant, CheckVtimezone]) +def test_unread_without_a_status_says_so_plainly(check): + verdict = _run(check, RewritingServer(get_status=503)) + assert verdict["support"] == "unknown" + assert "None" not in verdict["behaviour"] diff --git a/tests/test_todo_roundtrip.py b/tests/test_todo_roundtrip.py new file mode 100644 index 0000000..0ca7392 --- /dev/null +++ b/tests/test_todo_roundtrip.py @@ -0,0 +1,154 @@ +"""Whether a task keeps what it was stored with. + +Ox answers a task carrying an ATTENDEE with 201 and then 404 on GET, drops +ESTIMATED-DURATION, and drops COMPLETED where the task has no STATUS. +""" + +import re +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet + +from caldav_server_tester.checks import ( + CheckCompletedWithoutStatus, + CheckTodoAttendee, + CheckUnknownProperty, + PrepareCalendar, +) + +from .test_floating_event import RewritingServer + +BASE = "http://example.com/cal/" +TASKS = "http://example.com/tasks/" +CHECKS = [CheckTodoAttendee, CheckUnknownProperty, CheckCompletedWithoutStatus] + + +class VanishingServer(RewritingServer): + """Accepts the PUT, but stores nothing: with ``everything``, no object at + all is readable at the URL it was put to; otherwise only a task with an + ATTENDEE vanishes (Ox).""" + + def __init__(self, everything=False, **kwargs): + super().__init__(**kwargs) + self.everything = everything + + def put(self, url, body, headers=None): + if self.everything or "\r\nATTENDEE" in body: + return Mock(status=201, headers={}) + return super().put(url, body, headers) + + +def _run(check, server, todo="full", tasklist=True, whole=False): + """``whole``: through run_check(), with its check that every declared + feature got a value""" + checker = Mock() + checker._checks_run = {PrepareCalendar} + checker._features_checked = FeatureSet({"save-load.todo": {"support": todo}}) + checker.features_checked = checker._features_checked + checker.debug_mode = None + checker._client_obj = server + server.features = FeatureSet() + checker.fixture_base_year = 2027 + checker.calendar.url.join.side_effect = lambda name: BASE + name + if tasklist: + checker.tasklist.url.join.side_effect = lambda name: TASKS + name + else: + checker.tasklist = None + if whole: + check(checker).run_check() + else: + check(checker)._run_check() + return checker._features_checked.is_supported(check.FEATURE, return_type=dict, return_defaults=False) + + +def _drop(name): + return lambda body: re.sub(rf"\r\n{name}[;:][^\r]*", "", body) + + +@pytest.mark.parametrize("check", CHECKS) +def test_kept_is_full_and_stored_as_a_task_on_the_task_list(check): + sent = [] + server = RewritingServer(rewrite=lambda body: sent.append(body) or body) + assert _run(check, server)["support"] == "full" + assert list(server.objects) == [] + assert "BEGIN:VTODO\r\n" in sent[0] and "VEVENT" not in sent[0] + + +@pytest.mark.parametrize("check", CHECKS) +def test_a_task_lands_in_the_task_calendar(check): + server = RewritingServer() + urls = [] + put = server.put + server.put = lambda url, body, headers=None: urls.append(url) or put(url, body, headers) + _run(check, server) + assert urls[0].startswith(TASKS) + + +def test_a_task_with_an_attendee_that_vanishes_is_broken(): + """Ox: 201, then 404, while a task without the ATTENDEE reads back""" + server = VanishingServer() + verdict = _run(CheckTodoAttendee, server) + assert verdict["support"] == "broken" + assert "404" in verdict["behaviour"] + assert not server.objects + + +def test_a_404_for_every_object_says_nothing_about_the_attendee(): + """Objects not reachable at the URL they were put to (Zimbra's cal_id alias)""" + assert _run(CheckTodoAttendee, VanishingServer(everything=True))["support"] == "unknown" + + +def test_the_attendee_dropped_is_unsupported(): + assert _run(CheckTodoAttendee, RewritingServer(rewrite=_drop("ATTENDEE")))["support"] == "unsupported" + + +def test_estimated_duration_dropped_is_unsupported(): + verdict = _run(CheckUnknownProperty, RewritingServer(rewrite=_drop("ESTIMATED-DURATION"))) + assert verdict["support"] == "unsupported" + assert "ESTIMATED-DURATION" in verdict["behaviour"] + + +def test_completed_without_status_is_sent_without_status(): + sent = [] + _run(CheckCompletedWithoutStatus, RewritingServer(rewrite=lambda body: sent.append(body) or body)) + assert "\r\nCOMPLETED:" in sent[0] + assert "\r\nSTATUS:" not in sent[0] + + +def test_completed_dropped_is_unsupported(): + assert _run(CheckCompletedWithoutStatus, RewritingServer(rewrite=_drop("COMPLETED")))["support"] == "unsupported" + + +def test_a_status_added_with_completed_kept_is_full(): + def add_status(body): + return body.replace("\r\nCOMPLETED:", "\r\nSTATUS:COMPLETED\r\nCOMPLETED:") + + assert _run(CheckCompletedWithoutStatus, RewritingServer(rewrite=add_status))["support"] == "full" + + +@pytest.mark.parametrize("check", CHECKS) +def test_refused_is_ungraceful(check): + assert _run(check, RewritingServer(put_answer=403))["support"] == "ungraceful" + + +@pytest.mark.parametrize("check", CHECKS) +@pytest.mark.parametrize("todo", ["unsupported", "ungraceful"]) +def test_not_probed_where_tasks_cannot_be_stored(check, todo): + """A save-load.todo child is left unset and inherits its status; any + other feature is unknown""" + server = RewritingServer(put_answer=AssertionError("nothing is to be stored")) + verdict = _run(check, server, todo=todo, whole=True) + if check.FEATURE.startswith("save-load.todo."): + assert verdict["support"] == todo + else: + assert verdict == {"support": "unknown", "behaviour": "cannot test, tasks cannot be stored"} + + +@pytest.mark.parametrize("check", CHECKS) +def test_no_task_calendar_is_unknown(check): + assert _run(check, RewritingServer(), tasklist=False)["support"] == "unknown" + + +def test_an_unreadable_task_other_than_404_is_unknown(): + assert _run(CheckTodoAttendee, RewritingServer(get_status=500))["support"] == "unknown" From f9b5529daeb22b132a3751190962a66dd129dd0e Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 26 Oct 2026 11:18:27 +0100 Subject: [PATCH 09/12] docs: TODO on Bedework TZIDs and delete.if-match Two findings from the compset-multiple handover that no issue or TODO entry tracked: Bedework refuses tzdata link names as TZID, and the delete.if-match hint has no consumer. Prompt: Probably the "also found"-items in the old handover doc is on the github issue tracker, please verify. (...) And continue working with the handover. [the handover of a previous session listed Bedework TZID link names and delete.if-match among findings; those two were on no tracker] Followup-Prompt: fold #1 into an open PR. [the TODO commit sat on a local branch; it goes onto the VTIMEZONE probe PR] Prompts (comments made inline in the document while reviewing it): > (on a review finding about the TODO naming no project) Add a link to github pycalendar/calendaring Assisted-By: Claude Sonnet 5.5 Reviewed-by: Tobias Brox --- docs/TODO.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/TODO.md b/docs/TODO.md index 8f359a2..66b2477 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -523,3 +523,28 @@ Against it: If it is done, the generic shape is a hook in the test-server definition ("expire the change history now"), with the probe `unknown` wherever no hook is defined; Stalwart would be the only server with one. + +# Bedework refuses TZIDs that are tzdata link names + +Bedework answers 403 `unknownTimezone` for a TZID that is a tzdata link name +(Europe/Oslo, Europe/Stockholm, Europe/Amsterdam, Europe/Kiev), even with a +matching VTIMEZONE in the object. Canonical names (Europe/Berlin, +Europe/Paris, America/New_York) are accepted. Seen 2026-10-26 on the +Bedework3 docker server. + +No probe or hint covers it. The `save-load.icalendar.vtimezone` probe +already sidesteps it by using Europe/Berlin, which is deliberate; only +Bedework 3 was tried, Bedework 5 was not. A probe would save an event with a +link-name TZID (with and without VTIMEZONE) next to a canonical-name control, +and grade the refusal `ungraceful`; it belongs with that probe +(https://github.com/python-caldav/caldav-server-tester/issues/39). + +# `delete.if-match: broken` has no consumer + +The hint is set for Zimbra, SOGo, Bedework 3 and 5, and Posteo, but nothing +acts on it: caldav's `delete(if_match=)` does not read it, and neither, as far +as known, does https://github.com/PyCalendar/calendaring. `broken` means the +server deletes whatever the etag says, so skipping the header would change +nothing. Decide whether `delete(if_match=True)` should warn or raise when the +hint says `broken`, or emulate the check with a GET and an etag compare +before the DELETE (racy, but closer to what was asked for). From dc7df4a4f55c40e98d12680361339b7913ab8ac7 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Mon, 26 Oct 2026 12:43:30 +0100 Subject: [PATCH 10/12] feat: grade component-set features apart The component-set probe folded three questions into one grade. It now sets supported-components.property-exists (the former get-supported-components, read from the test calendar) and .honoured (a calendar takes exactly what it lists, RFC 4791 section 5.2.3), and create-calendar.with-supported-component-types asks again whether MKCALENDAR yields the requested set. A new .multiple child asks the same for a set of several types. Bedework 3.10, DAViCal and Stalwart are honoured: broken. Zimbra's VFREEBUSY is ignored, and a VJOURNAL is saved too. Needs caldav 3.7.0, which has the supported-components features. Prompt: Do proper research and fix the probe now [the component-set probe graded Bedework 3.10 full although a VTODO-only calendar there still accepts a VEVENT] Followup-Prompt: Perhaps we need several children features here to catch all the degrees of behaviour? Followup-Prompt: Perhaps put in a comment note in the compatibility matrix that the proposed .multiple that this is already covered by save-load.todo.mixed-calendar [superseded below: .multiple became a feature of its own] Followup-Prompt: go for version 2 then [existence and obedience as sibling features under a grouping node, creation kept apart] Followup-Prompt: go for it, and then implement [after an independent review of the design] Followup-Prompt: finish it yourself [the tester side, after the implementing agent hit its usage limit] Followup-Prompt: yes, zimbra should be up now. nextcloud should also be up? The pin can be bumped. It can be tested locally towards the local caldav installation, and on github the pin for caldav ... it should be 3.7.0 shouldn't it? ... will cause the integration tests to fail fast rather than slow Prompts (comments made inline the "proposed alternative solutions" document while reviewing it): > (on RFC 4791 section 5.2.3) I read [this] that the existence of the [supported-components] property is one feature, enforcement of the property is a subfeature. > then comes the problem when the feature is unsupported ... then that "unlocks" another feature "the server does accept all component types". This is not very easy to squeeze into the feature tree as it is. We had some similar problem with the etag given on servers rewriting vs not rewriting the icalendar data, please check up if we can do something similar here. > [and] This has per se nothing to do with calendar creation if I understand it correctly. > (on RFC 4791 section 5.3.1.1) [Therefore we should have a] child property under calendar creation on weather (sic) the calendar creation matches the support levels of supported-calendar-component > Also, although .multiple is related to (sic) .mixed-calendar it's not the same thing being measured, so we should add this one too. > Please consider an "option E" based on this Prompts (2nd iteration): > (on the proposed feature tree) I'm not sure if this is possible. If get-supported-components is unsupported, the default for get-supported-components.honoured will also be unsupported. Prompt (on a suggestion to regrade Robur in the matrix in the caldav library): > (on a review finding about Robur's honoured hint) This should be fixed in caldav-server-tester - the features defined for a server can only deviate if one of the sides returns "fragile" or "unknown". If the caldav-server-tester tests only with VTODO and not VJOURNAL, it needs fixing. > (I also argued for raising the caldav version pin to 3.7.0 - it doesn't matter during local testing, but it makes the github CI fail fast instead of churning CPU and then failing. Probably a longer-term solution should be made here, this is a constant itch) Assisted-By: Claude Sonnet 5.5, Claude Opus 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 7 +- docs/ROADMAP.md | 4 +- docs/TODO.md | 5 +- pyproject.toml | 2 +- src/caldav_server_tester/checks.py | 635 +++++++++++++++++--------- tests/test_supported_component_set.py | 550 +++++++++++++++++----- 6 files changed, 878 insertions(+), 325 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d33c8d9..26fc7ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,11 @@ This library is tightly dependent on the CalDAV-library, particularly the `compa ## Unreleased -Needs a caldav with the features below in its `compatibility_hints.py`. +Needs caldav 3.7.0, which has the features below in its `compatibility_hints.py`. ### Added +- **`supported-components.property-exists`** (the former `get-supported-components`, now probed), **`supported-components.honoured`** and **`create-calendar.with-supported-component-types.multiple`** - whether a calendar reports the component set, whether it takes exactly the types it lists (and every type where it lists none, RFC 4791 §5.2.3), and whether a MKCALENDAR naming several types gives a calendar taking all of them. A calendar that reports `[VTODO]` and still takes a VEVENT is `broken` on `.honoured` (Bedework 3.10, DAViCal, Stalwart); CCS refuses a set of several components the §5.3.1.1 way and is `unsupported` on `.multiple`. - **`delete.if-match`** - whether a DELETE with an If-Match etag the object no longer has is refused with 412, and one with the current etag goes through. SOGo refuses a stale If-Match on PUT, but deletes the object on DELETE. - **`save-load.event.floating`** - whether an event with a floating DTSTART and DTEND is stored floating. SOGo gives the times TZID=Etc/UTC (tasks it keeps floating). The probe reads the server's own text: the icalendar library writes a TZID=Etc/UTC time back with a `Z` after it, which RFC 5545 §3.3.5 forbids. - **`save-load.icalendar.attendee-params`** - whether ATTENDEE parameters are stored as sent. An added RFC 5545 default (PARTSTAT=NEEDS-ACTION and the like) or SCHEDULE-STATUS is fine; SOGo also adds RSVP=TRUE, where the default is FALSE. @@ -28,6 +29,10 @@ Needs a caldav with the features below in its `compatibility_hints.py`. The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst +### Fixed + +- **`create-calendar.with-supported-component-types`** - back to asking whether a MKCALENDAR naming `VTODO` gives a calendar with that set. Bedework 5 and SOGo report another set and are `unsupported`; a calendar that reports the set and ignores it is a matter for `.honoured`. + ## 1.7.0 - 2026-10-23 This release works with caldav 3.6.0. 1.6.0 was accidentally released without two probes and a grading fix that caldav 3.6.0's server hints were based on. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index c415cd7..77cbb02 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -185,12 +185,12 @@ library *declares* but nothing measures, and quirks that predate the feature database entirely and still live in the old flat flag list. At the baseline nine features were defined in `compatibility_hints.py` and probed -by nothing. Eight still are: `get-supported-components`, `save-load.reuse-deleted-uid`, +by nothing. Seven still are: `save-load.reuse-deleted-uid`, `save-load.todo.recurrences.thisandfuture`, `search.time-range.accurate`, `search.time-range.journal`, `search.text.category.substring`, `wrong-password-check` and `http.multiplexing`. Every one of them is currently reported from its default rather than from evidence - which is exactly the -failure mode constraint 2 is about, at eight known sites. The ninth, +failure mode constraint 2 is about, at seven known sites. The ninth, `create-calendar.with-supported-component-types`, got its probe on 2026-09-13, after Bedework 5 turned out to ignore the component set - the first server where the default was wrong. diff --git a/docs/TODO.md b/docs/TODO.md index 66b2477..da6793a 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -230,8 +230,9 @@ cases apart in the probe. (Clean-context review 2026-09-14, filed rather than fixed.) -`CheckSupportedComponentSet._delete_probe_calendar` deletes -`principal.calendar(cal_id=CAL_ID)`, although the check itself notes that the +`CheckSupportedComponentSet._delete_probe_calendars` deletes +`principal.calendar(cal_id=...)` for both its probe calendars (`CAL_ID` and +`CAL_ID_MULTIPLE`), although the check itself notes that the `cal_id` is not the collection's address on a server that relocates it. The probe calendar has no display name either, so a prefix sweep may miss it too. No server is known to leak one - OX, the relocating server, also answers at the diff --git a/pyproject.toml b/pyproject.toml index a06302f..34fc8f3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] keywords = ["caldav", "calendar", "testing", "compatibility", "server"] dependencies = [ - "caldav>=3.6.0", + "caldav>=3.7.0", "click", "pyyaml", ] diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index 3221486..90e65cd 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -8,6 +8,7 @@ from datetime import date, datetime, timedelta, timezone from typing import NamedTuple from urllib.parse import quote, unquote, urlsplit, urlunsplit +from xml.etree import ElementTree from xml.sax.saxutils import escape as xml_escape from zoneinfo import ZoneInfo @@ -1617,222 +1618,6 @@ def _fallback_displayname(reason): _cleanup() -class CheckSupportedComponentSet(Check): - """ - Checks whether a calendar created with a component-set restriction really - is restricted. - - RFC 4791 section 5.2.3 lets a client hand MKCALENDAR a - CALDAV:supported-calendar-component-set, and section 5.3.1 has a server - that cannot do what was asked answer 403 with a - CALDAV:supported-calendar-component-set precondition rather than create - something else. A server may legitimately not report the property at all - - 5.2.3 makes it optional, and an absent property means every component type - is welcome - so what the collection *advertises* cannot be the only - question asked. The restriction is therefore measured twice over, from a - calendar the probe asks to be VTODO-only: - - * advertised - does the created collection report the component set that - was requested? - * enforced - does it refuse a VEVENT, which the restriction excludes? - - Either one is enough for "full", which is what the feature's own - description says ("the server both advertises (or enforces) the - restriction"); the behaviour note says so when only one of them holds. - Neither is "unsupported": the restriction was silently ignored and - wrong-type objects can be saved to the calendar. A MKCALENDAR that is - refused only when it carries the component set is "ungraceful". - - Bedework 5 is the server this was written for: the component set of a - Bedework collection is a function of its internal calType, which nothing in - CalDAV can select, so the property is accepted with a "200 ok" propstat and - the collection is VEVENT-only regardless. - """ - - depends_on = {CheckMakeDeleteCalendar} - features_to_be_checked = {"create-calendar.with-supported-component-types"} - - FEATURE = "create-calendar.with-supported-component-types" - - ## The cal_id the probe uses. Prefixed so the cleanup sweep recognises it - ## (see checker.PROBE_CALENDAR_PREFIXES). - CAL_ID = "caldav-server-checker-compset-test" - - ## What the probe calendar is asked to hold, and the component type that - ## restriction excludes. VTODO rather than VJOURNAL because a server that - ## can only do one of them does VTODO (and save-load.todo is measured - ## anyway, so a failure here has a neighbour to be read against). - RESTRICTION = ["VTODO"] - WRONG_TYPE = "VEVENT" - - def _run_check(self) -> None: - ## No calendar can be created, so there is nothing to restrict: the - ## feature collapses under create-calendar's own verdict, the way the - ## set-displayname features do. takes_effect(), not is_supported(): - ## a creation that is merely fragile or rude still produces a calendar - ## to look at. - if not self.takes_effect("create-calendar"): - return - - ## A leftover from an interrupted run would answer the wrong question - - ## it may well be a calendar this probe created *unrestricted* as its - ## own control below. - self._delete_probe_calendar() - try: - self._probe() - finally: - self._delete_probe_calendar() - - def _probe(self) -> None: - cal, _attempts, error = self.make_calendar_with_retries( - self.CAL_ID, supported_calendar_component_set=self.RESTRICTION - ) - if cal is None: - self.set_feature(self.FEATURE, self._grade_refused_creation(error)) - return - - ## Some servers process MKCALENDAR asynchronously; poll on the calendar - ## object the library handed back rather than on the cal_id, which is - ## not the collection's address on a server that relocates it (see - ## create-calendar.stable-url). - cal, _waited = self._poll_calendar(cal=cal, timeout=self.checker.delay_probe_timeout) - if cal is None: - self.set_feature( - self.FEATURE, - { - "support": "unknown", - "behaviour": "the restricted calendar was accepted but never became queryable", - }, - ) - return - - advertised = self._advertised(cal) - enforced = self._enforced(cal) - self.set_feature(self.FEATURE, self._grade(advertised, enforced)) - - def _grade_refused_creation(self, error): - """The MKCALENDAR carrying the component set was refused - by this - server, or only because of the component set? - - The control is the same cal_id without the restriction: a server that - creates it then is refusing the component set specifically, which is - what "ungraceful" describes. One that refuses either way is refusing - to create calendars for a reason of its own, and create-calendar is the - feature that gets to say so - nothing is claimed here. - """ - control, _attempts, control_error = self.make_calendar_with_retries(self.CAL_ID) - if control is None: - return { - "support": "unknown", - "behaviour": f"cannot test, creating the probe calendar failed with and without the component set ({control_error})", - } - return { - "support": "ungraceful", - "behaviour": f"MKCALENDAR is refused when it carries a component set ({error})", - } - - def _advertised(self, cal): - """What the created collection reports, or ``None`` when it cannot be read. - - ``with_fallback=False``: the fallback answers with the RFC default set - for a server that reports nothing, which is the right answer for a - client and the wrong one for a probe - it would read as "advertises - everything" where nothing was advertised at all. - """ - try: - return cal.get_supported_components(with_fallback=False) - except (DAVError, AuthorizationError): - return None - - def _enforced(self, cal) -> bool | int | None: - """Does the collection refuse the component type the restriction excludes? - - The status, rather than a bool, where the save failed with a 5xx the - server software is to blame for: that is enforcement, ungracefully. - ``None`` where it failed without saying anything - a 502-504, or an - error carrying no status - since that says nothing about the - restriction either way. - """ - uid = f"csc_compset_{uuid.uuid4()}" - ## _base_year() rather than checker.fixture_base_year: that attribute is - ## set by PrepareCalendar, which this check does not depend on and may - ## well run after it. Same year either way - both read this function. - year = _base_year() - try: - obj = cal.save_event( - dtstart=datetime(year, 1, 8, 10, 0, tzinfo=utc), - dtend=datetime(year, 1, 8, 11, 0, tzinfo=utc), - summary="component set probe", - uid=uid, - ) - except AuthorizationError: - return True - except DAVError as exc: - status = dav_error_status(exc) - if status is not None and 400 <= status < 500: - return True - return status if is_server_failure(status) else None - ## It went in. The calendar is deleted at the end of the check either - ## way, but a server that cannot delete a calendar would keep the - ## object, so take it out again. - try: - obj.delete() - except (DAVError, AuthorizationError): - logging.warning("Could not delete the %s the component-set probe saved (uid %s)", self.WRONG_TYPE, uid) - return False - - def _grade(self, advertised, enforced): - """Grade the restriction from what the collection says and what it takes.""" - asked = f"asked for {self.RESTRICTION}" - if advertised is None: - advertises = "its component set cannot be read" - elif not advertised: - advertises = "it advertises no component set" - else: - advertises = f"it advertises {sorted(advertised)}" - ## A restriction is advertised when nothing outside it is: a server - ## reporting a subset of what was asked for restricts at least as much. - restricted = bool(advertised) and not set(advertised) - set(self.RESTRICTION) - - if enforced is not None and not isinstance(enforced, bool): - return { - "support": "ungraceful", - "behaviour": f"saving a {self.WRONG_TYPE} fails with {enforced} rather than a refusal; {advertises}", - } - if restricted and enforced: - return True - if restricted and enforced is None: - return { - "support": "full", - "behaviour": f"the restriction is advertised; saving a {self.WRONG_TYPE} failed without a refusal, so enforcement was not tested", - } - if restricted: - return { - "support": "full", - "behaviour": f"the restriction is advertised but not enforced; a {self.WRONG_TYPE} can be saved to the calendar", - } - if enforced is None: - return { - "support": "unknown", - "behaviour": f"cannot test, {advertises} and saving a {self.WRONG_TYPE} failed without a refusal", - } - if enforced: - return { - "support": "full", - "behaviour": f"the restriction is enforced but not advertised ({advertises})", - } - return { - "support": "unsupported", - "behaviour": f"the restriction is ignored: {asked}, {advertises}, and a {self.WRONG_TYPE} can be saved to the calendar", - } - - def _delete_probe_calendar(self) -> None: - try: - DAVObject.delete(self.checker.principal.calendar(cal_id=self.CAL_ID)) - except Exception: - pass - - class PrepareCalendar(Check): """ This "check" doesn't check anything, but ensures the calendar has some known events @@ -3383,6 +3168,424 @@ def add_if_not_existing(*largs, **kwargs): self._check_encode_at(calendar) +class CheckSupportedComponentSet(Check): + """ + Grades the supported-calendar-component-set property two ways, and the + MKCALENDAR that sets it two. + + RFC 4791 section 5.2.3 makes the property optional. Where a calendar has + it, storing a component type it does not list "MUST result in an error" + (the CALDAV:supported-calendar-component precondition of section 5.3.2.1); + where it has none, "the server MUST accept all component types". Section + 5.3.1.1 has a MKCALENDAR whose property cannot be set answer 207 with + 403/409 for that property and create nothing. So four questions, kept + apart because servers answer them independently: + + * ``supported-components.property-exists`` - does the *test* calendar + report the property? Absent is conformant, never a failure. + * ``supported-components.honoured`` - does a calendar take exactly the + types it lists, and every type where it lists none? Measured on a + calendar the probe has asked to be VTODO-only (a VEVENT, a VTODO and a + VJOURNAL saved into it), and, where the test calendar has no property, from + what PrepareCalendar found out about it. A calendar that reports + [VTODO] and still takes a VEVENT is "broken" (Bedework 3.10, DAViCal, + Stalwart); so is one that reports nothing and refuses a type, though + the restriction it was asked for is in force. + * ``create-calendar.with-supported-component-types`` - does MKCALENDAR + naming {VTODO} give a calendar with that set? Bedework 5 and SOGo + answer the request and then ignore it, which is "unsupported". + * ``...with-supported-component-types.multiple`` - the same naming + {VEVENT, VTODO}. CCS refuses it the section 5.3.1.1 way, which is + conformant but "unsupported"; Bedework 3.10 is said to report one + concatenated name, which is "broken". Independent of + ``save-load.todo.mixed-calendar``, which asks about the default + calendar. Measured whenever the {VTODO} calendar could be + created, even where the parent is graded negative. + + ``with_fallback=False`` when reading the property: the fallback answers + with the RFC default set for a server that reports nothing, which is the + right answer for a client and the wrong one for a probe. + + Like save.etag.matches-stored, the child features are conditions, not + modes, so "rewrites it or not" never became a feature of its own. + ``supported-components`` itself is a grouping node nothing may set, hence + both of its children are set explicitly on every path. + """ + + depends_on = {CheckMakeDeleteCalendar, PrepareCalendar} + FEATURE = "create-calendar.with-supported-component-types" + MULTIPLE = FEATURE + ".multiple" + EXISTS = "supported-components.property-exists" + HONOURED = "supported-components.honoured" + features_to_be_checked = {FEATURE, MULTIPLE, EXISTS, HONOURED} + + ## The cal_ids the probe uses. Prefixed so the cleanup sweep recognises + ## them (see checker.PROBE_CALENDAR_PREFIXES). + CAL_ID = "caldav-server-checker-compset-test" + CAL_ID_MULTIPLE = "caldav-server-checker-compset-multi-test" + + ## What the probe calendar is asked to hold, and the component type that + ## restriction excludes. VTODO rather than VJOURNAL because a server that + ## can only do one of them does VTODO (and save-load.todo is measured + ## anyway, so a failure here has a neighbour to be read against). + RESTRICTION = ["VTODO"] + WRONG_TYPE = "VEVENT" + SEVERAL = ["VEVENT", "VTODO"] + + ## A save-load.*.mixed-calendar verdict that says the test calendar + ## refused the type. + _REFUSED_BY_TEST_CALENDAR = ("unsupported", "ungraceful", "broken") + + def _run_check(self) -> None: + reported = self._reported(getattr(self.checker, "calendar", None)) + self._set_exists(reported) + + honoured = None + ## No calendar can be created, so there is nothing to restrict: the + ## creation features collapse under create-calendar's own verdict, the + ## way the set-displayname features do. takes_effect(), not + ## is_supported(): a creation that is merely fragile or rude still + ## produces a calendar to look at. + if self.takes_effect("create-calendar"): + ## A leftover from an interrupted run would answer the wrong + ## question - it may well be a calendar this probe created + ## *unrestricted* as its own control below. + self._delete_probe_calendars() + try: + honoured = self._probe() + finally: + self._delete_probe_calendars() + ## What PrepareCalendar found out about the test calendar is evidence + ## only where no probe calendar could be made to look at: its flag is + ## set by any failure of its first save, a gateway error included. + self._set_honoured(honoured, self._test_calendar_refuses(reported) if honoured is None else []) + + ## property-exists, on the test calendar + + def _reported(self, cal): + """The component set ``cal`` reports: a list, ``[]`` when the property + is absent, ``None`` when it could not be read.""" + if cal is None: + return None + try: + return list(cal.get_supported_components(with_fallback=False)) + except (DAVError, AuthorizationError) as exc: + ## The library raises a PropfindError carrying no status for an + ## absent property; a failed PROPFIND carries one. + return None if (dav_error_status(exc) or getattr(exc, "status", None)) else [] + + def _set_exists(self, reported) -> None: + if reported is None: + self.set_feature( + self.EXISTS, + {"support": "unknown", "behaviour": "the test calendar's component set could not be read"}, + ) + else: + self.set_feature(self.EXISTS, bool(reported)) + + ## honoured + + def _test_calendar_refuses(self, reported): + """The types PrepareCalendar found the test calendar refusing, where it + reports no property - and so has to take every type.""" + if reported is None or reported: + return [] + refused = [] + for component in ("todo", "journal"): + feature = f"save-load.{component}.mixed-calendar" + if self.checker.features_checked.is_supported(feature, str) in self._REFUSED_BY_TEST_CALENDAR: + refused.append(component.upper()) + return refused + + def _set_honoured(self, probed, refused_by_test_calendar) -> None: + """Set ``honoured`` from the probe calendar's verdict, or - where there + was none (``None``) - from the test calendar's, inferred.""" + if probed is None and refused_by_test_calendar: + probed = { + "support": "broken", + "behaviour": ( + "inferred from save-load.*.mixed-calendar: the test calendar reports no component set, " + f"which means every type is accepted, yet it refuses {', '.join(refused_by_test_calendar)}" + ), + } + if probed is None: + probed = { + "support": "unknown", + "behaviour": "cannot test, no calendar with a component set could be created", + } + self.set_feature(self.HONOURED, probed) + + def _grade_honoured(self, reported, stored): + """Does the probe calendar take exactly what it lists? + + ``stored`` maps a component type to what saving one did: ``"stored"``, + ``"refused"``, a 5xx status the server broke on, or ``None``. + """ + if reported is None: + return { + "support": "unknown", + "behaviour": "cannot test, the probe calendar's component set could not be read", + } + listed = set(reported) if reported else None + mismatches = [] + for component, outcome in stored.items(): + if outcome not in ("stored", "refused"): + continue + takes = listed is None or component in listed + if (outcome == "stored") == takes: + continue + if listed is None: + mismatches.append( + f"the calendar lists no component set, which means every type is accepted, " + f"yet a {component} was refused" + ) + elif outcome == "stored": + mismatches.append( + f"a {'-'.join(self.RESTRICTION)}-only calendar reports {sorted(listed)}, " + f"and a {component} can be saved to it" + ) + else: + mismatches.append(f"a {component} the calendar lists ({sorted(listed)}) was refused") + if mismatches: + return {"support": "broken", "behaviour": "; ".join(mismatches)} + failed = {c: o for c, o in stored.items() if isinstance(o, int) and not isinstance(o, bool)} + if failed: + what = ", ".join(f"{c} with {o}" for c, o in failed.items()) + return {"support": "ungraceful", "behaviour": f"saving fails rather than being refused: {what}"} + if any(outcome is None for outcome in stored.values()): + return {"support": "unknown", "behaviour": "saving failed without a refusal, so obedience was not tested"} + return True + + ## MKCALENDAR naming {VTODO}, and {VEVENT, VTODO} + + def _probe(self): + """The creation features are set here; returns the verdict on + ``honoured`` from the probe calendar, ``None`` where there was no + calendar to look at.""" + cal, _attempts, error = self.make_calendar_with_retries( + self.CAL_ID, supported_calendar_component_set=self.RESTRICTION + ) + if cal is None: + self.set_feature(self.FEATURE, self._grade_refused_creation(error)) + return None + + ## Some servers process MKCALENDAR asynchronously; poll on the calendar + ## object the library handed back rather than on the cal_id, which is + ## not the collection's address on a server that relocates it (see + ## create-calendar.stable-url). + cal, _waited = self._poll_calendar(cal=cal, timeout=self.checker.delay_probe_timeout) + if cal is None: + self.set_feature( + self.FEATURE, + { + "support": "unknown", + "behaviour": "the restricted calendar was accepted but never became queryable", + }, + ) + return None + + reported = self._reported(cal) + ## VJOURNAL as well: it is the type the most servers refuse outright, so + ## where the calendar reports no set (every type must then be taken) it + ## is the one that shows the property is missing for a reason. + stored = {component: self._store(cal, component) for component in ("VEVENT", "VTODO", "VJOURNAL")} + self.set_feature(self.FEATURE, self._grade_creation(reported, stored[self.WRONG_TYPE])) + ## Out of the way before the second: a provider with a calendar quota + ## would otherwise refuse that one for a reason of its own. + self._delete_probe_calendars() + self._probe_several() + return self._grade_honoured(reported, stored) + + def _grade_refused_creation(self, error): + """The MKCALENDAR carrying the component set was refused - by this + server, or only because of the component set? + + The control is the same cal_id without the restriction: a server that + creates it then is refusing the component set specifically, which is + what "ungraceful" describes. One that refuses either way is refusing + to create calendars for a reason of its own, and create-calendar is the + feature that gets to say so - nothing is claimed here. + """ + control, _attempts, control_error = self.make_calendar_with_retries(self.CAL_ID) + if control is None: + return { + "support": "unknown", + "behaviour": f"cannot test, creating the probe calendar failed with and without the component set ({control_error})", + } + return { + "support": "ungraceful", + "behaviour": f"MKCALENDAR is refused when it carries a component set ({error})", + } + + def _grade_creation(self, reported, wrong_type_outcome): + """Did MKCALENDAR naming {VTODO} give a VTODO-only calendar? + + What the calendar reports decides. Where it reports nothing (or could + not be read), whether the wrong type is refused does: the restriction + asked for is then in force, which is what a client needs to know, even + though an absent property is not what RFC 4791 section 5.2.3 has a + restricted calendar look like - that is ``honoured``'s business. + """ + asked = f"asked for {self.RESTRICTION}" + if reported: + ## VFREEBUSY is not a type anything is stored as - Zimbra lists it + ## next to VTODO - so it says nothing about the restriction. + if set(reported) - set(self.RESTRICTION) - {"VFREEBUSY"} or not set(self.RESTRICTION) <= set(reported): + return { + "support": "unsupported", + "behaviour": f"the component set is not honoured at creation: {asked}, the calendar reports {sorted(reported)}", + } + return True + advertises = "its component set cannot be read" if reported is None else "it reports no component set" + if wrong_type_outcome == "refused": + return { + "support": "full", + "behaviour": f"the restriction is in force but not reported ({advertises})", + } + if wrong_type_outcome == "stored": + return { + "support": "unsupported", + "behaviour": f"the restriction is ignored: {asked}, {advertises}, and a {self.WRONG_TYPE} can be saved to the calendar", + } + if wrong_type_outcome is None: + return { + "support": "unknown", + "behaviour": f"cannot test, {advertises} and saving a {self.WRONG_TYPE} failed without a refusal", + } + return { + "support": "ungraceful", + "behaviour": f"saving a {self.WRONG_TYPE} fails with {wrong_type_outcome} rather than a refusal; {advertises}", + } + + def _probe_several(self) -> None: + cal, _attempts, error = self.make_calendar_with_retries( + self.CAL_ID_MULTIPLE, supported_calendar_component_set=self.SEVERAL + ) + if cal is None: + self.set_feature(self.MULTIPLE, self._grade_refused_several(error)) + return + cal, _waited = self._poll_calendar(cal=cal, timeout=self.checker.delay_probe_timeout) + if cal is None: + self.set_feature( + self.MULTIPLE, + {"support": "unknown", "behaviour": "the calendar was accepted but never became queryable"}, + ) + return + reported = self._reported(cal) + stored = {component: self._store(cal, component) for component in self.SEVERAL} + self.set_feature(self.MULTIPLE, self._grade_several(reported, stored)) + + def _grade_refused_several(self, error): + """A MKCALENDAR naming several types was refused. 207 with 403/409 on + the property, nothing created, is what RFC 4791 section 5.3.1.1 has a + server do that cannot set it: conformant, but the feature is not + there. Anything else is a server failing on the request.""" + status = dav_error_status(error) or getattr(error, "status", None) + if status == 207: + refusal = self._component_set_refusal(getattr(error, "body", None)) + if refusal: + return { + "support": "unsupported", + "behaviour": ( + f"MKCALENDAR naming {self.SEVERAL} is refused as RFC 4791 section 5.3.1.1 has it: " + f"207, {refusal} on the property, no calendar created" + ), + } + return graded_refusal(error, f"MKCALENDAR naming {self.SEVERAL} fails with {{answer}}") + + @staticmethod + def _component_set_refusal(body): + """The 403 or 409 a 207 reply gives the supported-calendar-component-set + property, or ``None`` when the reply is unreadable or refuses something + else.""" + if not body: + return None + try: + root = ElementTree.fromstring(body) + except ElementTree.ParseError: + return None + for propstat in root.iter("{DAV:}propstat"): + props = [el.tag for el in propstat.iter() if el.tag.endswith("supported-calendar-component-set")] + status = propstat.findtext("{DAV:}status") or "" + refused = re.search(r"\s(403|409)\b", status) + if props and refused: + return refused.group(1) + return None + + def _grade_several(self, reported, stored): + if reported: + missing = [c for c in self.SEVERAL if c not in reported] + if missing: + return { + "support": "broken", + "behaviour": f"asked for {self.SEVERAL}, the calendar reports {sorted(reported)}", + } + refused = [c for c, o in stored.items() if o == "refused"] + if refused: + return { + "support": "broken", + "behaviour": f"the calendar was created, but a {' and a '.join(refused)} named in its set was refused", + } + failed = {c: o for c, o in stored.items() if isinstance(o, int) and not isinstance(o, bool)} + if failed: + what = ", ".join(f"{c} with {o}" for c, o in failed.items()) + return {"support": "ungraceful", "behaviour": f"saving a type named in the set fails: {what}"} + if any(outcome is None for outcome in stored.values()): + return {"support": "unknown", "behaviour": "saving failed without a refusal, so the set was not verified"} + return True + + ## Saving + + def _store(self, cal, component): + """What saving a ``component`` into ``cal`` did. + + ``"stored"`` (and the object is taken out again: the calendar is deleted + at the end of the check, but a server that cannot delete a calendar + would keep it), ``"refused"`` for an authorisation error or a 4xx, + the status for a 5xx the server software is to blame for, and ``None`` + where it failed without saying anything - a gateway, an error carrying + no status - since that says nothing about the restriction either way. + """ + uid = f"csc_compset_{uuid.uuid4()}" + ## _base_year() rather than checker.fixture_base_year: that attribute is + ## set by PrepareCalendar, but only for the checks that depend on it. + year = _base_year() + try: + if component == "VEVENT": + obj = cal.save_event( + dtstart=datetime(year, 1, 8, 10, 0, tzinfo=utc), + dtend=datetime(year, 1, 8, 11, 0, tzinfo=utc), + summary="component set probe", + uid=uid, + ) + elif component == "VTODO": + obj = cal.save_todo(summary="component set probe", uid=uid) + else: + obj = cal.save_journal(summary="component set probe", uid=uid) + except AuthorizationError: + return "refused" + except DAVError as exc: + status = dav_error_status(exc) + if status is not None and 400 <= status < 500: + return "refused" + return status if is_server_failure(status) else None + except Exception as exc: + logging.warning("The component-set probe could not save a %s (uid %s): %s", component, uid, exc) + return None + try: + obj.delete() + except (DAVError, AuthorizationError): + logging.warning("Could not delete the %s the component-set probe saved (uid %s)", component, uid) + return "stored" + + def _delete_probe_calendars(self) -> None: + for cal_id in (self.CAL_ID, self.CAL_ID_MULTIPLE): + try: + DAVObject.delete(self.checker.principal.calendar(cal_id=cal_id)) + except Exception: + pass + + class CheckTodoNoDtstartSearch(Check): """ Checks whether a VTODO without DTSTART (but with DUE) is returned by a diff --git a/tests/test_supported_component_set.py b/tests/test_supported_component_set.py index 7c7e3a7..37adfee 100644 --- a/tests/test_supported_component_set.py +++ b/tests/test_supported_component_set.py @@ -1,73 +1,147 @@ -"""A calendar created VTODO-only, and whether it really is one. - -RFC 4791 section 5.2.3 lets a client hand MKCALENDAR a -CALDAV:supported-calendar-component-set, and section 5.3.1 has the server -answer 403 (CALDAV:supported-calendar-component-set) if it cannot do what was -asked. Bedework 5 does neither: it answers the property "200 ok" and then -creates a VEVENT-only collection anyway, because a Bedework collection's -component set is a function of its internal calType, which CalDAV cannot set. -So the restriction has to be measured after the fact, from what the collection -advertises and from what it accepts. +"""The supported-calendar-component-set property, asked four questions. + +RFC 4791 section 5.2.3 makes the property optional; where a calendar has it, +storing a component type it does not list "MUST result in an error", and +where it has none, every type must be accepted. Section 5.3.1.1 has a +MKCALENDAR whose property cannot be set answer 207 with 403/409 for it and +create nothing. So: + +- ``supported-components.property-exists``: does the test calendar report it? +- ``supported-components.honoured``: does a calendar accept exactly what it + lists - everything where it lists nothing? +- ``create-calendar.with-supported-component-types``: does a MKCALENDAR asking + for {VTODO} give a calendar with that set? And ``.multiple``: one asking for + {VEVENT, VTODO}? + +Bedework 5 answers the property "200 ok" and creates a VEVENT-only collection +anyway; Bedework 3.10, DAViCal and Stalwart report the VTODO-only set and take +a VEVENT; CCS refuses a two-type MKCALENDAR the 5.3.1.1 way. """ from unittest.mock import Mock import pytest from caldav.compatibility_hints import FeatureSet -from caldav.lib.error import PutError +from caldav.lib.error import MkcalendarError, PropfindError, PutError import caldav_server_tester.checks as checks_mod from caldav_server_tester.checker import ServerQuirkChecker -from caldav_server_tester.checks import CheckMakeDeleteCalendar, CheckSupportedComponentSet +from caldav_server_tester.checks import CheckMakeDeleteCalendar, CheckSupportedComponentSet, PrepareCalendar -FEATURE = "create-calendar.with-supported-component-types" +PARENT = "create-calendar.with-supported-component-types" +MULTIPLE = "create-calendar.with-supported-component-types.multiple" +EXISTS = "supported-components.property-exists" +HONOURED = "supported-components.honoured" + +ALL = ["VEVENT", "VTODO", "VJOURNAL"] +MULTI_ERROR_BODY = ( + '' + "/cal/x/" + "HTTP/1.1 403 Forbidden" +) + + +def listing_as_asked(asked): + return list(asked) + + +def accepting_listed(asked, component): + return component in asked + + +def accepting_all(asked, component): + return True class ComponentSetServer: - """A server that answers a component-restricted MKCALENDAR in one of the - ways the four support levels describe. - - ``advertises`` is what the created collection reports for - supported-calendar-component-set (``[]`` for a server that does not report - the property at all), ``accepts_wrong_type`` whether a VEVENT may be saved - to the collection the probe asked to be VTODO-only, and ``refuses`` whether - the MKCALENDAR carrying the component set is refused outright. + """A server answering the probe's MKCALENDARs and saves. + + ``reports(asked)`` is what a calendar created for ``asked`` lists (``None``: + the property is absent, ``"fail"``: the PROPFIND fails), ``accepts(asked, + component)`` whether such a calendar stores ``component``. ``refuses`` is + ``"always"``, ``"with-component-set"``, or ``"multiple-207"`` / + ``"multiple-500"`` / ``"multiple-403"`` for a MKCALENDAR naming several + types. ``test_calendar_reports`` is what the test calendar lists. """ - def __init__(self, advertises=("VTODO",), accepts_wrong_type=False, refuses=None): - self.advertises = list(advertises) - self.accepts_wrong_type = accepts_wrong_type + def __init__( + self, + reports=listing_as_asked, + accepts=accepting_listed, + refuses=None, + test_calendar_reports=ALL, + raises_on=None, + property_raises=False, + multi_body=None, + ): + self.reports = reports + self.accepts = accepts self.refuses = refuses + self.test_calendar_reports = test_calendar_reports + self.raises_on = raises_on + self.property_raises = property_raises + self.multi_body = multi_body self.created = [] self.deleted = [] self.saved = [] + self.objects_deleted = [] def make_calendar(self, cal_id=None, supported_calendar_component_set=None, **kwargs): - if self.refuses == "always" or (self.refuses == "with-component-set" and supported_calendar_component_set): - raise checks_mod.DAVError("403 Forbidden") - self.created.append((cal_id, supported_calendar_component_set)) - return self.calendar(cal_id=cal_id) - - def calendar(self, cal_id=None, name=None): + asked = supported_calendar_component_set + if self.refuses == "always" or (self.refuses == "with-component-set" and asked): + raise MkcalendarError("403 Forbidden", status=403) + if asked and len(asked) > 1: + if self.refuses == "multiple-207": + raise MkcalendarError("207 Multi-Status", body=self.multi_body or MULTI_ERROR_BODY, status=207) + if self.refuses == "multiple-500": + raise MkcalendarError("500 Internal Server Error", status=500) + if self.refuses == "multiple-403": + raise MkcalendarError("403 Forbidden", status=403) + self.created.append((cal_id, asked)) + return self._calendar(cal_id, asked or ALL, self.reports(asked) if asked else ALL) + + def _calendar(self, cal_id, asked, reported): cal = Mock() cal.cal_id = cal_id cal.url = f"/dav/user/{cal_id}/" cal.events.return_value = [] - cal.get_supported_components.return_value = list(self.advertises) - cal.save_event.side_effect = self._save_event + + def get_supported_components(with_fallback=True): + if self.property_raises and cal_id == CheckSupportedComponentSet.CAL_ID: + raise ValueError("the fake blew up in a place the probe does not catch") + if reported == "fail": + raise PropfindError("500 Internal Server Error", status=500) + if reported is None: + raise PropfindError("supported-calendar-component-set not supported") + return list(reported) + + cal.get_supported_components.side_effect = get_supported_components + cal.save_event.side_effect = lambda **kw: self._save(asked, "VEVENT", kw) + cal.save_todo.side_effect = lambda **kw: self._save(asked, "VTODO", kw) + cal.save_journal.side_effect = lambda **kw: self._save(asked, "VJOURNAL", kw) return cal - def _save_event(self, *args, **kwargs): - if not self.accepts_wrong_type: + def _save(self, asked, component, kwargs): + if self.raises_on == component: + raise RuntimeError("the probe blew up") + if not self.accepts(asked, component): raise PutError("403 Forbidden") - self.saved.append(kwargs.get("uid")) - return Mock() + self.saved.append((asked, component)) + obj = Mock() + obj.delete.side_effect = lambda: self.objects_deleted.append(kwargs.get("uid")) + return obj + + def calendar(self, cal_id=None, name=None): + return self._calendar(cal_id, ALL, ALL) + + def test_calendar(self): + return self._calendar("caldav-server-checker-calendar", ALL, self.test_calendar_reports) def delete(self, cal): self.deleted.append(getattr(cal, "cal_id", None)) -def make_check(monkeypatch, server, create_calendar="full"): +def make_check(monkeypatch, server, create_calendar="full", mixed=None): monkeypatch.setattr(checks_mod.time, "sleep", lambda _s: None) client = Mock() client.features = FeatureSet() @@ -77,124 +151,394 @@ def make_check(monkeypatch, server, create_calendar="full"): checker.principal.calendar.side_effect = server.calendar checker.principal.make_calendar.side_effect = server.make_calendar checker.principal.calendars.return_value = [] + checker.calendar = server.test_calendar() checker.features_checked.set_feature("create-calendar", {"support": create_calendar}) - checker._checks_run.add(CheckMakeDeleteCalendar) + for feature, value in (mixed or {}).items(): + checker.features_checked.set_feature(feature, value) + checker._checks_run.update({CheckMakeDeleteCalendar, PrepareCalendar}) probe = CheckSupportedComponentSet(checker) probe.expected_features = client.features return probe -def observed(check, as_type=dict): - return check.checker.features_checked.is_supported(FEATURE, as_type) +def observed(check, feature, as_type=str): + return check.checker.features_checked.is_supported(feature, as_type) -def test_a_restriction_that_sticks_is_full(monkeypatch) -> None: - check = make_check(monkeypatch, ComponentSetServer(advertises=["VTODO"], accepts_wrong_type=False)) +def explicit(check, feature): + return feature in check.checker.features_checked._server_features - check._run_check() - assert observed(check, str) == "full" +def run(monkeypatch, server, **kwargs): + check = make_check(monkeypatch, server, **kwargs) + check.run_check() + return check -def test_a_restriction_that_is_ignored_is_unsupported(monkeypatch) -> None: - """The Bedework 5 shape: the collection comes back VEVENT-only and takes a - VEVENT quite happily.""" - server = ComponentSetServer(advertises=["VEVENT"], accepts_wrong_type=True) - check = make_check(monkeypatch, server) +## One fake server per column of the design's grade table - check._run_check() - assert observed(check, str) == "unsupported" - assert "VTODO" in observed(check)["behaviour"] - assert "VEVENT" in observed(check)["behaviour"] +def test_sabredav_shape_is_full_throughout(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer()) + assert observed(check, EXISTS) == "full" + assert observed(check, HONOURED) == "full" + assert observed(check, PARENT) == "full" + assert observed(check, MULTIPLE) == "full" -def test_a_restriction_that_is_advertised_but_not_enforced_is_full(monkeypatch) -> None: - """Advertising it is half the job and the feature says so ("advertises (or - enforces)"), but a wrong-type object getting in is worth writing down.""" - check = make_check(monkeypatch, ComponentSetServer(advertises=["VTODO"], accepts_wrong_type=True)) - check._run_check() +def test_ccs_shape_refuses_a_set_of_several(monkeypatch) -> None: + """CCS answers 207 with 403 for the property and creates nothing, as RFC + 4791 section 5.3.1.1 has a server do that cannot set it.""" + check = run(monkeypatch, ComponentSetServer(refuses="multiple-207")) - assert observed(check, str) == "full" - assert "not enforced" in observed(check)["behaviour"] + assert observed(check, PARENT) == "full" + assert observed(check, HONOURED) == "full" + assert observed(check, MULTIPLE) == "unsupported" + assert "403" in observed(check, MULTIPLE, dict)["behaviour"] -def test_a_restriction_that_is_enforced_but_not_advertised_is_full(monkeypatch) -> None: - """No property to read back - RFC 4791 section 5.2.3 makes it optional - - but the collection refuses the wrong type, so the restriction is real.""" - check = make_check(monkeypatch, ComponentSetServer(advertises=[], accepts_wrong_type=False)) +def test_advertised_but_not_enforced_is_disobedience_not_a_creation_failure(monkeypatch) -> None: + """Bedework 3.10, DAViCal, Stalwart: the calendar lists ['VTODO'], as asked, + and takes a VEVENT anyway.""" + check = run(monkeypatch, ComponentSetServer(accepts=accepting_all)) - check._run_check() + assert observed(check, PARENT) == "full" + assert observed(check, HONOURED) == "broken" + assert "VEVENT" in observed(check, HONOURED, dict)["behaviour"] + assert observed(check, EXISTS) == "full" + assert observed(check, MULTIPLE) == "full" - assert observed(check, str) == "full" - assert "not advertised" in observed(check)["behaviour"] +def test_a_set_of_several_reported_as_one_name_is_broken(monkeypatch) -> None: + """Bedework 3.10 is said to report a VEVENT+VTODO calendar as 'VEVENTVTODO'.""" + server = ComponentSetServer( + reports=lambda asked: ["".join(asked)] if len(asked) > 1 else list(asked), + accepts=accepting_all, + ) + check = run(monkeypatch, server) -def test_a_creation_refusing_the_component_set_is_ungraceful(monkeypatch) -> None: - check = make_check(monkeypatch, ComponentSetServer(refuses="with-component-set")) + assert observed(check, MULTIPLE) == "broken" + assert "VEVENTVTODO" in observed(check, MULTIPLE, dict)["behaviour"] - check._run_check() - assert observed(check, str) == "ungraceful" +def test_bedework_5_shape(monkeypatch) -> None: + """Every calendar is VEVENT-only, whatever was asked for, and says so.""" + server = ComponentSetServer( + reports=lambda asked: ["VEVENT"], accepts=lambda asked, component: component == "VEVENT" + ) + check = run(monkeypatch, server) + assert observed(check, PARENT) == "unsupported" + assert observed(check, HONOURED) == "full" + ## measured, not inherited from the negative parent + assert explicit(check, MULTIPLE) + assert observed(check, MULTIPLE) == "broken" -def test_a_creation_failing_either_way_says_nothing(monkeypatch) -> None: - """Not evidence about the component set: this server is refusing to create - calendars at all, which is create-calendar's verdict to give.""" - check = make_check(monkeypatch, ComponentSetServer(refuses="always")) - check._run_check() +def test_sogo_shape(monkeypatch) -> None: + """The set is ignored: VEVENT, VFREEBUSY and VTODO are listed and taken.""" + server = ComponentSetServer( + reports=lambda asked: ["VEVENT", "VFREEBUSY", "VTODO"], + accepts=lambda asked, component: component in ("VEVENT", "VTODO"), + ) + check = run(monkeypatch, server) - assert observed(check, str) == "unknown" + assert observed(check, PARENT) == "unsupported" + assert observed(check, HONOURED) == "full" + assert explicit(check, MULTIPLE) + assert observed(check, MULTIPLE) == "full" -def test_it_collapses_under_an_unsupported_create_calendar(monkeypatch) -> None: - server = ComponentSetServer() - check = make_check(monkeypatch, server, create_calendar="unsupported") +def test_a_set_lacking_the_requested_type_is_not_honoured_at_creation(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(reports=lambda asked: ["VFREEBUSY"])) + + assert observed(check, PARENT) == "unsupported" + + +def test_zimbra_shape(monkeypatch) -> None: + """Zimbra adds VFREEBUSY to the set it reports - which nothing is ever stored + as - so a VTODO calendar is a VTODO calendar; a mixed one is not, since + tasks live on task lists of their own.""" + server = ComponentSetServer( + reports=lambda asked: ["VFREEBUSY", "VTODO"] if asked == ["VTODO"] else ["VEVENT", "VFREEBUSY"], + accepts=lambda asked, component: component in asked or (component == "VEVENT" and len(asked) > 1), + ) + check = run(monkeypatch, server) + + assert observed(check, PARENT) == "full" + assert observed(check, HONOURED) == "full" + assert observed(check, MULTIPLE) == "broken" + assert "VFREEBUSY" in observed(check, MULTIPLE, dict)["behaviour"] + + +def test_robur_shape(monkeypatch) -> None: + """No property, so every type must be accepted - yet a VJOURNAL is refused, + on the probe calendar itself and not just on a dedicated one.""" + server = ComponentSetServer( + reports=lambda asked: None, + accepts=lambda asked, component: component in ("VEVENT", "VTODO"), + test_calendar_reports=None, + ) + check = run(monkeypatch, server) + + assert observed(check, EXISTS) == "unsupported" + assert observed(check, HONOURED) == "broken" + assert "VJOURNAL" in observed(check, HONOURED, dict)["behaviour"] + + +def test_a_listed_set_without_the_journal_refuses_it(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer()) + + assert observed(check, HONOURED) == "full" + + +def test_an_ignored_and_unreported_set_is_unsupported(monkeypatch) -> None: + """Nothing reported, and the VEVENT goes in: an absent property means every + type is welcome, so obedience holds, but the calendar is not VTODO-only.""" + server = ComponentSetServer(reports=lambda asked: None, accepts=accepting_all) + check = run(monkeypatch, server) + + assert observed(check, PARENT) == "unsupported" + assert observed(check, HONOURED) == "full" + + +def test_enforced_but_not_advertised(monkeypatch) -> None: + """The VEVENT is refused, though the calendar lists no set: the creation did + what was asked, but a calendar without the property must take every type.""" + check = run(monkeypatch, ComponentSetServer(reports=lambda asked: None)) + + assert observed(check, PARENT) == "full" + assert observed(check, HONOURED) == "broken" + assert "no component set" in observed(check, HONOURED, dict)["behaviour"] + + +def test_another_set_reported_and_the_wrong_type_refused(monkeypatch) -> None: + """Undecided case in the design: another set is a creation failure, and + refusing a type the property lists is disobedience.""" + server = ComponentSetServer(reports=lambda asked: ["VEVENT", "VTODO"]) + check = run(monkeypatch, server) + + assert observed(check, PARENT) == "unsupported" + assert observed(check, HONOURED) == "broken" + + +## The test calendar + + +def test_property_exists_follows_the_test_calendar(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(test_calendar_reports=None)) + + assert observed(check, EXISTS) == "unsupported" + + +def test_a_failing_propfind_on_the_test_calendar_is_unknown(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(test_calendar_reports="fail")) + + assert observed(check, EXISTS) == "unknown" - check._run_check() - assert FEATURE not in check.checker.features_checked._server_features +@pytest.mark.parametrize("mixed", ["save-load.todo.mixed-calendar", "save-load.journal.mixed-calendar"]) +def test_a_refused_type_on_a_test_calendar_without_the_property_is_disobedience(monkeypatch, mixed) -> None: + """No property, yet no VJOURNAL either. Where no calendar can be made to + look at, what PrepareCalendar found out is all there is, and the verdict + says it is inferred.""" + check = run( + monkeypatch, ComponentSetServer(test_calendar_reports=None), create_calendar="unsupported", mixed={mixed: False} + ) + + assert observed(check, EXISTS) == "unsupported" + assert observed(check, HONOURED) == "broken" + assert "save-load" in observed(check, HONOURED, dict)["behaviour"] + + +def test_a_flag_on_the_test_calendar_does_not_override_a_probe_calendar(monkeypatch) -> None: + """PrepareCalendar sets mixed-calendar False after any failure of its first + save, a gateway error included. With a probe calendar that took every type + there is better evidence.""" + server = ComponentSetServer(reports=lambda asked: None, accepts=accepting_all, test_calendar_reports=None) + check = run(monkeypatch, server, mixed={"save-load.journal.mixed-calendar": False}) + + assert observed(check, HONOURED) == "full" + + +def test_a_refused_type_the_test_calendar_does_not_list_is_obedience(monkeypatch) -> None: + server = ComponentSetServer(test_calendar_reports=["VEVENT", "VTODO"]) + check = run(monkeypatch, server, mixed={"save-load.journal.mixed-calendar": False}) + + assert observed(check, HONOURED) == "full" + + +def test_journals_refused_everywhere_count_as_refused(monkeypatch) -> None: + """save-load.journal negative: the mixed-calendar child inherits it.""" + server = ComponentSetServer(test_calendar_reports=None) + check = run(monkeypatch, server, create_calendar="unsupported", mixed={"save-load.journal": "ungraceful"}) + + assert observed(check, HONOURED) == "broken" + + +def test_an_unreadable_probe_calendar_property_is_not_reported_as_not_created(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(reports=lambda asked: "fail")) + + assert observed(check, HONOURED) == "unknown" + assert "could not be read" in observed(check, HONOURED, dict)["behaviour"] + + +## Without MKCALENDAR + + +def test_without_mkcalendar_obedience_is_unknown(monkeypatch) -> None: + server = ComponentSetServer() + check = run(monkeypatch, server, create_calendar="unsupported") + + assert observed(check, EXISTS) == "full" + assert observed(check, HONOURED) == "unknown" + assert not explicit(check, PARENT) + assert not explicit(check, MULTIPLE) + assert observed(check, MULTIPLE) == "unsupported" assert server.created == [] -@pytest.mark.parametrize("advertises,accepts", [(["VTODO"], False), (["VEVENT"], True), ([], False)]) -def test_the_probe_calendar_is_deleted_again(monkeypatch, advertises, accepts) -> None: - server = ComponentSetServer(advertises=advertises, accepts_wrong_type=accepts) - check = make_check(monkeypatch, server) +def test_without_mkcalendar_a_refusing_test_calendar_is_still_disobedience(monkeypatch) -> None: + server = ComponentSetServer(test_calendar_reports=None) + check = run(monkeypatch, server, create_calendar="unsupported", mixed={"save-load.journal.mixed-calendar": False}) + + assert observed(check, HONOURED) == "broken" + + +## Refused or failing creation - check._run_check() - assert CheckSupportedComponentSet.CAL_ID in server.deleted +def test_a_207_refusing_another_property_is_not_the_conformant_refusal(monkeypatch) -> None: + body = MULTI_ERROR_BODY.replace("", "") + check = run(monkeypatch, ComponentSetServer(refuses="multiple-207", multi_body=body)) + + assert observed(check, MULTIPLE) == "ungraceful" + + +def test_a_creation_refusing_the_component_set_is_ungraceful(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(refuses="with-component-set")) + + assert observed(check, PARENT) == "ungraceful" + ## not probed: inherits the parent + assert not explicit(check, MULTIPLE) + assert observed(check, MULTIPLE) == "ungraceful" + assert observed(check, HONOURED) == "unknown" + + +def test_a_creation_failing_either_way_says_nothing(monkeypatch) -> None: + check = run(monkeypatch, ComponentSetServer(refuses="always")) + + assert observed(check, PARENT) == "unknown" + assert not explicit(check, MULTIPLE) + + +@pytest.mark.parametrize("refuses", ["multiple-500", "multiple-403"]) +def test_a_set_of_several_refused_otherwise_is_ungraceful(monkeypatch, refuses) -> None: + check = run(monkeypatch, ComponentSetServer(refuses=refuses)) + + assert observed(check, PARENT) == "full" + assert observed(check, MULTIPLE) == "ungraceful" class FailingComponentSetServer(ComponentSetServer): - """Fails the wrong-type save with ``status`` rather than a refusal.""" + """Fails the VEVENT save into the VTODO-only calendar with ``status``.""" def __init__(self, status, **kwargs): super().__init__(**kwargs) self.status = status - def _save_event(self, *args, **kwargs): - raise PutError(f"{self.status} Server Error") + def _save(self, asked, component, kwargs): + if component == "VEVENT" and asked == ["VTODO"]: + raise PutError(f"{self.status} Server Error") + return super()._save(asked, component, kwargs) + + +def test_a_500_on_the_wrong_type_is_ungraceful_obedience(monkeypatch) -> None: + """A 500 is the server failing on the wrong type: enforcement, ungracefully. + The listed set still answers the creation question.""" + check = run(monkeypatch, FailingComponentSetServer(500)) + + assert observed(check, PARENT) == "full" + assert observed(check, HONOURED) == "ungraceful" + assert "500" in observed(check, HONOURED, dict)["behaviour"] + +def test_a_500_on_the_wrong_type_with_nothing_reported_is_ungraceful(monkeypatch) -> None: + check = run(monkeypatch, FailingComponentSetServer(500, reports=lambda asked: None)) -@pytest.mark.parametrize("advertises", [[], ["VTODO"]], ids=["unadvertised", "advertised"]) -def test_a_500_on_the_wrong_type_is_ungraceful(monkeypatch, advertises) -> None: - """A 500 is the server failing on the wrong type: enforcement, ungracefully.""" - check = make_check(monkeypatch, FailingComponentSetServer(500, advertises=advertises)) + assert observed(check, PARENT) == "ungraceful" + assert "500" in observed(check, PARENT, dict)["behaviour"] - check._run_check() - assert observed(check, str) == "ungraceful" - assert "500" in observed(check, dict)["behaviour"] +def test_a_gateway_status_on_the_wrong_type_is_not_enforcement(monkeypatch) -> None: + check = run(monkeypatch, FailingComponentSetServer(503, reports=lambda asked: None, accepts=accepting_all)) + assert observed(check, PARENT) == "unknown" + assert observed(check, HONOURED) == "unknown" -def test_a_gateway_or_overload_status_on_the_wrong_type_is_not_enforcement(monkeypatch) -> None: - """A 503 says the server had a bad moment, not that it enforced anything.""" - check = make_check(monkeypatch, FailingComponentSetServer(503, advertises=[])) - check._run_check() +## Clean-up - assert observed(check, str) == "unknown" + +@pytest.mark.parametrize( + "server", + [ + ComponentSetServer(), + ComponentSetServer(accepts=accepting_all), + ComponentSetServer(refuses="multiple-207"), + ComponentSetServer(raises_on="VTODO"), + ComponentSetServer(property_raises=True), + ], + ids=["sabredav", "bedework3", "ccs", "save-raises", "probe-raises"], +) +def test_both_probe_calendars_are_deleted_before_and_after(monkeypatch, server) -> None: + check = run(monkeypatch, server) + + for cal_id in (CheckSupportedComponentSet.CAL_ID, CheckSupportedComponentSet.CAL_ID_MULTIPLE): + assert server.deleted.count(cal_id) >= 2, cal_id + assert cal_id.startswith("caldav-server-checker") + assert explicit(check, EXISTS) + assert explicit(check, HONOURED) + + +def test_the_first_probe_calendar_is_gone_before_the_second_is_made(monkeypatch) -> None: + """A provider with a calendar quota would refuse the second otherwise.""" + server = ComponentSetServer() + order = [] + original_make, original_delete = server.make_calendar, server.delete + server.make_calendar = lambda cal_id=None, **kw: (order.append(("make", cal_id)), original_make(cal_id, **kw))[1] + server.delete = lambda cal: (order.append(("delete", getattr(cal, "cal_id", None))), original_delete(cal))[1] + run(monkeypatch, server) + + first, second = CheckSupportedComponentSet.CAL_ID, CheckSupportedComponentSet.CAL_ID_MULTIPLE + assert order.index(("delete", first), order.index(("make", first))) < order.index(("make", second)) + + +def test_saved_objects_are_deleted_again(monkeypatch) -> None: + server = ComponentSetServer(accepts=accepting_all) + run(monkeypatch, server) + + assert len(server.objects_deleted) == len(server.saved) == 5 + + +@pytest.mark.parametrize( + ("create_calendar", "server"), + [ + ("full", ComponentSetServer()), + ("unsupported", ComponentSetServer()), + ("full", ComponentSetServer(refuses="always")), + ("full", ComponentSetServer(refuses="with-component-set")), + ("full", ComponentSetServer(raises_on="VEVENT")), + ("full", ComponentSetServer(property_raises=True)), + ("full", ComponentSetServer(test_calendar_reports="fail")), + ], + ids=["full", "no-mkcalendar", "no-creation", "ungraceful", "raises", "probe-raises", "propfind-fails"], +) +def test_run_check_finds_every_declared_feature(monkeypatch, create_calendar, server) -> None: + """run_check asserts every declared feature was set or has a set ancestor, + and the grouping node supported-components never is.""" + check = run(monkeypatch, server, create_calendar=create_calendar) + + assert explicit(check, EXISTS) + assert explicit(check, HONOURED) From 5c86f2445837e853fa0a14ab6b3583316e468450 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Wed, 28 Oct 2026 11:52:50 +0100 Subject: [PATCH 11/12] feat: grade bare UID-conflict refusals as quirks save.no-uid-conflict is graded by a plain PUT, and a refusal that is not the RFC's 403/409 with the precondition is a 'quirk' with the keys 'status' and 'precondition' - without the precondition only if nothing was stored and another UID goes through. New children: .if-none-match (Bedework's bare 412) and .href-given. Prompt: Please consider the `save.no-uid-conflict` "feature". The last two commits were done by Sonnet, but I think they should be rolled back or amended. I've written some thoughts in [a notes file], please read through. If it makes sense, then see if you can do a better job on implementing it. If something is unclear or not well enough thought through, get back to me. Prompt: (the notes: a bare refusal is a 'quirk' with keys for status and XML element, after the tester checks that GET gives 404 and the same PUT with another UID succeeds; a href-given child) Prompts (comments made inline in the document while reviewing it): > Fair point - leave `.no-uid-conflict` ungraceful for servers reporting > 403 with no XML error. (...) > I think this is a feature (it obeys "SHOULD" in the RFC) and not a > behaviour. > I agree with most of the proposals/recommendations in this document, > so go ahead and implement. Assisted-By: Claude Opus 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 6 + src/caldav_server_tester/checks.py | 118 ++++++++++++++----- tests/test_no_uid_conflict.py | 182 +++++++++++++++++++++++++---- 3 files changed, 259 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26fc7ae..a9b1c86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ This library is tightly dependent on the CalDAV-library, particularly the `compa Needs caldav 3.7.0, which has the features below in its `compatibility_hints.py`. +### Changed + +- **`save.no-uid-conflict`** is graded by a plain PUT. 403 or 409 with `CALDAV:no-uid-conflict` (or Cyrus' `CALDAV:unique-scheduling-object-resource`) is `full`; another status with it (Xandikos, Stalwart: 412) is `quirk`. A refusal without it is `quirk` too, with the extra keys `status` and `precondition`, if nothing was stored at the URL and the same PUT with another UID goes through (SOGo: 409). A bare 403 stays `ungraceful`. + ### Added - **`supported-components.property-exists`** (the former `get-supported-components`, now probed), **`supported-components.honoured`** and **`create-calendar.with-supported-component-types.multiple`** - whether a calendar reports the component set, whether it takes exactly the types it lists (and every type where it lists none, RFC 4791 §5.2.3), and whether a MKCALENDAR naming several types gives a calendar taking all of them. A calendar that reports `[VTODO]` and still takes a VEVENT is `broken` on `.honoured` (Bedework 3.10, DAViCal, Stalwart); CCS refuses a set of several components the §5.3.1.1 way and is `unsupported` on `.multiple`. @@ -26,6 +30,8 @@ Needs caldav 3.7.0, which has the features below in its `compatibility_hints.py` - **`sync-token.initial-sync`**, **`sync-token.delete.created-and-deleted`** and **`sync-token.recreated`** - which members a sync-collection report lists: an initial sync with deleted members or the collection itself in it, a member created and deleted between two reports left out, and a re-created member reported twice, or only as removed (Nextcloud, Synology). - **`multiget.missing-href`** - whether a calendar-multiget answers a missing href with a 404 entry; Baïkal, Nextcloud, DAViCal, Davis and Infomaniak leave it out, Posteo fails the whole multiget with a 404, and an entry claiming the object exists is broken. - **`multiget.limit`**, **`propfind.limit`** and **`search.limit`** - whether a calendar-multiget, a Depth: 1 PROPFIND or a calendar-query answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. +- **`save.no-uid-conflict.if-none-match`** - the same, for a PUT carrying `If-None-Match: *`, as `save(no_overwrite=True)` sends it. Bedework answers it with a bare 412, but a plain PUT with 403 and the precondition. +- **`save.no-uid-conflict.href-given`** - whether the precondition names the object holding the UID, as RFC 4791 section 5.3.2.1 says it SHOULD. The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index 90e65cd..b3e03d0 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -25,6 +25,7 @@ NotFoundError, PutError, ReportError, + UidConflictError, parse_preconditions, ) from caldav.lib.http_sync import CaseInsensitiveDict @@ -4430,6 +4431,7 @@ class _StatusOnly(NamedTuple): status: int preconditions: tuple = () + body: bytes | None = None class _Unauthorized(int): @@ -4502,7 +4504,11 @@ def _answer(request, *args): return request(*args) except AuthorizationError as exc: ## 401/403 are raised before a response exists, but they are answers. - return _StatusOnly(dav_error_status(exc) or _Unauthorized(), tuple(getattr(exc, "preconditions", ()))) + return _StatusOnly( + dav_error_status(exc) or _Unauthorized(), + tuple(getattr(exc, "preconditions", ())), + getattr(exc, "body", None), + ) except (DAVError, OSError): ## Rate limited for longer than the client waits, or no answer at all. return None @@ -4790,27 +4796,38 @@ class CheckNoUidConflict(_RawPut, Check): the calendar already holds is refused. RFC 4791 section 5.3.2.1 requires it, with the CALDAV:no-uid-conflict - precondition. Radicale answers 409, Xandikos and Stalwart 412, Cyrus 403 - with RFC 6638's CALDAV:unique-scheduling-object-resource instead. Where - it holds, save(no_overwrite=True) leaves out the UID search before its - PUT: If-None-Match: * guards the URL, and this the UID under any other. - The second PUT carries If-None-Match: *, as the library's does where - save.if-none-match holds. A - refusal without a precondition the library knows (SOGo's bare 409, - Bedework's bare 412, Purelymail's no-uid-conflict in the CardDAV - namespace) keeps the duplicate out, but the library cannot tell it from - any other failed PUT, so it is ungraceful: there the search, and its - ConsistencyError, stays. + precondition (Cyrus names RFC 6638's CALDAV:unique-scheduling-object-resource + instead); 403 or 409 with it, the statuses RFC 4918 section 16 gives a + failed precondition, is 'full'. Xandikos and Stalwart answer 412 + with it: a 'quirk', since the library goes by the body. + + A refusal without the precondition is a 'quirk' only where it is + confirmed to be about the UID: nothing was stored at the URL, and the + same PUT with another UID goes through. The library does the first of + those checks itself before it raises UidConflictError. A bare 403 is + 'ungraceful' all the same - a read-only calendar answers it too - as is + any other bare status outside 4xx, and any refusal failing either check + (one of them getting no answer leaves it 'unknown'). The extra keys 'status' + and 'precondition' say what a 'quirk' is. + + The parent is graded by a plain PUT, save.no-uid-conflict.if-none-match + by one carrying If-None-Match: *, as save(no_overwrite=True) sends it + where save.if-none-match holds (Bedework answers that with a bare 412, + a plain PUT with 403 and the precondition). + save.no-uid-conflict.href-given says whether the precondition names the + object holding the UID. """ depends_on = {PrepareCalendar, CheckPutEtag} LABEL = "no-uid-conflict" FEATURE = "save.no-uid-conflict" - features_to_be_checked = {FEATURE} + IF_NONE_MATCH = "save.no-uid-conflict.if-none-match" + HREF = "save.no-uid-conflict.href-given" + features_to_be_checked = {FEATURE, IF_NONE_MATCH, HREF} UID = "csc_no_uid_conflict" NAMES = ("csc_no_uid_conflict_1", "csc_no_uid_conflict_2") - ## RFC 4791 section 5.3.2.1 names 403 and 409; Xandikos and Stalwart use 412. - REFUSALS = frozenset({403, 409, 412}) + ## The statuses RFC 4918 section 16 gives a failed precondition + RFC_REFUSALS = frozenset({403, 409}) def _run_check(self) -> None: first, second = map(self._url, self.NAMES) @@ -4818,31 +4835,78 @@ def _run_check(self) -> None: self._delete(second) try: if not self._ok(self._put(first, self._ical(self.UID))): - self.set_feature(self.FEATURE, None) + for feature in (self.FEATURE, self.IF_NONE_MATCH, self.HREF): + self.set_feature(feature, None) return - ## With If-None-Match: *, as save(no_overwrite=True) sends it - ## (Bedework then refuses with a bare 412) - but only where the - ## server handles it, or a 412 may be about the header. - inm = self.checker._features_checked.is_supported("save.if-none-match", str) in ("full", "quirk") - second_put = self._put(second, self._ical(self.UID), if_none_match="*" if inm else None) - self.set_feature(self.FEATURE, self._grade(second_put)) + hrefs = [] + self.set_feature(self.FEATURE, self._probe(second, None, hrefs)) + ## Only where the server handles If-None-Match: *, or a 412 may be about the + ## header. Elsewhere the library never sends it, and the child is left to + ## say what the parent says. + if self.checker._features_checked.is_supported("save.if-none-match", str) in ("full", "quirk"): + self.set_feature(self.IF_NONE_MATCH, self._probe(second, "*", hrefs)) + ## Explicitly unknown without a precondition: the parent's grade says nothing about it + self.set_feature(self.HREF, ("full" if all(hrefs) else "unsupported") if hrefs else None) finally: for url in (first, second): self._delete(url) - def _grade(self, response): + def _probe(self, url, if_none_match, hrefs): + """The grade of a PUT of a second object with the UID to ``url``. + Whether a refusal with the precondition names the holder is + appended to ``hrefs``.""" + try: + response = self._put(url, self._ical(self.UID), if_none_match=if_none_match) + return self._grade(response, url, if_none_match, hrefs) + finally: + self._delete(url) + + def _grade(self, response, url, if_none_match, hrefs): status = self._status(response) if status is None: return self._graded("unknown", "cannot test, a second object with the same UID got no answer") if 200 <= status < 300: return self._graded("unsupported", f"a second object with the same UID was stored ({status})") - if status in self.REFUSALS: - if UID_CONFLICT_PRECONDITIONS.intersection(self._preconditions(response)): + if UID_CONFLICT_PRECONDITIONS.intersection(self._preconditions(response)): + hrefs.append(UidConflictError(body=self._body(response)).href is not None) + if status in self.RFC_REFUSALS: return "full" + return self._quirk(status, True, f"refused with {status} and the precondition") + if status == 403: + return self._graded("ungraceful", f"refused with {status}, without the precondition: may be anything") + if not 400 <= status < 500: + return self._graded("ungraceful", f"a second object with the same UID was answered with {status}") + stored = self._get_status(url) + if stored is None: + return self._graded("unknown", f"refused with a bare {status}, and a GET of the URL got no answer") + if stored != 404: + return self._graded("ungraceful", f"refused with a bare {status}, yet a GET of the URL answered {stored}") + other = self._put(url, self._ical(self.UID + "-other"), if_none_match=if_none_match) + if self._status(other) is None: + return self._graded( + "unknown", f"refused with a bare {status}, and the same PUT with another UID got no answer" + ) + if not self._ok(other): return self._graded( - "ungraceful", f"refused with {status}, but without the CALDAV:no-uid-conflict precondition" + "ungraceful", + f"refused with a bare {status}, and so was the same PUT with another UID ({self._status(other)})", ) - return self._graded("ungraceful", f"a second object with the same UID was answered with {status}") + return self._quirk(status, False, f"refused with a bare {status}") + + @staticmethod + def _quirk(status, precondition, behaviour): + return {"support": "quirk", "status": int(status), "precondition": precondition, "behaviour": behaviour} + + def _get_status(self, url): + """The status a GET of ``url`` answers with, None for no answer.""" + try: + return self._status(self.client.request(url)) + except (DAVError, OSError): + return None + + @staticmethod + def _body(response): + return response.body if isinstance(response, _StatusOnly) else getattr(response, "raw", None) class CheckDeleteIfMatch(_RawPut, Check): diff --git a/tests/test_no_uid_conflict.py b/tests/test_no_uid_conflict.py index 31ce645..95a4504 100644 --- a/tests/test_no_uid_conflict.py +++ b/tests/test_no_uid_conflict.py @@ -2,7 +2,8 @@ RFC 4791 section 5.3.2.1 has the server refuse it with the CALDAV:no-uid-conflict precondition. Radicale answers 409, Xandikos and Stalwart 412, Cyrus 403 (which -the client raises rather than returns). +the client raises rather than returns). Bedework answers a plain PUT with 403 +and the precondition, one carrying If-None-Match: * with a bare 412. """ from unittest.mock import Mock @@ -30,27 +31,55 @@ class UidServer: ``create_status`` is the answer to the first object. """ - def __init__(self, conflict=409, body=CONFLICT, create_status=201): + def __init__( + self, + conflict=409, + body=CONFLICT, + create_status=201, + inm_conflict=None, + inm_body=b"", + stored_anyway=False, + get_status=None, + ): self.conflict = conflict self.body = body self.create_status = create_status + ## The answer to a PUT carrying If-None-Match: *, where it differs + self.inm_conflict = inm_conflict + self.inm_body = inm_body + ## Refuses the duplicate, yet stores it + self.stored_anyway = stored_anyway + ## Answers every GET with this, where given + self.get_status = get_status self.objects = {} self.deleted = [] self.headers = [] def put(self, url, body, headers=None): - self.headers.append(headers or {}) + headers = headers or {} + self.headers.append(headers) uid = next(line[4:] for line in body.split("\r\n") if line.startswith("UID:")) if any(u == uid and other != url for other, u in self.objects.items()): - if isinstance(self.conflict, Exception): - raise self.conflict - if self.conflict is not None: - return self._response(self.conflict, self.body) + conflict, body = self.conflict, self.body + if self.inm_conflict is not None and "If-None-Match" in headers: + conflict, body = self.inm_conflict, self.inm_body + if isinstance(conflict, Exception): + raise conflict + if conflict is not None: + if self.stored_anyway: + self.objects[url] = uid + return self._response(conflict, body) elif self.create_status >= 300: return self._response(self.create_status, b"") self.objects[url] = uid return self._response(201, b"") + def request(self, url, method="GET", *args, **kwargs): + assert method == "GET" + if self.get_status is not None: + return self._response(self.get_status, b"") + return self._response(200 if url in self.objects else 404, b"") + @staticmethod def _response(status, raw): response = Mock() @@ -65,7 +94,7 @@ def delete(self, url): return Mock(status=204) -def _run(server, features=None): +def _run(server, features=None, feature="save.no-uid-conflict"): checker = Mock() checker._features_checked = FeatureSet() if features: @@ -75,16 +104,22 @@ def _run(server, features=None): checker.fixture_base_year = 2027 checker.calendar.url.join.side_effect = lambda name: BASE + name CheckNoUidConflict(checker)._run_check() - return checker._features_checked.is_supported("save.no-uid-conflict", return_type=dict) + return checker._features_checked.is_supported(feature, return_type=dict) -@pytest.mark.parametrize("status", [403, 409, 412]) +@pytest.mark.parametrize("status", [403, 409]) def test_refused_with_the_precondition_is_full(status): server = UidServer(conflict=status) assert _run(server) == {"support": "full"} assert server.objects == {} +def test_412_with_the_precondition_is_a_quirk(): + """Xandikos, Stalwart: recognisable by the body, but the RFC status is 403 or 409.""" + verdict = _run(UidServer(conflict=412)) + assert (verdict["support"], verdict["status"], verdict["precondition"]) == ("quirk", 412, True) + + def test_forbidden_raised_by_the_client_is_full(): """Cyrus: the client raises 403 before a response exists.""" exc = AuthorizationError( @@ -103,12 +138,42 @@ def test_cyrus_scheduling_precondition_is_full(): assert _run(UidServer(conflict=exc)) == {"support": "full"} -def test_refused_without_the_precondition_is_ungraceful(): - """SOGo: the library cannot tell it from any other failed PUT.""" - verdict = _run(UidServer(conflict=409, body=b"")) +def test_bare_refusal_is_a_quirk(): + """SOGo: nothing stored at the URL, and the same PUT with another UID goes through.""" + server = UidServer(conflict=409, body=b"") + verdict = _run(server) + assert (verdict["support"], verdict["status"], verdict["precondition"]) == ("quirk", 409, False) + assert server.objects == {} + + +def test_bare_refusal_storing_the_object_anyway_is_ungraceful(): + verdict = _run(UidServer(conflict=409, body=b"", stored_anyway=True)) assert verdict["support"] == "ungraceful" - assert "no-uid-conflict" in verdict["behaviour"] - assert "409" in verdict["behaviour"] + assert "GET" in verdict["behaviour"] + + +def test_bare_refusal_of_any_uid_is_ungraceful(): + """The same PUT with another UID is refused too: not about the UID.""" + server = UidServer(conflict=409, body=b"", create_status=201) + put = server.put + + def put_refusing_other_uids(url, body, headers=None): + if "-other" in body: + server.headers.append(headers or {}) + return server._response(409, b"") + return put(url, body, headers) + + server.put = put_refusing_other_uids + verdict = _run(server) + assert verdict["support"] == "ungraceful" + assert "another UID" in verdict["behaviour"] + + +def test_bare_403_is_ungraceful(): + """A read-only calendar answers 403 as well: it may be anything.""" + exc = AuthorizationError(url="403 Forbidden", reason="Forbidden") + server = UidServer(conflict=exc) + assert _run(server)["support"] == "ungraceful" def test_precondition_in_the_wrong_namespace_is_ungraceful(): @@ -119,19 +184,54 @@ def test_precondition_in_the_wrong_namespace_is_ungraceful(): assert _run(UidServer(conflict=exc))["support"] == "ungraceful" -def test_second_put_carries_if_none_match(): - """As save(no_overwrite=True) does: Bedework drops the precondition then.""" +def test_plain_put_grades_the_parent(): server = UidServer() _run(server) - assert server.headers[1].get("If-None-Match") == "*" + assert "If-None-Match" not in server.headers[1] + assert any(h.get("If-None-Match") == "*" for h in server.headers[2:]) + + +def test_if_none_match_graded_apart(): + """Bedework: 403 with the precondition, but a bare 412 with If-None-Match: *.""" + exc = AuthorizationError( + url="403 Forbidden", + reason="Forbidden", + preconditions=["{urn:ietf:params:xml:ns:caldav}no-uid-conflict"], + ) + server = UidServer(conflict=exc, inm_conflict=412) + assert _run(server) == {"support": "full"} + verdict = _run(server, feature="save.no-uid-conflict.if-none-match") + assert (verdict["support"], verdict["status"], verdict["precondition"]) == ("quirk", 412, False) + + +def test_if_none_match_same_answer(): + verdict = _run(UidServer(), feature="save.no-uid-conflict.if-none-match") + assert verdict == {"support": "full"} @pytest.mark.parametrize("support", ["broken", "unsupported", "unknown"]) def test_no_if_none_match_where_the_server_mishandles_it(support): """A 412 there could be about the header, not the UID.""" server = UidServer() - _run(server, {"save.if-none-match": {"support": support}}) - assert "If-None-Match" not in server.headers[1] + verdict = _run(server, {"save.if-none-match": {"support": support}}, feature="save.no-uid-conflict.if-none-match") + assert all("If-None-Match" not in h for h in server.headers) + ## Left unset, so it is what the parent says + assert verdict == {"support": "full"} + + +def test_href_given(): + assert _run(UidServer(), feature="save.no-uid-conflict.href-given") == {"support": "full"} + + +def test_href_not_given(): + body = CONFLICT.replace(b"/cal/csc_no_uid_conflict_1.ics", b"") + verdict = _run(UidServer(body=body), feature="save.no-uid-conflict.href-given") + assert verdict["support"] == "unsupported" + + +def test_href_not_graded_without_the_precondition(): + verdict = _run(UidServer(body=b""), feature="save.no-uid-conflict.href-given") + assert verdict["support"] == "unknown" def test_leftover_second_object_deleted_before_the_first_put(): @@ -183,3 +283,45 @@ def test_no_answer_is_not_graded(answer): def test_first_object_that_cannot_be_created_grades_nothing(): server = UidServer(create_status=500) assert _run(server)["support"] == "unknown" + + +def test_no_answer_to_the_get_is_not_graded(): + server = UidServer(conflict=409, body=b"", get_status=503) + assert _run(server)["support"] == "unknown" + + +def test_no_answer_to_the_other_uid_is_not_graded(): + server = UidServer(conflict=409, body=b"") + put = server.put + + def put_overloaded_for_other_uids(url, body, headers=None): + if "-other" in body: + server.headers.append(headers or {}) + return server._response(503, b"") + return put(url, body, headers) + + server.put = put_overloaded_for_other_uids + assert _run(server)["support"] == "unknown" + + +def test_href_in_a_raised_403(): + """Cyrus: the 403 is raised by the client, the href is in its body.""" + exc = AuthorizationError( + url="403 Forbidden", + reason="Forbidden", + preconditions=["{urn:ietf:params:xml:ns:caldav}no-uid-conflict"], + body=CONFLICT, + ) + assert _run(UidServer(conflict=exc), feature="save.no-uid-conflict.href-given") == {"support": "full"} + + +def test_if_none_match_probe_cleans_up(): + """Bedework-like: the INM probe's PUT with another UID is deleted too.""" + exc = AuthorizationError( + url="403 Forbidden", + reason="Forbidden", + preconditions=["{urn:ietf:params:xml:ns:caldav}no-uid-conflict"], + ) + server = UidServer(conflict=exc, inm_conflict=412) + _run(server) + assert server.objects == {} From 11bf74d32c905c52dd17692ec10aea478d7af8c7 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Tue, 27 Oct 2026 18:06:33 +0100 Subject: [PATCH 12/12] feat: probe search.text.uid Check whether a calendar-query with a UID filter answers the one object with that UID. iCloud fails it with 412 (ungraceful); a server ignoring the filter returns everything (unsupported); an empty answer, or the wrong object, is broken. Needs the feature from the matching caldav change, so the tests skip without it. See https://github.com/python-caldav/caldav/issues/783 Prompt: (paraphrased) Add the feature search.text.uid to compatibility_hints.py and create checks for it. Assisted-By: Claude Sonnet 5.5 Reviewed-by: Tobias Brox --- CHANGELOG.md | 1 + src/caldav_server_tester/checks.py | 29 ++++++++++++ tests/test_search_text_uid.py | 75 ++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 tests/test_search_text_uid.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a9b1c86..2cd15fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Needs caldav 3.7.0, which has the features below in its `compatibility_hints.py` - **`multiget.limit`**, **`propfind.limit`** and **`search.limit`** - whether a calendar-multiget, a Depth: 1 PROPFIND or a calendar-query answers every member, however many. Stalwart stops at its `maxResults` with a 507. These store as many objects as `CALDAV_SERVER_TESTER_LIMIT_OBJECTS` says, and are not probed without it. - **`save.no-uid-conflict.if-none-match`** - the same, for a PUT carrying `If-None-Match: *`, as `save(no_overwrite=True)` sends it. Bedework answers it with a bare 412, but a plain PUT with 403 and the precondition. - **`save.no-uid-conflict.href-given`** - whether the precondition names the object holding the UID, as RFC 4791 section 5.3.2.1 says it SHOULD. +- **`search.text.uid`** - whether a calendar-query with a UID filter answers the one object with that UID. iCloud answers it with 412 (`ungraceful`), so `get_object_by_uid()` and what is built on it fail there. See https://github.com/python-caldav/caldav/issues/783. The behaviours came from calendaring-sync's server compatibility notes: https://github.com/PyCalendar/calendaring-sync/blob/c40782bc7afe7b350fe63bf27ef13374c280ea8b/docs/reference/server-compatibility.rst diff --git a/src/caldav_server_tester/checks.py b/src/caldav_server_tester/checks.py index b3e03d0..3dad96a 100644 --- a/src/caldav_server_tester/checks.py +++ b/src/caldav_server_tester/checks.py @@ -5778,6 +5778,7 @@ class CheckSearch(Check): "search.time-range.event", "search.time-range.event.old-dates", "search.text.category", + "search.text.uid", "search.time-range.todo", "search.time-range.todo.old-dates", "search.comp-type", @@ -5788,6 +5789,33 @@ class CheckSearch(Check): "search.unlimited-time-range", } ## TODO: we can do so much better than this + def _probe_text_uid(self): + """search.text.uid: does a calendar-query with a UID filter answer + the one object with that UID? + + iCloud answers any UID filter with 412 (caldav issue 783); a server + ignoring the filter returns the whole calendar. Uses the fixture + event csc_event_with_categories, which no other UID contains. + """ + uid = "csc_event_with_categories" + try: + events = self.checker.calendar.search(uid=uid, event=True, post_filter=False) + except (ReportError, AuthorizationError, DAVError): + self.set_feature("search.text.uid", "ungraceful") + return + if len(events) == 1 and str(events[0].icalendar_component.get("UID")) == uid: + self.set_feature("search.text.uid", True) + elif len(events) > 1: + self.set_feature( + "search.text.uid", + {"support": "unsupported", "behaviour": "objects not matching the UID come back"}, + ) + else: + self.set_feature( + "search.text.uid", + {"support": "broken", "behaviour": "the object is not found by its UID"}, + ) + def _check_time_range_with_recent_data(self, cal, tasklist): """Test time-range searches using temporary near-future objects. @@ -6125,6 +6153,7 @@ def _found(objs, uid): self.set_feature("search.text.category", len(events) == 1) except (ReportError, AuthorizationError, DAVError): self.set_feature("search.text.category", "ungraceful") + self._probe_text_uid() ## search.combined - category "hands" AND a time-range. Uses the ## in-window "hands" fixture (csc_event_with_categories, -01-07), so ## it works regardless of old-date support; only category search is needed. diff --git a/tests/test_search_text_uid.py b/tests/test_search_text_uid.py new file mode 100644 index 0000000..90b1ddd --- /dev/null +++ b/tests/test_search_text_uid.py @@ -0,0 +1,75 @@ +"""Unit tests for CheckSearch._probe_text_uid (search.text.uid). + +A calendar-query with a UID filter works if it answers the one object +with that UID. iCloud answers it with 412. +https://github.com/python-caldav/caldav/issues/783 +""" + +from unittest.mock import Mock + +import pytest +from caldav.compatibility_hints import FeatureSet +from caldav.lib.error import AuthorizationError, DAVError, ReportError + +from caldav_server_tester.checker import ServerQuirkChecker +from caldav_server_tester.checks import CheckSearch + +pytestmark = pytest.mark.skipif( + "search.text.uid" not in FeatureSet.FEATURES, + reason="needs a caldav with the search.text.uid feature", +) + + +UID = "csc_event_with_categories" + + +def _event(uid: str = UID) -> Mock: + event = Mock() + event.icalendar_component = {"UID": uid} + return event + + +def _probe(**search_result) -> tuple[CheckSearch, Mock]: + cal = Mock() + cal.search.configure_mock(**search_result) + client = Mock() + client.features = FeatureSet() + checker = ServerQuirkChecker(client, debug_mode=None) + checker.calendar = cal + check = CheckSearch(checker) + check._probe_text_uid() + return check, cal + + +def _support(check: CheckSearch) -> str: + return check.checker.features_checked.is_supported("search.text.uid", str) + + +def test_full_when_one_object_is_returned() -> None: + check, cal = _probe(return_value=[_event()]) + assert _support(check) == "full" + assert cal.search.call_args.kwargs["uid"] == "csc_event_with_categories" + assert cal.search.call_args.kwargs["post_filter"] is False + + +def test_unsupported_when_the_filter_is_ignored() -> None: + check, _ = _probe(return_value=[_event(), _event("other-1"), _event("other-2")]) + assert _support(check) == "unsupported" + + +def test_broken_when_another_object_is_returned() -> None: + check, _ = _probe(return_value=[_event("some-other-uid")]) + assert _support(check) == "broken" + + +def test_broken_when_the_object_is_not_found() -> None: + check, _ = _probe(return_value=[]) + assert _support(check) == "broken" + + +@pytest.mark.parametrize( + "exc", [ReportError("412"), AuthorizationError("403"), DAVError("500")], ids=["412", "403", "500"] +) +def test_ungraceful_when_the_search_fails(exc: Exception) -> None: + check, _ = _probe(side_effect=exc) + assert _support(check) == "ungraceful"