Robur compatibility, async propfind, and url.encode-at by tobixen · Pull Request #705 · python-caldav/caldav

tobixen · 2026-08-27T11:20:33Z

We should get version 3.3 released, but first the tests should pass on
all the external servers I have available, and also fixing quality
issues found on the way. This pull request is part of the preparations for releasing 3.3

Robur

compatibility_hints.py claimed Robur did not support calendar
deletion. It does.

As soon as the flag was switched in the compatibility matrix, other
test breakages surfaced. Claude wrote up some kilobytes of details,
to be short most of it is due to the Robur-quirk to return 403 instead
of 404 on non-existing resources.

`AsyncDAVClient.propfind(props=...)`

The sync version accepts a string for the props parameter, string will be sent directly as the body in the HTTP-request. However, this is legacy code for backward compatibility and should not be needed for the async version. However, some internal code was sending a string. The async version of propfind now throws a TypeError instead of behaving in an undefined way when receiving a string.

`load(multiget_fallback=False)`

To allow the caldav-server-checker to probe properly for the Roburs behaviour of returning 403, it is needed to disable the multiget fallback logic. New parameter added for enabling/disabling it.

`url.encode-at`

This is unrelated to robur, but probably surfaced during some code review. Three "features" added - url.encode-at.identity (default unsupported - if supported, URLs containing %40 points to a different resource than URLs containing @. Claude came up with some text snippets in the RFCs defining this to be the correct behaviour - though none of the servers probed works like that), url.encode-at.literal (the server accepts URLs with @ in them) and url.encode-at.encoded (the server accepts %40). The default behaviour is to encode unless .identity is set (this for backward compatibility reasons). In practice the logic (should) work exactly as before, as no servers have .identity set.

Two things discovered while probing:

Stalwart canonicalises an object PUT to a literal @ path and serves it
back only under %40.
Nextcloud does not — probed with an account actually named
at@e.email, both spellings resolve at the collection and object level and
the href comes back with a literal @ either way. Recorded as a comment on
the profile rather than acted on; the workaround stays on for backward
compatibility.

Also in that commit: the ownCloud @-quoting heuristic existed in three copies
that had drifted, and only one of them skipped a URL already containing %40,
so a part-encoded home-set went through quote() twice and came back with
%2540. All three are now one helper, with the double-encoding fixed for the
rest of the path too.

Note: testCheckCompatibility needs a caldav-server-tester checkout developed in
step with this branch; the matching probe changes are not released yet.

Five findings from the github-code-quality bot on #705 - none of them user-visible, hence chore and no changelog entry: * `URL.objectify` is a classmethod, so its first parameter is now named `cls`. It was named `self` since 2017 and is unused in the body; the line was touched by c53366c, which is why the bot noticed it now. * Dropped two imports left over from earlier drafts: `at_spelling_to_mint` in collection.py (the module reaches the same answer through `self._at_spelling`) and `to_wire` in tests/test_async_davclient.py. * `_delete_used_calendar()` had a bare `except error.NotFoundError: pass` followed by `except self._notFound(collection=True)`. On a compliant server `_notFound()` *is* NotFoundError, so the second clause was dead; on Robur it widens to DAVError. The two are merged into one clause with an `isinstance` check, which keeps the behaviour (NotFoundError tolerated silently, anything wider logged) and leaves no empty except. Prompt: PR #705 has code quality comments, please look into it Co-authored-by: Claude Opus 5 (claude-opus-5) <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

AsyncDAVClient.propfind() declared props as list[str] and handed it straight to _build_propfind_body(). If sending a string, it would be iterated character by character, causing potentially buggy behaviour. The sync client accepts a body in props for backward compatibility. We doh not need to support legacy usage patterns in the new async code. The correct thing to do is to detect a string and raise TypeError. prompt: running `pytest -k robur` in the ~/caldav-with-passwords directory gives those errors: (...) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com> AI Prompts: claude-opus-5: . unknown: Can we search a bit through the history of the last few claude sessions for this project? I'm pretty sure that at some point synology support in caldav/compatibility_hints.py was rewritten to just inheritate ... baikal? ... plus the extra information that calendar deletion does not work on synology. I wonder what happened with that commit. It's not very important, if it's difficult then skip it. claude-opus-5: so the commit was basically lost during some rebase operation probably? Please investigate if there are more lost commits. The current branch is the relevant one

