CardDAV: discover the address-book home instead of assuming /addressbooks/ - #5
Merged
Merged
Conversation
…ooks/ The backend appended /addressbooks/ to whatever carddav_url it was given -- Soverin's layout, and nobody else's. Any provider that publishes a bare host name (iCloud, GMX, Yandex), a discovery root (mailbox.org) or the home collection itself (Fastmail) was therefore unreachable unless the customer found and pasted a home collection by hand, which is why prefilling a CardDAV URL was not something the admin catalog could do. _discover now walks RFC 6764's current-user-principal -> addressbook-home-set hops from the URL as given, then /.well-known/carddav, then the origin. The old path stays as the fallback for a server answering neither property, so an existing Soverin-shaped setup keeps working. A 401 during discovery raises ProviderAuthError immediately rather than being retried into a confusing "no address books found", and an unreachable host becomes a ProviderError instead of leaking httpx's own exception through the protocol layer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ktjy3ubr7ESGeSN1dYuKx5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The CardDAV backend appended
/addressbooks/to whatevercarddav_urlit wasgiven — Soverin's layout, and nobody else's.
_discover()now walks RFC 6764'scurrent-user-principal→addressbook-home-sethops instead, starting at theURL as given, then
/.well-known/carddav, then the origin.Why
CardDAV standardises no paths, so the URL a provider publishes is whatever that
provider felt like publishing:
/addressbooks/Everything but the last resolved to a 404, so a customer could paste the exact
address their provider documents and be told there were no address books. That
is what kept the admin package's provider catalog from prefilling a CardDAV URL
at all: only a home collection spelled out per customer would have worked, and
most providers do not publish one.
The CalDAV side never had this problem — the
caldavlibrary'sprincipal()already does the same discovery.
Behaviour kept
/addressbooks/path survives as the fallback for a server answeringneither property, so an existing Soverin-shaped setup keeps working
unchanged.
ProviderAuthErrorimmediately rather thanbeing retried against the next URL — otherwise a wrong password surfaced as a
confusing "no address books found" three requests later.
ProviderError, so httpx's own exception stopsleaking through the protocol layer.
Tests
tests/test_carddav_discovery.py(fast suite,httpx.MockTransport, nonetwork) pins all four URL shapes, the fallback, the 401 and the unreachable
host. Full fast suite green: 53 passed;
ruffandtyclean.Generated by Claude Code