testCheckCompatibility reported four mismatches against calendar.robur.coop. The server checker observes "full" for delete-calendar, delete-calendar.free-namespace, non-existing-raises-not-found and search.comp-type.optional. The profile declared it as unsupported/ungraceful. It has been verified that calendar deletion works now. Possibly the reason why this has been marked as unsupported is that Robur yields 403 instead of 404 when accessing the calendar that does not exist. prompt: running `pytest -k robur` in the ~/caldav-with-passwords directory gives those errors: (...) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

Section 4.5 of the feature-complete roadmap has been updated with the design choices documented in docs/design/RETRY_AND_RESILIENCE_DESIGN.md Estimate drops from 16-24h to 8-12h, because the design lands most of the retry logic in urllib3's Retry rather than in caldav. The overall total is adjusted by the same delta; note that it did not match the sum of the table rows before this change either. Also drops a stale note under 1.4 (uncommitted working-tree change). Related: #695 Related: #647 Related: #620 Related: #648 Prompt: dosc/design/FEATURE_COMPLETE_ROADMAP.md [sic] - 4.5 Transport Robustness: Retries and Rate Limiting - it probably needs updating, there is a new document docs/design/RETRY_AND_RESILCENCE_DESIGN.md [sic] with the details. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

Robur answers 403 instead of 404 on attempts on accessing a calendar that does not exist (probably to prevent data leakages). This caused the test cleanup to fail if the calendar was already deleted. The breakage was observed now after the compatibility matrix declares Robur to support calendar deletion; the tests was skipped prior to this. prompt: run the pytest for robur again, from the ~/caldav-with-passwords directory Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

test_principal_make_calendar created and deleted "pythoncaldav-async-test", the same cal_id the async_calendar fixture hands to other tests. This may occasionally have caused some race conditions for Robur - or perhaps I managed to run two tests in parallell by accident. More research should probably have been made on this, but having a separate cal_id here probably doesn't hurt (as long as we're sure it's rigged down properly in the cleanup). prompt: run the pytest for robur again, from the ~/caldav-with-passwords directory Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

There has been some confusion on weather Robur gives 403 only on non-existing collections (calendars), non-existing calendar objects (like events) or both. Now the caldav-server-tester is probing both. They are siblings, `non-existing-raises-not-found.object' and `non-existing-raises-not-found.collection', under a grouping node that carries no default of its own. Neither answer can be derived from the other - a server is free to answer one way for a missing object and another for a missing collection, and the library reaches the two by different code paths - so neither may be the other's parent. Declaring the grouping node still reaches both through the ancestor walk, which is a claim to have observed both. Robur gives 403 on both. However, it's still possible to get out a 404 embedded in a `207 multistatus` when doing a multiget. `CalendarObjectResource.load()` happens to fall back to doing a multiget after a 403. prompt: do some research on 1bc9d904 and 46a2e9da. Robur compatibility test can be run in the ~/caldav-with-passwords directory. The caldav server tester is at ~/caldav-server-tester [both SHAs predate a rebase and no longer resolve: 46a2e9da is the Robur profile refresh earlier in this series, 1bc9d904 an intermediate "fragile" declaration squashed into this commit] followup-prompt: Try to fix issues found both in ~/caldav and ~/caldav-server-tester, but be aware that other agents are currently working on the code [= the findings of that research: the misdiagnosed Robur profile fixed here, and a flaky delete-calendar.free-namespace probe fixed in caldav-server-tester] followup-prompt: do we have a pull request yet? Please /review-and-push and create a pull request. [the review-and-push skill's step 4 spawned a clean-context review of the branch; three of its findings - the too-broad tolerated exception in _delete_used_calendar(), the .collection description, and the CHANGELOG section the feature was filed under - are folded in here rather than kept as a separate commit] followup-prompt: rethink the parent/child/sibling-question [a comment left in tmp-push-review-gate.md against this commit] Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

This is the fourth attempt on getting this right, with the human review being one of the trickest barrieres to pass. It's "fix" because the library should work more correctly when this compatibility flag is correctly set Three different axis has been identified: Axis A, reachability: which spellings can work at all. This is what the URL-writing code needs, because it picks what goes on the wire. Axis B, identity: RFC3986 section 2.2 (as ready by Claude - but it's hard to read it in another way) states that an URL with @ and %40 should be considered to be two different URLs. Axis B matters because this client rewrites spellings. The moment something rewrites '@' to '%40' - as the ownCloud/Nextcloud calendar-home-set heuristic does - "distinct" stops being harmless and becomes silent data loss: you write one resource and read another. That is the reason the feature exists at all. Axis C is weather the href returned from the server matches what we sent in. This is recorded in *.stable-url, and hence considered irrelevant for url.encode-at After some rounds it was decided to store the axis information as subfeatures, one per thing a probe can observe on its own: * 'url.encode-at.literal' - a literal '@' resolves | axis A * 'url.encode-at.encoded' - '%40' resolves | axis A * 'url.encode-at.identity' - the two are two resources | axis B The 3.x default for '.identity' is the *non*-conformant one, and that is deliberate. Treating the two spellings as one URL is what this library has always done, so defaulting to conformance would change URL identity under every user of a server nobody has probed - a breaking change smuggled in as a default. It is also the accurate reading: of the twelve test servers, every one that resolved both spellings served them as one. A conformant server has to say so out loud. That default is the whole switch, and it is what keeps this from being a breaking change. Where the two spellings name one resource the spelling carries no information, so every path is normalised exactly the way this library has always normalised it: an '@' the client mints is still sent as '%40', an incoming href is still decoded, and the ownCloud home-set workaround from 2021 still fires. Nothing moves for a server nobody has configured this for. Declaring '.identity: full' is what makes the spelling part of the resource name, and only then does the client stop rewriting it - everywhere at once, rather than in some places and not others, which is how the two halves of this library came to disagree in the first place (an href decoded on the way in, a home-set encoded on the way out). On such a server the home-set workaround switches off too, unless '.literal' says the literal spelling is the one that server will not serve - which is the case it was written for. '.encoded: unsupported' is the only thing that changes which spelling gets minted, and no server tested needs it. Probed 2026-08-26 against all twelve test servers, and `pytest -k testCheckCompatibility` is green for every one of them. Two findings worth keeping: * Stalwart canonicalises an object PUT to a literal '@' path and serves it back only under '%40' - the one server that needs the client to encode, and it is declared. * Nextcloud does not. Probed with a user actually named "at@e.email" (`occ user:add`), both spellings resolve at the collection and the object level, and the server reports the href with a literal '@' either way. Prompt: read the handover document in tmp-todo-urlencode.md and work with that. Make sure `pytest -k compat` works in the ~/caldav directory. [the handover note is caldav-server-tester/tmp-todo-urlencode.md, committed there in 71dbcbc] Followup-Prompt: (answer to a clarifying question) Actually deliver it [the option chosen after the clean-context review reported that declaring at-identity 'distinct' protected nobody; the option read "Teach _normalize_href to keep the raw href and route URL.canonical/__eq__/_post_multiget/_post_request_report_build_resultlist through the feature. Large blast radius - it changes URL identity across the whole library"] Followup-Prompt: I don't like this. Why not have a url.encode-at.identity (default: "full support", meaning the server follows the RFC and handles it as distinct paths), an an url.encode-at.literal indicating that the server accepts '@' in the URL, and an url.encode-at.encoded indicating that the server accepts '%40' in the URL? The client should NOT rewrite anything unless needed. This should be fixed both in caldav and caldav-server-tester, and the compat test should pass on all the docker-test-servers. The changes I've done below can be discarded. The rest of the document is not read. I don't think I changed any commit messages above. (sic) [left as a comment in tmp-push-review-gate.md against the previous shape of this commit] Followup-Prompt: Ok, so this is actually a breaking change. That's a bit dangerous. In the 3.x-series I think we need this explicitly flagged in the config. It's a bit silly, but the default setting seems to be "unsupported" for the RFC-conforming feature in 3.x at least Followup-Prompt: The breaking change is actually in /not using %40 by default/. The code need to encode those by default to preserve backward-compatibility. Please check that it's like that - otherwise NextCloud-users without a specific nextcloud-instruction in the config will hit regression-problems. [a comment in tmp-push-review-gate.md against this commit] Followup-Prompt: It seems to be a mess ... at some points it's promised that the "spelling" we get from the server should be kept (which is not always true - but we figured this should be recorded in *.stable-url - or maybe it shouldn't, the *.stable-url was created for cases when the URL changes completely, not just some "spelling"), then at one point it suddenly says "it should always be preserved, except in the home set for NextCloud". Please work more on this. [ditto] Followup-Prompt: I don't understand this. The promise (preserve existing "spelling") seems not to match what the thing is actually doing (for me it looks like @ should be replaced with %40?)? Why can't we just return url? Why do we need this at all? [ditto, against the _quote_url_path hunk in the diff] Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

CalendarObjectResource.load() retries a failed GET as a calendar-multiget REPORT against the parent collection. That rescue is what makes servers that do not serve objects over GET usable at all, but it may also hide server compatibility problems. For the caldav-server-tester we do NOT want the caldav library to do any workarounds, we want to see the behaviour as it comes from the server. The new parameter can be set to False to disable the rescue. Default True. Prompt: I hope the work tasks and my wish to reword the git commit messages survived the /compact run. Please deal with it. [the work tasks are the review notes left in tmp-push-review-gate.md, quoted below] Followup-Prompt: This needs more review and work. 1) load() needs a parameter on weather (sic) to fall back to multiget or not (default=True) 2) rethink the child/parent-relationship 3) on the object level, the feature should be "quirk" for robur, with a behaviour-note saying that the 404 can be found through multiget. It's needed to amend caldav-server-tester to detect this. 4) "load()'s blanket `except Exception: return self.load_by_multiget()` converts a 403 into a 404 for every server" - this must be wrong? A _real_ 403 will still be a 403 when using multistatus? [the review notes on a61f4528 in tmp-push-review-gate.md] Followup-Prompt: Drop the comments [= the delete-calendar comment block in the Robur profile] Followup-Prompt: add to the comment "possibly because the feature has been split [= the search.comp-type.optional comment in the Robur profile] Followup-Prompt: this may need redoing, ref my comments above [= the non-existing-raises-not-found comment block in the Robur profile] Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

Five findings from the github-code-quality bot on #705 - none of them user-visible, hence chore and no changelog entry: * `URL.objectify` is a classmethod, so its first parameter is now named `cls`. It was named `self` since 2017 and is unused in the body; the line was touched by c53366c, which is why the bot noticed it now. * Dropped two imports left over from earlier drafts: `at_spelling_to_mint` in collection.py (the module reaches the same answer through `self._at_spelling`) and `to_wire` in tests/test_async_davclient.py. * `_delete_used_calendar()` had a bare `except error.NotFoundError: pass` followed by `except self._notFound(collection=True)`. On a compliant server `_notFound()` *is* NotFoundError, so the second clause was dead; on Robur it widens to DAVError. The two are merged into one clause with an `isinstance` check, which keeps the behaviour (NotFoundError tolerated silently, anything wider logged) and leaves no empty except. Prompt: PR #705 has code quality comments, please look into it Co-authored-by: Claude Opus 5 (claude-opus-5) <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

search.comp-type.optional was mis-reported from earlier versions of caldav-server-tester Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

Five findings from the github-code-quality bot on #705 - none of them user-visible, hence chore and no changelog entry: * `URL.objectify` is a classmethod, so its first parameter is now named `cls`. It was named `self` since 2017 and is unused in the body; the line was touched by c53366c, which is why the bot noticed it now. * Dropped two imports left over from earlier drafts: `at_spelling_to_mint` in collection.py (the module reaches the same answer through `self._at_spelling`) and `to_wire` in tests/test_async_davclient.py. * `_delete_used_calendar()` had a bare `except error.NotFoundError: pass` followed by `except self._notFound(collection=True)`. On a compliant server `_notFound()` *is* NotFoundError, so the second clause was dead; on Robur it widens to DAVError. The two are merged into one clause with an `isinstance` check, which keeps the behaviour (NotFoundError tolerated silently, anything wider logged) and leaves no empty except. Prompt: PR #705 has code quality comments, please look into it Co-authored-by: Claude Opus 5 (claude-opus-5) <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

github-code-quality Bot found potential problems Aug 27, 2026

View reviewed changes

Comment thread tests/test_caldav.py Fixed

Comment thread caldav/lib/url.py Fixed

Comment thread caldav/collection.py Fixed

Comment thread tests/test_async_davclient.py Fixed

Comment thread tests/test_caldav.py Fixed

tobixen and others added 10 commits August 27, 2026 16:09

test: remove false incompatibility observations

577d2fa

search.comp-type.optional was mis-reported from earlier versions of caldav-server-tester Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

tobixen force-pushed the fix/robur-compat-and-async-propfind branch from f98cb13 to 577d2fa Compare August 27, 2026 14:15

tobixen merged commit ab5238a into master Aug 27, 2026
16 checks passed

Navigation Menu

Uh oh!

Robur compatibility, async propfind, and url.encode-at - #705

Robur compatibility, async propfind, and url.encode-at#705
tobixen merged 10 commits into
masterfrom
fix/robur-compat-and-async-propfind

tobixen commented Aug 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tobixen commented Aug 27, 2026

Robur

AsyncDAVClient.propfind(props=...)

load(multiget_fallback=False)

url.encode-at

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`AsyncDAVClient.propfind(props=...)`

`load(multiget_fallback=False)`

`url.encode-at`