diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml new file mode 100644 index 00000000..20e6ae0c --- /dev/null +++ b/.github/workflows/audit.yml @@ -0,0 +1,43 @@ +--- +name: audit + +# Dependency vulnerability audit (pip-audit, see the `audit` env in tox.ini). +# +# This is deliberately *not* wired into the `tests` workflow: a new advisory +# can be published against an unchanged dependency tree, so the audit is +# time-triggered rather than change-triggered. The pull_request trigger is +# narrowed to the files that can change the dependency tree. +on: + pull_request: + paths: + - pyproject.toml + - tox.ini + - .github/workflows/audit.yml + workflow_dispatch: + schedule: + # Mondays 04:17 UTC, well clear of the nightly link check (22:03). + - cron: "17 4 * * 1" + +concurrency: + group: audit-${{ github.ref }} + cancel-in-progress: false + +jobs: + pip-audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + # hatch-vcs derives the version from git tags; without them the + # project metadata pip-audit reads cannot be built. + fetch-depth: 0 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - uses: actions/cache@v4 + with: + path: ~/.cache/pip + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} + - run: pip install tox + - name: Audit dependencies for known vulnerabilities + run: tox -e audit diff --git a/.github/workflows/linkcheck.yml b/.github/workflows/linkcheck.yml index 061798b9..9a1a1717 100644 --- a/.github/workflows/linkcheck.yml +++ b/.github/workflows/linkcheck.yml @@ -7,6 +7,10 @@ on: schedule: - cron: "03 22 * * *" +concurrency: + group: linkcheck-${{ github.ref }} + cancel-in-progress: false + jobs: linkcheck: runs-on: ubuntu-latest @@ -14,6 +18,12 @@ jobs: issues: write steps: - uses: actions/checkout@v5 + - name: Restore lychee cache + uses: actions/cache@v4 + with: + path: .lycheecache + key: cache-lychee-${{ github.run_id }} + restore-keys: cache-lychee- - name: Check links with Lychee id: lychee uses: lycheeverse/lychee-action@v2 @@ -21,15 +31,41 @@ jobs: fail: false args: >- --root-dir "$(pwd)" - --timeout 20 - --max-retries 3 + --timeout 30 + --max-retries 6 + --retry-wait-time 2 --cache --max-cache-age 14d . - - name: Create Issue From File - if: steps.lychee.outputs.exit_code != 0 - uses: peter-evans/create-issue-from-file@v5 - with: - title: Link Checker Report - content-filepath: ./lychee/out.md - labels: report, automated issue + # The exit_code comparisons are quoted on purpose. A missing output is the + # empty string, and GitHub coerces '' to 0 when comparing against a number - + # so an unquoted `== 0` would treat "lychee did not run" as "all links are + # healthy" and close every open report. Comparing two strings does no + # coercion. + - name: Create or update Link Checker issue + if: steps.lychee.outputs.exit_code != '' && steps.lychee.outputs.exit_code != '0' && github.ref == 'refs/heads/master' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + # Keep the oldest open report issue as canonical, fold duplicates in. + # Both labels are required: `gh issue list` ANDs them, so a + # human-written issue that merely carries "report" cannot become the + # canonical one and have its body overwritten by lychee output. + ISSUES=$(gh issue list --label "report" --label "automated issue" --state open --json number --jq '.[].number' | sort -n) + CANON=$(printf '%s\n' "$ISSUES" | head -1) + for n in $(printf '%s\n' "$ISSUES" | tail -n +2); do + gh issue close "$n" --comment "Duplicate of #${CANON} - auto-closed by the link checker." + done + if [ -n "$CANON" ]; then + gh issue edit "$CANON" --body-file ./lychee/out.md + else + gh issue create --title "Link Checker Report" --body-file ./lychee/out.md --label "report" --label "automated issue" + fi + - name: Close Link Checker issue if all links are healthy + if: steps.lychee.outputs.exit_code == '0' && github.ref == 'refs/heads/master' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + for n in $(gh issue list --label "report" --label "automated issue" --state open --json number --jq '.[].number'); do + gh issue close "$n" --comment "All links are now healthy." + done diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml new file mode 100644 index 00000000..fd2d0643 --- /dev/null +++ b/.github/workflows/package.yml @@ -0,0 +1,57 @@ +--- +name: package + +# Builds the release artifacts and verifies their contents. +# +# Nothing in CI used to build an sdist at all, so what actually went into a +# release was only ever discovered after it was published: caldav-3.2.1.tar.gz +# shipped .claude/settings.json and 1755 files under venv/. The check runs on +# every change to the packaging configuration, and nightly, so a stray file in +# a contributor's tree cannot ride along into a tarball unnoticed. +# +# See tests/tools/check_dist.py for what is verified. +on: + push: + branches: + - master + pull_request: + paths: + - pyproject.toml + - tox.ini + - MANIFEST.in + - .gitignore + - tests/tools/check_dist.py + - .github/workflows/package.yml + workflow_dispatch: + schedule: + # Sundays 05:23 UTC, clear of the Monday audit (04:17) and the nightly + # link check (22:03). + - cron: "23 5 * * 0" + +concurrency: + group: package-${{ github.ref }} + cancel-in-progress: true + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + # hatch-vcs derives the version from git tags. + fetch-depth: 0 + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + - uses: actions/cache@v4 + with: + path: ~/.cache/pip + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} + - run: pip install tox + - name: Build sdist and wheel, and check what is in them + run: tox -e package + - uses: actions/upload-artifact@v4 + with: + name: dist + path: .tox/package/tmp/dist/* + if-no-files-found: error diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 778054ad..6661ebc8 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -97,7 +97,7 @@ jobs: - uses: actions/cache@v4 with: path: ~/.cache/pip - key: pip|${{ hashFiles('setup.py') }}|${{ hashFiles('tox.ini') }} + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} - run: pip install tox - name: Configure Baikal with pre-seeded database run: | @@ -183,13 +183,28 @@ jobs: docker exec ${{ job.services.nextcloud.id }} php occ config:system:set ratelimit.whitelist.0 --value='172.17.0.0/16' || true docker exec ${{ job.services.nextcloud.id }} php occ config:system:set ratelimit.whitelist.1 --value='127.0.0.1' || true - # Clear rate limit cache - docker exec ${{ job.services.nextcloud.id }} php -r " - \$db = new PDO('sqlite:/var/www/html/data/nextcloud.db'); - \$db->exec('DELETE FROM oc_ratelimit_entries'); - \$db->exec('DELETE FROM oc_bruteforce_attempts'); - echo 'Cleared rate limit and bruteforce caches\n'; - " || true + # Clear rate limit cache. The SQLite file is named after the `dbname` + # config value, which defaults to `owncloud` — look it up rather than + # guessing, and check it exists first: PDO creates a missing SQLite + # file, so a wrong path silently yields "no such table" for every + # DELETE below. + DB_NAME=$(docker exec ${{ job.services.nextcloud.id }} php occ config:system:get dbname 2>/dev/null | tr -d '\r\n') + DB_PATH="/var/www/html/data/${DB_NAME:-owncloud}.db" + if docker exec ${{ job.services.nextcloud.id }} test -f "$DB_PATH"; then + docker exec ${{ job.services.nextcloud.id }} php -r " + \$db = new PDO('sqlite:$DB_PATH'); + foreach (['oc_ratelimit_entries', 'oc_bruteforce_attempts'] as \$table) { + try { + \$db->exec(\"DELETE FROM \$table\"); + } catch (PDOException \$e) { + fwrite(STDERR, \"skipping \$table: \" . \$e->getMessage() . \"\n\"); + } + } + echo \"Cleared rate limit and bruteforce caches\n\"; + " || true + else + echo "No database found at $DB_PATH — skipping cache cleanup" + fi echo "Nextcloud is configured!" - name: Configure Cyrus @@ -326,7 +341,7 @@ jobs: - uses: actions/cache@v4 with: path: ~/.cache/pip - key: pip|${{ hashFiles('setup.py') }}|${{ hashFiles('tox.ini') }} + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} - run: pip install tox - run: tox -e docs style: @@ -339,7 +354,7 @@ jobs: - uses: actions/cache@v4 with: path: ~/.cache/pip - key: pip|${{ hashFiles('setup.py') }}|${{ hashFiles('tox.ini') }} + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} - uses: actions/cache@v4 with: path: ~/.cache/pre-commit @@ -356,7 +371,7 @@ jobs: - uses: actions/cache@v4 with: path: ~/.cache/pip - key: pip|${{ hashFiles('setup.py') }}|${{ hashFiles('tox.ini') }} + key: pip|${{ hashFiles('pyproject.toml') }}|${{ hashFiles('tox.ini') }} - run: pip install tox - run: tox -e deptry # The three async-* jobs below exist to test the async backend *selection* logic, diff --git a/.gitignore b/.gitignore index bbbc3491..708a129a 100644 --- a/.gitignore +++ b/.gitignore @@ -21,12 +21,15 @@ accountific.db tests/.noseids *.bak *~ -#*# +# Emacs auto-save files. The backslashes are required: a bare "#" starts a +# comment, so the pattern was silently a no-op. +\#*\# caldav.egg-info/ tests/conf_private.py .tox .eggs .venv +venv caldav/_version.py tests/docker-test-servers/baikal/baikal-backup/ tests/docker-test-servers/*/baikal-backup/ @@ -34,3 +37,7 @@ tests/docker-test-servers/*/baikal-backup/ !tests/docker-test-servers/baikal/Specific/ # Local test server configuration (may contain credentials) tests/caldav_test_servers.yaml +# Lychee link checker cache +.lycheecache +# Scratch files from AI sessions (review notes, draft commit messages) +docs/design/tmp-* diff --git a/.lycheeignore b/.lycheeignore index 2a9c7c03..c04f7500 100644 --- a/.lycheeignore +++ b/.lycheeignore @@ -2,6 +2,7 @@ https?://your\.server\.example\.com/.* https?://.*\.example\.com(:\d+)?(/.*)?$ https?://domain/.* +https?://evil.attacker.com/caldav/ # Localhost URLs for test servers (not accessible in CI) http://localhost:\d+/.* @@ -17,6 +18,7 @@ https://caldav\.gmx\.net/.* https://caldav\.icloud\.com/.* https://p\d+-caldav\.icloud\.com/.* https://posteo\.de:\d+/.* +https://sync\.infomaniak\.com/.* https://purelymail\.com/.* https://webmail\.all-inkl\.com/.* https://www\.google\.com/calendar/dav/.* @@ -36,6 +38,10 @@ https://oauth2\.googleapis\.com/.* # Personal/demo test server (may be down) https?://davical\.bekkenstenveien53c\.oslo\.no/.* +# Sites that serve 403 to non-browser clients. The links are fine in a +# browser; lychee just isn't one. +https://stackoverflow\.com/.* + # Dead or broken links we can't fix http://fsf\.org/.* http://oxpedia\.org/.* diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e25a8425..a2489f42 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,21 +1,21 @@ --- repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.9.4 + rev: v0.15.20 hooks: - - id: ruff + - id: ruff-check args: [--fix] - id: ruff-format - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v5.0.0 + rev: v6.0.0 hooks: - - id: check-byte-order-marker + - id: fix-byte-order-marker - id: trailing-whitespace - id: end-of-file-fixer - repo: https://github.com/pycalendar/ai-prompt-auto-commit - rev: v0.0.5 + rev: v0.0.8 hooks: - id: unstage-ai-prompts - id: append-ai-prompts @@ -26,13 +26,13 @@ repos: stages: [manual] - repo: https://github.com/compilerla/conventional-pre-commit - rev: v3.4.0 + rev: v4.4.0 hooks: - id: conventional-pre-commit stages: [commit-msg] - repo: https://github.com/lycheeverse/lychee - rev: lychee-v0.24.1 + rev: lychee-v0.24.2 hooks: - id: lychee args: ["--no-progress", "--timeout", "10", "--exclude-path", ".lycheeignore", "--max-cache-age=30d", "--cache"] diff --git a/CHANGELOG.md b/CHANGELOG.md index 486aa3ed..e8a8019f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,79 @@ Changelogs prior to v3.0 is pruned, but was available in the v3.1 release This project should adhere to [Semantic Versioning](https://semver.org/spec/v2.0.0.html), though for pre-releases PEP 440 takes precedence. +## [Unreleased] + +### Added + +* New `write-delay` server-peculiarity in `compatibility_hints.py`: for servers that process writes asynchronously (a PUT/DELETE/MKCALENDAR/PROPPATCH/... returns before the change is queryable, so an immediate read-back 404s or returns stale data), a client must wait a bit after every write. This is the general, write-side counterpart of `search-cache` (which only delays searches). Configured as `{'behaviour': 'delay', 'delay': }`; the integration test suites (sync and async) honour it by sleeping after every write request. The new Infomaniak profile uses `write-delay` (16s) rather than `search-cache`, since the asynchronicity there is server-wide rather than search-specific. +* New compatibility flag `save-load.event.recurrences.exception.reschedule`: whether the server accepts re-anchoring a whole recurring event (moving the master `DTSTART`) while detached exceptions (`RECURRENCE-ID`) are attached. OX App Suite rejects this with `409 Conflict` even with a matching `If-Match` etag, although rescheduling an exception-free recurring event works there. `testEditSingleRecurrence` now gates its final `save(all_recurrences=True)` dtstart/dtend step on this flag. +* The JMAP clients now keep a persistent HTTP session, so connections are reused across requests instead of a fresh TCP+TLS handshake per JMAP call. `JMAPClient` and `AsyncJMAPClient` can be used as (async) context managers, and the session can also be released explicitly with `client.close()` / `await client.aclose()`. +* `caldav.config.extract_conn_params_from_section` is now public API (renamed from `_extract_conn_params_from_section`), so that downstream tools like plann can map plann-style config sections (`caldav_url`, `caldav_user`, `features`, etc.) to `DAVClient` parameters without duplicating the logic. +* New compatibility feature `create-calendar.stable-url` (default `full`): whether a calendar, once created, remains addressable at the URL derived from the requested `cal_id`. Some servers assign a different *canonical* URL: Zimbra relocates the collection to a display-name-derived path when a display name is set (a collection alias lingers at the `cal_id` and answers `PROPFIND`/`REPORT`, but a `GET` on a child object under it 404s, so the `cal_id` is not a usable address); OX always exposes an opaque `cal://0/NNN` (base64-segment) canonical URL. Both are marked `create-calendar.stable-url: unsupported`. For such servers `Calendar._create()` now discovers and adopts the canonical URL after creation (re-pointing `self.url`) instead of dropping the display name, so the calendar keeps its name *and* every later URL-based operation resolves — identical handling for Zimbra and OX. +* New `compatibility_workarounds` parameter on `Calendar.search()` / `CalDAVSearcher.search()` / `async_search()`. When `False`, all server-compatibility workarounds are disabled and the query is sent verbatim (a single REPORT, no comp-type splitting, no filter rewriting, no fallback retries). Mainly for the server-compatibility checker, to observe raw server behaviour. + +### Fixed + +* The source distribution no longer ships stray local files. hatchling's VCS-ignore support only honours the *root* `.gitignore`, so files hidden from `git status` by a nested `.gitignore` or by the packager's global git ignore file were invisible locally and packaged anyway — `caldav-3.2.1.tar.gz` contains `.claude/settings.json` and 1755 files under `venv/`, and the current tree would have added 443 files under `.prompts/`. A new `package` tox environment (run in CI, and now part of the release procedure) builds both artifacts and fails if anything git does not track turns up in them. +* Looking up a calendar or object that does not exist now raises `NotFoundError` also when the server reports the 404 inside a `207 Multi-Status` (a bare `` on the `` element, which RFC 4918 §14.24 allows) rather than as a plain HTTP 404. Previously the property lookup found no `propstat` elements, ignored the 404, and returned `None` for every requested property — so e.g. `calendar.get_display_name()` on a non-existent calendar silently returned `None` while `calendar.get_events()` on the same calendar raised. Observed against Xandikos. +* `compatibility_hints.py`: `testCheckCompatibility` had a blind spot — a sub-feature the server-tester explicitly probed whose *observed* status happened to equal the type default (e.g. a server-feature observed as `full`) was dropped from the compacted observed dict, and if its *declared* status was only inherited from a parent (not an explicit key) it was absent from the compacted expected dict too. Being in neither dict, it was never compared, so a real conflict went unreported. Concretely, Infomaniak declares `search.comp-type` `unsupported` while genuinely supporting the optional-comp-type behaviour (`search.comp-type.optional`), and the mismatch slipped through. The comparison is now a unit-tested `FeatureSet.compare()` method that also iterates the probed feature set, and the Infomaniak profile declares `search.comp-type.optional: full` explicitly. +* `jmap/client.py` and `jmap/async_client.py` `update_event()`: to honour RFC 8620 PatchObject merge semantics, the update null-injects every optional property absent from the new iCalendar so removed properties are actually cleared server-side. Some servers (observed with Stalwart) reject a property they do not support — e.g. `recurrenceRules`/`excludedRecurrenceRules` — as `invalidProperties` even when it is being set to `null`, which made every `update_event()` against such a server fail. Nulling an absent property is harmless cleanup, so the update now drops the server-rejected null-cleanup keys and retries (looping, since some servers report only one offending property per response) until the update succeeds. A rejection of a property the client actually assigned a value still surfaces as `JMAPMethodError`. +* `async_davclient.py` `_async_request()`: the issue-#158 connection-abort workaround sent a probe GET to detect the auth challenge; if the probe returned anything other than 401+WWW-Authenticate (e.g. a 200 HTML login page), the code fell through to `response = DAVResponse(probe_r, self)` — returning the probe response as if it were the original request's response, and silently swallowing the real connection error. Now the original exception is re-raised when the probe does not yield a challenge. +* `collection.py` `freebusy_request()`: for async clients, `add_attendee()` was called on each attendee before the `is_async_client` check dispatched to `_async_freebusy_request()`. For a `Principal` attendee on an async client, `get_vcal_address()` returns a coroutine; `add_attendee` then tried to set `.params` on the coroutine → AttributeError. `_async_save_with_invites` already awaited `get_vcal_address()` correctly. Fixed by passing attendees to `_async_freebusy_request()` and performing the await there. +* `search.py`: the documented `operator='=='` exact-match guarantee was never enforced — `post_filter` was not set to `True` for `==` searches, so the server's substring semantics leaked through. `icalendar_searcher.check_component()` already handles `==` as exact-match; the fix adds `==` to the `post_filter=True` trigger conditions. +* `async_davclient.py` `aio.get_calendars(calendar_name=...)`: name-based lookup iterated `self.get_calendars()` synchronously through the non-async `Principal.calendar(name=...)` path, which returns a coroutine for async clients; the loop body was iterating over the coroutine object, never the calendars, so name-based lookup returned nothing. Fixed by calling `await principal.get_calendars()` and filtering by display-name in the async path. +* `async_davclient.py` `get_calendars()`: lacked the GMX principal-URL fallback present in the sync client — when `calendar-home-set` was missing the async path returned `[]` immediately instead of falling back to the principal URL as calendar home. Parity restored. +* `collection.py` `Principal.calendar(cal_id=...)`: for async clients a bare (non-URL) `cal_id`/`name` raised `TypeError: argument of type 'coroutine' is not a container or iterable`, because the synchronous `calendar_home_set` property evaluated `"@" in ` without awaiting the async `get_property`. It now returns a coroutine that resolves the calendar home set (PROPFIND) before constructing the `Calendar`; a full-URL `cal_id`/`cal_url` needs no home set and still returns synchronously. The async integration tests' pre-test calendar cleanup relied on this call and swallowed the error in a bare `except`, so leftover calendars were never removed and a later MKCALENDAR `405 "resource already exists"` followed (seen against Infomaniak). Cleanup is now centralised in the `adelete_calendar_if_present()` test helper with a narrow `except`. +* `search.py`: the `undef` operator branch used `property.upper()` without the `category→CATEGORIES` alias mapping that the regular filter branch applies, so `add_property_filter('category', '', operator='undef')` queried the nonexistent `CATEGORY` property. `is-not-defined` on a nonexistent property matches every object, so the filter silently returned all events regardless of whether they had categories. +* `davclient.py` and `async_davclient.py`: rate-limit retry raised `TypeError: unsupported operand type(s) for +=: 'NoneType' and 'float'` on the second 429 response when the server provides no usable `Retry-After` value (`compute_sleep_seconds` returns `None`). The `sleep_seconds += rate_limit_time_slept / 2` line executed before the `sleep_seconds is None` guard. Now re-raise `RateLimitError` first, then update the sleep estimate. +* `davclient.py` `DAVClient.__init__()`: (a) `DAVClient(url='https://user@host/', password='secret')` crashed with `TypeError` — `unquote(self.url.password)` was called unconditionally when the URL had a username, but `self.url.password` is `None` when the URL contains no password. (b) URL-embedded credentials silently overrode explicit `username`/`password` kwargs; the async client already gave explicit kwargs higher precedence. Now: explicit kwargs win; URL credentials are only used as fallback when kwargs are absent. An explicit `username` discards the URL credentials wholesale rather than merging them field by field — otherwise `DAVClient(url='https://bob:hunter2@cal.example.com/', username='alice')` would ship alice's login with bob's password. Overriding only the password keeps the URL's username, which stays a coherent pair. +* `config.py` `resolve_features()`: returning a named profile (`features='xandikos'`) returned the module-level dict object directly without copying it. Any code that then mutated the returned dict (e.g. `testing.py` patching `auto-connect.url.domain`) permanently corrupted the module-level dict for the whole process lifetime, so a second `DAVClient(features='xandikos')` would see the mutated domain. Similarly `testing.py` `XandikosServer`/`RadicaleServer` used a shallow `.copy()` — nested dict mutation still reached the module level. All three now use `copy.deepcopy()`. +* `config.py` `get_connection_params()`: explicit keyword arguments (e.g. `get_davclient(password='secret')`) were only respected when `url` or `features` was also present. When an env-var or config-file source was found instead, explicit params were silently dropped. Now the explicit params are merged (overlaid) on top of whatever lower-priority source wins — including the test-server source, which previously returned its configuration verbatim. A keyword argument whose value is `None` counts as "not supplied" rather than "unset it", so the common `get_davclient(url=args.url, username=args.user, password=args.password)` CLI wrapper no longer wipes `CALDAV_URL` when the user only passed `--password`. An empty string is still explicit, since that is meaningful for servers with no authentication. +* `calendarobjectresource.py` `_complete_recurring_safe()`: completing a recurring task passed the caller-supplied `completion_timestamp` to `_next()` correctly but then called `completed.complete()` without it, so the completed copy always recorded the current wall-clock time as `COMPLETED` regardless of the timestamp the caller specified. The async twin already passed `completion_timestamp` through; sync is now consistent. +* `calendarobjectresource.py` `_get_duration()`: `isinstance(i["DTSTART"], datetime)` tested the `vDDDTypes` wrapper object (which is never a `datetime`), so the date-vs-datetime branch always took the "is a date" path. A VTODO with a timed DTSTART and no DUE/DURATION got `duration = timedelta(days=1)` instead of `timedelta(0)`, shifting the next due date by one day when completing a recurring task. Fixed: test `isinstance(i["DTSTART"].dt, datetime)`. +* `jmap/client.py` and `jmap/async_client.py` `create_task()`: a JMAP server response that returned an empty `created` dict (with neither a `created` entry nor a `notCreated` entry for `"new-0"`) raised a bare `KeyError` instead of the documented `JMAPMethodError`. The `create_event()` method already had the required guard; `create_task()` was missing it in both sync and async clients. +* `lib/vcal.py` `fix()`: truncated iCalendar data (no `END:` line) triggered a bare `assert` which gave no useful message and was silently skipped under `python -O`. Now logs a warning and returns the data unchanged instead. +* `lib/vcal.py` `create_ical()`: when both `alarm_*` props and `ical_fragment` were supplied, the fragment was injected before the first `END:V` line — which is `END:VALARM`, placing e.g. an `RRULE` *inside* the alarm component. The regex now targets `END:V(EVENT|TODO|JOURNAL)` specifically. +* `lib/vcal.py` `fix()`: the backslash-unescape step used `('\"')` as a regex group, which matches only the literal two-character sequence `'"`. A backslash before a lone `'` or lone `"` was silently left in place. Fixed by using the character class `['\"]`. +* `lib/vcal.py` `fix()`: the trailing-whitespace fixup (`re.sub(" *$", "", fixed)`) lacked `re.MULTILINE`, so it only stripped trailing spaces at the very end of the document and never per-line. It now strips per-line — but deliberately *not* from a line that is continued by a folded line: RFC 5545 3.1 folds blind at 75 octets, so the fold may land right after a space belonging to the value, and stripping it would join two words together. Junk whitespace in front of a fold (e.g. in iCloud `X-APPLE-STRUCTURED-LOCATION` base64 values) is therefore still left alone; it cannot be told apart from real content. +* `lib/error.py` `PYTHON_CALDAV_COMMDUMP`: when this debug env-var is set, a `logging.warning()` is now emitted at import time to remind the operator that request/response bodies and headers (including credentials and calendar PII) are being written to uniquely-named files under `/tmp` that accumulate indefinitely. +* `jmap/objects/calendar.py` `JMAPCalendar.search()`: `datetime` arguments for `start`/`end` were formatted with `datetime.isoformat()`, which produces `+HH:MM` offsets for aware non-UTC datetimes and no timezone indicator for naive datetimes. JMAP requires UTCDate format (`YYYY-MM-DDTHH:MM:SSZ`). Fixed by converting to UTC and using `strftime`. +* `jmap/client.py` and `jmap/async_client.py` `get_objects_by_sync_token()`: the `newState` from `CalendarEvent/changes` was discarded into `_`, so callers could not chain sync calls without a separate `get_sync_token()` round-trip — a race window where intervening changes would be silently missed. The method now returns a 4-tuple `(added, modified, deleted, new_sync_token)` instead of a 3-tuple. +* `compatibility_hints.py` `FeatureSet.copyFeatureSet()`: merging a plain-string feature value over an existing string-valued entry in the feature set raised a bare `AssertionError` — the `'support' not in server_node` guard blocked the update branch and fell through to `else: raise AssertionError`. Plain strings are the dominant style in the hint dicts, so any two-layer server config expressing the same feature crashed. Fixed by removing the `not in server_node` condition. +* `compatibility_hints.py` `FeatureSet.copyFeatureSet()`: an unknown feature name in a config file produced only a `UserWarning` but still stored the bad key in `_server_features`; a later `collapse()`/`is_supported()` call then raised a message-less `AssertionError` far from the original config. Fixed by `continue`-ing after the warning so unknown keys are never stored. +* `async_davclient.py`: HTML-on-401 diagnostic hint checked `self.headers` (the client's own request headers) for `Content-Type: text/html` instead of `r.headers`, so the intended "server returned an HTML login page, consider setting auth_type" message could never fire. +* `base_client.py` `get_calendars(calendar_urls=...)`: a calendar explicitly requested by URL was silently omitted from the result when its `displayname` property is the empty string `""`, because the check `if _try(calendar.get_display_name, ...)` was a truthiness test. The async counterpart already used `is not None`; sync is now consistent. +* `config.py` `expand_config_section()`: requesting a section name that is absent from the config raised `KeyError` instead of returning `[]`, causing plain `caldav.get_calendars()` to crash with `KeyError: 'default'` on configs with no `default` section. +* `config.py` `expand_config_section()`: `disable: true` was silently ignored for sections fetched by explicit name or via a `contains` list — the check used the string literal `"section"` as the config key instead of the `section` variable. Only the glob `"*"` path honoured `disable`. +* `lib/auth.py` `extract_auth_types()`: a `WWW-Authenticate` header ending with a trailing comma (seen in the wild) raised `IndexError` in the set comprehension because `h.split()` on an empty segment fails. Added an `if h.strip()` guard. +* `calendarobjectresource.py` `change_attendee_status()`: calling the method on an event with no `ATTENDEE` properties at all raised a bare `KeyError('ATTENDEE')` instead of the expected `NotFoundError`; the `try/except NotFoundError` wrapper in the `Principal` dispatch path could not catch it, so the "Principal is not invited" message was unreachable. Also, the genuine not-found error message contained a literal `%s` that was never substituted with the attendee address. +* `calendarobjectresource.py` `add_attendee()`: passing an attendee address with an uppercase or mixed-case URI scheme (`"MAILTO:user@example.com"`) raised `UnboundLocalError` — the scheme check used `str.startswith("mailto:")` which is case-sensitive, so the address fell through all branches without assigning `attendee_obj`. RFC 3986 §3.1 specifies URI schemes are case-insensitive. +* `response.py`: the XML parser is now constructed with `resolve_entities=False, no_network=True`, so a malicious or MITM server cannot inject arbitrary text into parsed property values through inline DOCTYPE entity definitions. On the lxml versions most people have this was already the effective behaviour (`no_network` defaults to True, and `resolve_entities` defaults to `'internal'` from lxml 5.0), but lxml is an unpinned dependency and the parser should not be relying on another project's defaults for this. +* `discovery.py` `discover_service()`: `require_tls=True` was not enforced on the well-known URI redirect target — a same-domain `Location: http://...` passed the domain-validation check and was returned as `ServiceInfo(tls=False)`, allowing a misconfigured or MITM server to silently downgrade the connection to plaintext. Fixed by checking `well_known_info.tls` against `require_tls` before returning the result. +* `datastate.py` `RawDataState.get_component_type()`: tested for the string `"BEGIN:FREEBUSY"` but real iCalendar data uses `"BEGIN:VFREEBUSY"`, so any `FreeBusy` object holding raw data returned `component_type=None` — making `is_loaded()` and `has_component()` return `False`, `save()` silently no-op at its early return, and `load(only_if_unloaded=True)` reload spuriously on every call. The same typo appeared in the base-class `get_uid()` and `get_component_type()` fallback parsers which looked for `comp.name == "FREEBUSY"` instead of `"VFREEBUSY"`. +* `calendarobjectresource.py` `_set_data()`: the raw-string branch cleared the legacy `_data`/`_vobject_instance`/`_icalendar_instance` attributes but never reset `self._state`. Once `_state` was populated by an earlier call to `_ensure_state()` (triggered by e.g. `.id` or `is_loaded()`), all subsequent reads via `get_data()`, `get_icalendar_instance()`, and `.id` served the pre-reload content even after `load()` fetched new data from the server. +* `search.py`: the `search.combined-is-logical-and: unsupported` workaround (triggered on e.g. Nextcloud) stripped all property filters from the server query to send only the time range, but passed `post_filter=None` (the ambient value) to `filter()` instead of `True`. `_filter_search_results` short-circuits when `post_filter` is falsy, so a search with both a time range and a property filter (e.g. `SUMMARY contains "foo"`) returned every object in the time range — the property filter was silently dropped. The sibling workarounds in the same function already used `post_filter=True`; this one now does too. +* `URL.canonical()`: two related bugs — (a) the canonical form was built from `self.url_parsed` (which still contains `user:pass@` in the netloc) rather than the auth-stripped URL, so `canonical()` leaked credentials into the returned URL and `__eq__`/`__hash__` comparisons between an authenticated client URL and a server-returned href (no credentials) were False; (b) when a URL had no auth part, `unauth()` returned `self` and `canonical()` then overwrote `url_raw`/`url_parsed` in place — a bare `==` or `hash()` call silently mutated the URL object, potentially re-encoding special characters (e.g. `+` → `%2B`) and causing subsequent requests to target the wrong resource. Fixed by using the auth-stripped URL's parsed form for `arr` and always returning a fresh `URL` object. +* `_post_put`: a 302 response to `PUT` always raised `IndexError` instead of following the redirect — iterating the headers dict yields key strings, not tuples, so `x[0]` was the first character of each header name, never `"location"`. Fixed by using `r.headers.get("location")`. +* `vcal.fix()`: the `COMPLETED` date-to-datetime regex consumed the trailing newline, merging the following iCal property into the `COMPLETED` value on every inbound object from a server that stores `COMPLETED` as a plain date (e.g. SOGo). Fixed by using a lookahead `(?=\s)` instead of consuming `\s`. + +* Time-range searches without a component type (`search(start=..., end=...)` with no `event`/`todo`/`journal`/`comp_class`) crashed against SabreDAV-based servers (Baikal, Nextcloud, ...) with `ReportError`: *"You cannot add time-range filters on the VCALENDAR component"*. A `CALDAV:time-range` is only valid inside a `VEVENT`/`VTODO`/`VJOURNAL`/`VFREEBUSY`/`VALARM` comp-filter (RFC4791 section 9.7), never directly under `VCALENDAR`. The library now splits such a search into one query per component type, and additionally recovers from the server rejection at runtime if it occurs anyway. See https://github.com/python-caldav/caldav/issues/681 +* Property-filter searches without a component type (e.g. `search(category=...)` or other attribute filters with no `event`/`todo`/`journal`/`comp_class`) silently returned nothing on most servers (Xandikos, SabreDAV, ...): the prop-filter landed under the `VCALENDAR` comp-filter, which has no component properties like `CATEGORIES` to match. The library now splits such a search into one query per component type as well (`search.text.comp-type-optional`). See https://github.com/python-caldav/caldav/issues/681 Results from such a split are deduplicated by URL, since a resource that legally holds both a `VEVENT` and a `VTODO` matches two of the three queries; they come back grouped by component type rather than in server order, so pass a sort key if the order matters. +* `search()`'s generator driver now feeds exceptions raised while executing a request back into the search logic, so the server-compatibility fallbacks and per-object load error handling actually take effect (previously dead code). Applies to both the sync and async code paths. +* `compatibility_hints`: OX was pinned to `create-calendar.set-displayname: unsupported` (a value masked by a checker bug that verified the feature by display-name lookup, which a leftover/colliding calendar would shadow); OX stores the display name as a property separate from the calendar URL and honours it at creation time, so the expectation is corrected to `full`. +* `compatibility_hints`: Stalwart's `search.recurrences.expanded.exception` was inheriting the default `full`, but Stalwart's server-side `CALDAV:expand` only suppresses the exception-overridden occurrence when `SEQUENCE` is absent. With `SEQUENCE` present (as real-world clients always emit) it returns both the original occurrence and the override, so the expectation is corrected to `fragile`. +* Config file sections with `features` but no `caldav_url` were rejected, even though the URL can be derived from the `auto-connect.url` compatibility hints. Explicitly passed parameters already worked this way; now `get_davclient(config_section=...)` and friends behave consistently. +* `jmap/convert/jscal_to_ical.py`: a `recurrenceOverrides` entry that does not include a `"start"` key (the common case — title-only change, description update, etc.) produced a child `VEVENT` with `DTSTART` copied from the master event's start time rather than from the override key. This effectively relocated every non-rescheduled override to the master's first occurrence, breaking all override display. Default is now the override key itself. +* `jmap/convert/jscal_to_ical.py`: `EXDATE` and `RECURRENCE-ID` values were always emitted as floating (timezone-less) `DATE-TIME` regardless of the event's `timeZone` or `showWithoutTime` flag. Per RFC 5545 §3.8.5.1 the value type must match `DTSTART`; a floating `EXDATE` on a `TZID`-anchored event does not match any instance, so excluded occurrences reappear. Override keys are now parsed with the event timezone applied (`TZID`-anchored events) or as `date` objects (all-day events). +* `jmap/convert/_utils.py` `_format_local_dt()`: UTC datetimes produced a `Z`-suffixed string. RFC 8984 §1.4 defines `LocalDateTime` (the type required for `recurrenceOverrides` keys and `recurrenceRules.until`) as a bare `YYYY-MM-DDThh:mm:ss` without any suffix; `Z`-suffixed override keys cannot match `LocalDateTime` occurrence keys, causing mismatches on strict servers. The function now returns a bare local representation — converted into the event's own timezone first, since a `LocalDateTime` is local *to the event*: dropping the offset from a UTC value instead of converting it would shift `EXDATE`/`RECURRENCE-ID`/`UNTIL` by the UTC offset on every `TZID`-anchored event, and emit a floating `UNTIL` against a `TZID` `DTSTART`, which RFC 5545 §3.3.10 forbids. +* `jmap/convert/ical_to_jscal.py` and `jmap/convert/jscal_to_ical.py`: the `STATUS` property was silently dropped in both conversion directions. `STATUS:CANCELLED` round-tripped as `status: confirmed` (JSCalendar default), so cancelled meetings appeared active. Mappings `CONFIRMED ↔ confirmed`, `TENTATIVE ↔ tentative`, `CANCELLED ↔ cancelled` are now implemented. +* `jmap/client.py` and `jmap/async_client.py` `update_event()`: RFC 8620 §3.3 specifies that absent keys in a PatchObject preserve the server value. `update_event` sent the full converted JSCalendar dict as the patch; properties the caller removed (e.g. LOCATION, VALARM) were absent from the patch and therefore silently persisted on the server. `update_event` now explicitly sets all optional top-level JSCalendar properties to `null` when they are absent from the conversion result, ensuring the server removes them. + +### Changed + +* Search results that need loading are now fetched with a single `calendar-multiget` REPORT instead of one `GET` per object — a 200-event search made 200 requests before. If the multiget fails the library falls back to the per-object loads, so the failure semantics of `search()` are unchanged, but a server that answers the batched REPORT badly will now show up as a search problem rather than as one bad object. +* `compatibility_hints`: fourteen directly-probed feature *nodes* that also have refinement sub-features now carry their own explicit `default` (`calendar-color`, `delete-calendar`, `get-current-user-principal`, `propfind`, `propfind.allprop`, `save-load.event`, `save-load.journal`, `save-load.mutable`, `save-load.todo`, `save-load.todo.recurrences`, `scheduling`, `search.recurrences.includes-implicit.todo`, `search.text.category`, `sync-token`). This marks them as *independent* features: `is_supported()` and `collapse()` no longer derive/fold them away from their children, so e.g. `sync-token` stays `full` even when `sync-token.delete` is `unsupported`. Each such node has a corresponding check in the server-tester (`search.comp-type` gained one); `principal-search` deliberately keeps no default since it is a genuine OR-grouping of its sub-searches. + ## [3.2.1] - 2026-05-28 The changeset in 3.2.1 is predominently added async integration tests. Those tests should now be replicating all the logic in the good old sync integration tests under `test_caldav.py`. Some few more bugs were found while adding those tests. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6110246c..3e33f20a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,7 +20,7 @@ The types used should (as for now) be one of: The `compatibility_hints.py` has been moved from the test directory to the codebase not so very long ago. Some special rules here: * Adjusting the feature set for some calendar server? Check if there exists some workarounds etc in the code for said feature, if so, then it should be considered a fix or a feature. Perhaps even a breaking change. Otherwise, use `test: ...`. (because it is relevant for the compatibility test, if nothing else). -* Adding a new feature hint? Ensure it's covered by the caldav-server-tester. Since we have a compatibility test, it will be relevant for the test - so use `test: (...)`. It should be covered by the caldav-serveer-tester, so refer to some issue or pull request for the caldav-server-tester in the commit message. +* Adding a new feature hint? Ensure it's covered by the caldav-server-tester. Since we have a compatibility test, it will be relevant for the test - so use `test: (...)`. It should be covered by the caldav-server-tester. * Changing some descriptions? That goes as `docs: ...` even if it's actually changing a variable in the code. This is not set in stone. If you feel strongly for using something else, use something else in the commit message and update this file in the same commit. diff --git a/SECURITY.md b/SECURITY.md index 0a1a63aa..1da5501e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,48 +1,84 @@ # Security policy -Issues should be fixed ASAP, and information on any security issue should be published as soon as it's fixed. Use the GitHub issue tracker or check up [CONTACT](CONTACT.md) or even the [CODE OF CONDUCT](CODE_OF_CONDUCT) file to get in touch with the maintainer. +Issues should be fixed ASAP, and information on any security issue should be published as soon as it's fixed. Serious issues should be reported privately and kept under wraps until a fix is released — use [GitHub's private vulnerability reporting](https://github.com/python-caldav/caldav/security/advisories/new) (the "Report a vulnerability" button on the Security tab), or get in touch with the maintainer via [CONTACT](CONTACT.md) or the [CODE OF CONDUCT](CODE_OF_CONDUCT) file. Use the public GitHub issue tracker only for non-sensitive issues. There are no "LTS"-releases of the CalDAV package, but the maintainer will always consider backporting security fixes if it's deemed relevant. The maintainer is doing most of the maintenance on hobby-basis and may have other things in life preventing him from dealing with issues on the go, so no guarantees are given. -All contributions are carefully reviewed by the maintainer, and all releases are carefully tested and tagged with a PGP-signed commit. +All contributions are carefully reviewed by Tobias Brox, and AI-tools are used for code reviews prior to each release. All releases are carefully tested and tagged with a PGP-signed commit. # Known security issues and risks ## RFC6764 -I do see a major security flaw with the RFC6764 discovery. If the DNS is not to be trusted, someone can highjack the connection by spoofing the service records, and also spoofing the TLS setting, encouraging the client to connect over plain-text HTTP without certificate validation. Utilizing this it may be possible to steal the credentials. This flaw can be mitigated by using DNSSEC, but DNSSEC is not widely used, and fixing support for DNSSEC validation in the CalDAV library was found to be non-trivial (perhaps I'll look into it again some time after 3.0 has been released). This has been mitigated by adding a require_tls` connection parameter that is True by default, plus by ensuring one isn't routed to a different domain. +**Summary**: auto-discovery of the CalDAV-URL seems to be insecure by design, anyone controlling your local resolver (or upstream resolvers) may try to fish out username and password. -## DDoS/OOM risk +**Mitigation**: Leave `require_tls`, `ssl_verify_cert` to the default - or better still: use a URL rather than a domain when configuring the library. + +RFC6764 discovery depends on correct DNS-lookups, but DNS is not to be trusted. The proper solution is DNSSEC, unfortunately DNSSEC is not widely used, and fixing support for DNSSEC validation in the CalDAV library was found to be non-trivial (some work has been done, but it was found to be too difficult - the pull request is stalled as for now). + +Connections can be hijacked if someone spoofs the service records. This has been partly mitigated by adding a `require_tls` connection parameter that is `True` by default, cert validation, and ensuring one isn't routed to a different domain. + +Auto-discovery and HTTP redirects also mean the host you end up talking to may not be the one you configured. If you run the library in a context where it can reach internal/private network resources (server-side request forgery, SSRF), be aware that a malicious DNS resolver or a malicious/compromised server can steer requests towards other hosts. + +## DDoS/OOM risk - recurring events/tasks search + +**Summary:** If you allow untrusted parties to specify search-terms towards a calendar containing recurring events/tasks, bad things may happen. The package offers both client-side and server-side expansion of recurring events and tasks. It currently does not offer expansion for open-ended date searches - but with a large enough timespan and a frequent enough RRULE, there may be millions of recurrences returned. Those recurrences are returned as a generator, so things will not break down immediately. However, there is no guaranteed sort order of the recurrences ... and once you add sorting parameters to the search, bad things may happen. +## XML parsing + +**Summary:** XML responses are parsed defensively by default; only relax this against servers you trust. + +The library parses XML responses from the server using lxml. By default the parser is configured to resist common XML attacks: external entity resolution is disabled (`resolve_entities=False`) and network access during parsing is blocked (`no_network=True`), guarding against XXE (XML External Entity) attacks, and lxml's built-in limits protect against oversized "billion laughs"-style entity-expansion payloads. + +The `huge_tree` connection option (default off) disables lxml's built-in parser limits so that very large calendar objects can be handled. With `huge_tree` enabled, a malicious or compromised server can exhaust available memory with a crafted XML payload — only enable it against servers you trust. See the [lxml XMLParser documentation](https://lxml.de/api/lxml.etree.XMLParser-class.html). + ## Bugs causing weird things happening -Weird things may happen due to bugs both on in the CalDAV package, on your side and on the server side. Here are some weird experiences with Zimbra: +**Summary:** Always expect the unexpected -* I have experiences that cancelling participation in an event caused the event to be cancelled for all participants (even if the person deciding to not go to the event was not an organizer and should have no permissions to edit the event). Clearly a server-side issue. -* I once tried to restore from backup and push ten years of ical code to the calendar server. The calendar server responded by re-inviting people to the meetings we had ten years ago. I'm inclined to call that also a server side bug. -* Many other things may happen. +Weird things may happen due to bugs both on in the CalDAV package, on your side and on the server side. Some anecdotes from using Zimbra: -## Malicious usage +* I once tried to restore from backup and push ten years of ical code to the calendar server. The calendar server responded by re-inviting people to the meetings we had ten years ago. I'm inclined to call that a server side bug - but it also highlights the risk of using the CalDAV library for doing operations that ordinary calendaring clients aren't doing. +* It's been observed that cancelling participation in an event caused the event to be cancelled for all participants (even if the person deciding to not go to the event was not an organizer and should have no permissions to edit the event). Clearly a server-side issue. -Beware of risks and exposure when creating applications: +## Other things to consider -* Your code may handle username and password, be careful not to expose such credentials. Even the URL to the calendar server and/or calendar may be something people want to keep private. +**Summary:** Beware of risks and exposure when creating applications: + +* Your code may handle username and password, be careful not to expose such credentials. Even the URL to the calendar server and/or calendar may be something people want to keep private. The library includes code for reading this data from a standard config file - please use it rather than reinventing the wheel or hard-coding credentials directly into your code. * Consider that calendar events and such is personal data, which deserves protection. In the EU with the GDPR, such protection is even mandated by law. * If you allow arbitrary people to create calendar content to be saved to a server, there may be some risks involved: * Depending on the server implementation, it may be possible to use the caldav library for sending spam emails. * Be aware of DoS-attacks: By storing too much / too big / specially crafted icalendar data, the server and/or client may crash or consume all available resources. - * If allowing anonymous parties to save and retrieve data from your server, you may end up with responsibility for spreading illicit information. This may include things like child porn. Political or religious propaganda may be legitimate and legal in some countries, but may involve death penalty in other countries. Your calendar server may also be used for coordinating criminal activity. -* If you allow arbitrary people to fetch calendar content from the server, there may also be some risks involved - in particular, a DoS-attack by requesting a large time span of expanded events. + * If allowing anonymous parties to save and retrieve data from your server, you may end up with responsibility for spreading illicit information. This may include things like child sexual abuse material. Political or religious propaganda may be legitimate and legal in some countries, but may involve death penalty in other countries. Your calendar server may also be used for coordinating criminal activity. +* If you allow arbitrary people to fetch calendar content from the server, there may also be some risks involved - see the separate section on DoS-attack by requesting a large time span of expanded events. + +## Supply attack risk -## Malicious code +**Summary:** Stick to released versions and check the PGP signature in the release-tag -All code contributions are carefully reviewed by Tobias Brox. Version tags are signed with PGP. Of course there is always a risk that someone takes over my PGP key and github access (It's hard to be immune against a [5$ wrench attack](https://xkcd.com/538/)). The original owner of the repository is still alive and may take over the project again should something happen to me. I would anyway encourage using AI to do risk assessments. +All code contributions are carefully reviewed by Tobias Brox. Version tags are signed with PGP. Of course there is always a risk that someone takes over my PGP key and GitHub access (It's hard to be immune against a [$5 wrench attack](https://xkcd.com/538/)). The original owner of the repository is still alive and may take over the project again should something happen to me. I would encourage using AI to do risk assessments. The library comes with a number of dependencies, one may need to evaluate the security of those too. The pyproject contains the current list. Some notes: * niquests is an optional dependency - you may replace it with requests if you don't trust niquests -* recurring-ical-events and icalendar both has the same maintainer (Nicco Kunzmann). He is considered trustworthy. -* Tobias now has a policy of moving code not related to CalDAV into separate packages. Packages under the `python-caldav` ownership on GitHub should be considered to be of the same quality and security level as the CalDAV library. -* No security review have been done of the other dependencies. +* recurring-ical-events and icalendar both have the same maintainer (Nicco Kunzmann). He is considered trustworthy. +* Tobias now has a policy of moving code not related to CalDAV into separate packages. Those packages are most of the time either published under the `python-caldav` or `pycalendar` ownership on GitHub, and should be considered to be of the same quality and security level as the CalDAV library. +* No independent security review has been done of the other dependencies - those are all considered to be mature and robust projects. + +## Communication dumper debug hook + +**Summary:** If someone has the ability to both alter the environment and full read access to /tmp (basically, someone has root access to the computer where the code is run), it will be possible to get access to all communication. Also, anyone using this debug hook must take responsibility of deleting the dumped files. + +**Mitigation:** If this worries you, set `caldav.lib.error.debug_dump_communication=False` after importing caldav. + +The following was written when `PYTHON_CALDAV_COMMDUMP` was introduced in v1.4.0: + +* An attacker that has access to alter the environment the application is running under may cause a DoS-attack, filling up available disk space with debug logging. +* An attacker that has access to alter the environment the application is running under, and access to read files under /tmp (files being 0600 and owned by the uid the application is running under), will be able to read the communication between the server and the client, communication that may be private and confidential. + +Thinking it through three times, I'm not too concerned — if someone has access to alter the environment the process is running under and access to read files run by the uid of the application, then this someone should already be trusted and will probably have the possibility to DoS the system or gather this communication through other means. + +As of v3.3 (to be released towards the end of 2026-06), a warning is logged at import time when this variable is set, reminding the operator that request/response bodies and headers (including credentials and calendar PII) are written to uniquely-named files under `/tmp` that accumulate indefinitely. diff --git a/caldav/async_davclient.py b/caldav/async_davclient.py index 2c4bd006..8b671dd4 100644 --- a/caldav/async_davclient.py +++ b/caldav/async_davclient.py @@ -7,6 +7,7 @@ """ import asyncio +import inspect import logging import sys from collections.abc import Mapping @@ -163,6 +164,9 @@ def __init__( features: FeatureSet for server compatibility workarounds. enable_rfc6764: Enable RFC6764 DNS-based service discovery. require_tls: Require TLS for discovered services (security consideration). + Only gates the RFC6764 discovery path; it does NOT reject an + explicitly-passed http:// URL. Global enforcement is deferred to + 4.0 — see https://github.com/python-caldav/caldav/issues/687 rate_limit_handle: When True, automatically sleep and retry on 429/503 responses. When None (default), auto-detected from server features. When False, raise RateLimitError immediately. @@ -223,18 +227,21 @@ def __init__( # Parse and store URL self.url = URL.objectify(url_str) - # Extract auth from URL if present - url_username = None - url_password = None - if self.url.username: - url_username = unquote(self.url.username) - if self.url.password: - url_password = unquote(self.url.password) - - # Combine credentials (explicit params take precedence) - # Use explicit None check to preserve empty strings (needed for servers with no auth) - self.username = username if username is not None else url_username - self.password = password if password is not None else url_password + # Combine credentials (explicit params take precedence). + # An explicit username discards the URL credentials wholesale: they + # belong to a different account, and merging them field by field would + # let AsyncDAVClient(url="https://bob:hunter2@cal.example.com/", + # username="alice") ship alice's login with bob's password. Overriding + # only the password is a different thing - the username still comes + # from the URL, so the pair stays coherent. + # Use explicit None checks to preserve empty strings (needed for + # servers with no auth). + if self.url.username and username is None: + username = unquote(self.url.username) + if password is None and self.url.password: + password = unquote(self.url.password) + self.username = username + self.password = password # Strip credentials from stored URL to avoid leaking them in log messages self.url = self.url.unauth() @@ -258,19 +265,9 @@ def __init__( } self.headers.update(headers) - rate_limit = self.features.is_supported("rate-limit", dict) - if rate_limit_handle is None: - if rate_limit and rate_limit.get("enable"): - rate_limit_handle = True - if "default_sleep" in rate_limit: - rate_limit_default_sleep = rate_limit["default_sleep"] - if "max_sleep" in rate_limit: - rate_limit_max_sleep = rate_limit["max_sleep"] - else: - rate_limit_handle = False - self.rate_limit_handle = rate_limit_handle - self.rate_limit_default_sleep = rate_limit_default_sleep - self.rate_limit_max_sleep = rate_limit_max_sleep + self._init_rate_limit_config( + rate_limit_handle, rate_limit_default_sleep, rate_limit_max_sleep + ) def _create_session(self) -> None: """Create or recreate the async HTTP client with current settings.""" @@ -365,20 +362,7 @@ async def request( try: return await self._async_request(url, method, body, headers) except error.RateLimitError as e: - if not self.rate_limit_handle: - raise - sleep_seconds = error.compute_sleep_seconds( - e.retry_after_seconds, - self.rate_limit_default_sleep, - self.rate_limit_max_sleep, - ) - if rate_limit_time_slept: - sleep_seconds += rate_limit_time_slept / 2 - if sleep_seconds is None or ( - self.rate_limit_max_sleep is not None - and rate_limit_time_slept > self.rate_limit_max_sleep - ): - raise + sleep_seconds = self._rate_limit_sleep_seconds(e, rate_limit_time_slept) await asyncio.sleep(sleep_seconds) return await self.request( url, method, body, headers, rate_limit_time_slept + sleep_seconds @@ -432,7 +416,7 @@ async def _async_request( log.debug(f"server responded with {r.status_code} {reason}") if ( r.status_code == 401 - and "text/html" in self.headers.get("Content-Type", "") + and "text/html" in r.headers.get("Content-Type", "") and not self.auth ): msg = ( @@ -484,7 +468,11 @@ async def _async_request( # Retry original request with auth request_kwargs["auth"] = self.auth r = await self.session.request(**request_kwargs) - response = DAVResponse(r, self) + response = DAVResponse(r, self) + else: + # Probe GET did not give us a 401+WWW-Authenticate challenge — + # auth negotiation failed; re-raise the original connection error + raise # Handle 429/503 rate-limit responses error.raise_if_rate_limited(r.status_code, str(url_obj), r.headers.get("Retry-After")) @@ -936,14 +924,6 @@ async def get_calendars(self, principal: Optional["Principal"] = None) -> list[" for cal in calendars: print(f"Calendar: {cal.get_display_name()}") """ - from caldav.collection import Calendar - from caldav.collection import ( - _extract_calendar_home_set_from_results as extract_home_set, - ) - from caldav.collection import ( - _extract_calendars_from_propfind_results as extract_calendars, - ) - if principal is None: principal = await self.get_principal() @@ -953,12 +933,7 @@ async def get_calendars(self, principal: Optional["Principal"] = None) -> list[" props=self.CALENDAR_HOME_SET_PROPS, depth=0, ) - calendar_home_url = extract_home_set(response.results) - if not calendar_home_url: - return [] - - # Make URL absolute if relative - calendar_home_url = self._make_absolute_url(calendar_home_url) + calendar_home_url = self._calendar_home_url(response, principal) # Fetch calendars via PROPFIND response = await self.propfind( @@ -967,14 +942,7 @@ async def get_calendars(self, principal: Optional["Principal"] = None) -> list[" depth=1, ) - # Process results using shared helper - calendar_infos = extract_calendars(response.results) - - # Convert CalendarInfo objects to Calendar objects - return [ - Calendar(client=self, url=info.url, name=info.name, id=info.cal_id) - for info in calendar_infos - ] + return self._build_calendars_from_propfind(response) async def search_calendar( self, @@ -1222,7 +1190,11 @@ async def get_calendars( for cal in calendars: print(await cal.get_display_name()) """ - from caldav.base_client import CalendarCollection, _normalize_to_list + from caldav.base_client import ( + CalendarCollection, + _normalize_to_list, + _warn_unreadable_display_name, + ) def _try(coro_result, errmsg): """Handle errors based on raise_errors flag.""" @@ -1256,6 +1228,12 @@ def _try(coro_result, errmsg): calendar = principal.calendar(cal_url=cal_url) else: calendar = principal.calendar(cal_id=cal_url) + ## A bare cal_id has to resolve the calendar home set first, so + ## principal.calendar() hands back a coroutine here. Without the + ## await the AttributeError below was caught by the broad except + ## and the calendar was silently dropped from the collection. + if inspect.isawaitable(calendar): + calendar = await calendar try: display_name = await calendar.get_display_name() @@ -1267,13 +1245,31 @@ def _try(coro_result, errmsg): raise # Fetch specific calendars by name - for cal_name in calendar_names: + if calendar_names: try: - calendar = await principal.calendar(name=cal_name) - if calendar: - calendars.append(calendar) + all_cals_for_name = await principal.get_calendars() + for cal_name in calendar_names: + for cal in all_cals_for_name: + try: + display_name = await cal.get_display_name() + if display_name == cal_name: + calendars.append(cal) + break + except Exception as e: + # Skip calendars whose display name can't be read; warn + # only when the failure is unexpected (see helper). + # Continuing ensures one unreadable calendar doesn't abort + # the whole name lookup. + _warn_unreadable_display_name(client, cal, cal_name, e) + continue + else: + log.error(f"No calendar with name '{cal_name}' found") + if raise_errors: + raise error.NotFoundError(f"No calendar with name '{cal_name}' found") + except error.NotFoundError: + raise except Exception as e: - log.error(f"Problems fetching calendar by name '{cal_name}': {e}") + log.error(f"Problems fetching calendars by name: {e}") if raise_errors: raise diff --git a/caldav/base_client.py b/caldav/base_client.py index d1974bb7..5186a3c7 100644 --- a/caldav/base_client.py +++ b/caldav/base_client.py @@ -255,6 +255,99 @@ def _raise_authorization_error(self, url_str: str, reason_source: Any) -> NoRetu reason = "None given" raise error.AuthorizationError(url=url_str, reason=reason) + # ── Rate-limit handling ───────────────────────────────────────────────── + # Shared by the sync (DAVClient) and async (AsyncDAVClient) __init__ and + # request() retry loops, which are otherwise byte-identical apart from + # time.sleep vs asyncio.sleep. + + def _init_rate_limit_config( + self, + rate_limit_handle: bool | None, + rate_limit_default_sleep: int | None, + rate_limit_max_sleep: int | None, + ) -> None: + """Resolve and store rate-limit settings on self. + + When ``rate_limit_handle`` is None it is auto-detected from the + ``rate-limit`` feature; an enabled feature may also supply default and + max sleep durations (explicit constructor arguments are not overridden + because the feature values only fill in the auto-detected branch). + """ + rate_limit = self.features.is_supported("rate-limit", dict) + if rate_limit_handle is None: + if rate_limit and rate_limit.get("enable"): + rate_limit_handle = True + if "default_sleep" in rate_limit: + rate_limit_default_sleep = rate_limit["default_sleep"] + if "max_sleep" in rate_limit: + rate_limit_max_sleep = rate_limit["max_sleep"] + else: + rate_limit_handle = False + self.rate_limit_handle = rate_limit_handle + self.rate_limit_default_sleep = rate_limit_default_sleep + self.rate_limit_max_sleep = rate_limit_max_sleep + + def _rate_limit_sleep_seconds( + self, + exc: error.RateLimitError, + rate_limit_time_slept: float, + ) -> float: + """Decide how long to sleep before retrying a rate-limited request. + + Returns the sleep duration in seconds. Re-raises ``exc`` when no retry + should happen (rate-limit handling disabled, no usable duration, or the + accumulated sleep already exceeds ``rate_limit_max_sleep``). The caller + is responsible for actually sleeping (time.sleep vs asyncio.sleep) and + retrying with ``rate_limit_time_slept + ``. + """ + if not self.rate_limit_handle: + raise exc + sleep_seconds = error.compute_sleep_seconds( + exc.retry_after_seconds, + self.rate_limit_default_sleep, + self.rate_limit_max_sleep, + ) + if sleep_seconds is None or ( + self.rate_limit_max_sleep is not None + and rate_limit_time_slept > self.rate_limit_max_sleep + ): + raise exc + if rate_limit_time_slept: + sleep_seconds += rate_limit_time_slept / 2 + return sleep_seconds + + # ── Calendar discovery post-processing ────────────────────────────────── + # Pure result-handling shared by sync/async get_calendars; only the two + # awaited PROPFIND calls and the principal lookup differ between the twins. + + def _calendar_home_url(self, home_set_response: Any, principal: Any) -> str: + """Extract the calendar-home-set URL from a PROPFIND response. + + Falls back to the principal URL when the server does not advertise a + calendar-home-set (e.g. GMX), then makes the result absolute. + """ + from caldav.collection import ( + _extract_calendar_home_set_from_results as extract_home_set, + ) + + calendar_home_url = extract_home_set(home_set_response.results) + if not calendar_home_url: + calendar_home_url = str(principal.url) + return self._make_absolute_url(calendar_home_url) + + def _build_calendars_from_propfind(self, list_response: Any) -> list: + """Build Calendar objects from a calendar-home PROPFIND response.""" + from caldav.collection import Calendar + from caldav.collection import ( + _extract_calendars_from_propfind_results as extract_calendars, + ) + + calendar_infos = extract_calendars(list_response.results) + return [ + Calendar(client=self, url=info.url, name=info.name, id=info.cal_id) + for info in calendar_infos + ] + # ── XML builders ────────────────────────────────────────────────────────── # All methods are static: no I/O, no server interaction, pure data # transformation. Both DAVClient and AsyncDAVClient inherit these so @@ -645,6 +738,32 @@ def _normalize_to_list(obj: Any) -> list: return list(obj) +def _warn_unreadable_display_name(client: Any, calendar: Any, name: Any, exc: Exception) -> None: + """Log a warning when a calendar's display name couldn't be read during a + lookup by name -- unless the failure is expected per the compatibility matrix. + + Shared by the sync (:meth:`caldav.collection.CalendarSet.calendar`) and async + (:func:`caldav.async_davclient.get_calendars`) name-matching loops so the + warn-or-suppress decision lives in one place. + + The failure is treated as expected (and silently skipped) only when we + positively know the server doesn't support reading the DAV:displayname + property via PROPFIND (``propfind.displayname`` non-supported -- which falls + back to the ``propfind`` parent when not probed explicitly). When the + feature is supported, or when we have no feature matrix to consult, the + failure is unexpected and is warned about. + + The caller is responsible for continuing the loop afterwards, so that one + unreadable calendar never aborts the whole name lookup. + """ + features = getattr(client, "features", None) + if features is None or features.is_supported("propfind.displayname"): + log.warning( + f"Could not read display name for calendar " + f"{getattr(calendar, 'url', calendar)} while matching name '{name}': {exc}" + ) + + def _fetch_calendars_for_client( client: Any, calendar_url: Any | None, @@ -686,7 +805,7 @@ def _try(meth, kwargs, errmsg): calendar = principal.calendar(cal_url=cal_url) else: calendar = principal.calendar(cal_id=cal_url) - if _try(calendar.get_display_name, {}, f"calendar {cal_url}"): + if _try(calendar.get_display_name, {}, f"calendar {cal_url}") is not None: calendars.append(calendar) for cal_name in calendar_names: diff --git a/caldav/calendarobjectresource.py b/caldav/calendarobjectresource.py index a0f33976..9362a7e9 100644 --- a/caldav/calendarobjectresource.py +++ b/caldav/calendarobjectresource.py @@ -722,7 +722,7 @@ def add_attendee(self, attendee, no_default_parameters: bool = False, **paramete raise NotImplementedError( "do we need to support this anyway? Should be trivial, but can't figure out how to do it with the icalendar.Event/vCalAddress objects right now" ) - elif attendee.startswith("mailto:"): + elif attendee.lower().startswith("mailto:"): attendee_obj = vCalAddress(attendee) elif "@" in attendee and ":" not in attendee and ";" not in attendee: attendee_obj = vCalAddress("mailto:" + attendee) @@ -1000,11 +1000,7 @@ def load(self, only_if_unloaded: bool = False) -> "Self | Coroutine[Any, Any, Se except Exception: return self.load_by_multiget() - ## consider refactoring - this is repeated many places now - if "Etag" in r.headers: - self.props[dav.GetEtag.tag] = r.headers["Etag"] - if "Schedule-Tag" in r.headers: - self.props[cdav.ScheduleTag.tag] = r.headers["Schedule-Tag"] + self._update_tag_props(r) return self async def _async_load(self, only_if_unloaded: bool = False) -> Self: @@ -1046,10 +1042,7 @@ async def _async_load(self, only_if_unloaded: bool = False) -> Self: except Exception: return await self.load_by_multiget() - if "Etag" in r.headers: - self.props[dav.GetEtag.tag] = r.headers["Etag"] - if "Schedule-Tag" in r.headers: - self.props[cdav.ScheduleTag.tag] = r.headers["Schedule-Tag"] + self._update_tag_props(r) return self def load_by_multiget(self) -> "Self | Coroutine[Any, Any, Self]": @@ -1155,6 +1148,20 @@ async def _async_put(self, headers, retry_on_failure=True): # _post_put returned a retry coroutine (self._put(False) for async client) await result + def _update_tag_props(self, r) -> None: + """Capture the ETag / Schedule-Tag response headers into self.props. + + Called after both PUT (`_post_put`) and GET (`load`/`_async_load`); + keys are matched case-insensitively by the response header dict. + See RFC 6638 for Schedule-Tag. + """ + if not r.headers: + return + if "Etag" in r.headers: + self.props[dav.GetEtag.tag] = r.headers["Etag"] + if r.headers.get("Schedule-Tag"): + self.props[cdav.ScheduleTag.tag] = r.headers["Schedule-Tag"] + def _post_put(self, r, retry_on_failure): if r.status == 412: if self.schedule_tag: @@ -1164,7 +1171,7 @@ def _post_put(self, r, retry_on_failure): else: raise error.PutError(errmsg(r)) elif r.status == 302: - self.url = URL.objectify([x[1] for x in r.headers if x[0] == "location"][0]) + self.url = URL.objectify(r.headers.get("location")) elif r.status not in (204, 201): if retry_on_failure: try: @@ -1178,31 +1185,7 @@ def _post_put(self, r, retry_on_failure): return self._put(False) else: raise error.PutError(errmsg(r)) - if "Etag" in r.headers: - self.props[dav.GetEtag.tag] = r.headers["Etag"] - if r.headers and r.headers.get("schedule-tag"): - self.props[cdav.ScheduleTag.tag] = r.headers["schedule-tag"] - - if r.status == 302: - path = [x[1] for x in r.headers if x[0] == "location"][0] - self.url = URL.objectify(path) - elif r.status not in (204, 201): - if retry_on_failure: - try: - import vobject # noqa: F401 - except ImportError: - retry_on_failure = False - if retry_on_failure: - ## This seems like a noop, but it may "wash" the object - dummy = self.vobject_instance - return self._put(False) - else: - raise error.PutError(errmsg(r)) - ## TODO: refactor - those code lines are repeated all over the place - if "Etag" in r.headers: - self.props[dav.GetEtag.tag] = r.headers["Etag"] - if r.headers and r.headers.get("schedule-tag"): - self.props[cdav.ScheduleTag.tag] = r.headers["schedule-tag"] + self._update_tag_props(r) def _create( self, id=None, path=None, retry_on_failure=True @@ -1269,7 +1252,12 @@ def change_attendee_status(self, attendee: Any | None = None, **kwargs) -> None: return ical_obj = self.icalendar_component - attendee_lines = ical_obj["attendee"] + try: + attendee_lines = ical_obj["attendee"] + except KeyError: + raise error.NotFoundError( + f"Participant {attendee!r} not found in attendee list (no ATTENDEE properties)" + ) from None if isinstance(attendee_lines, str): attendee_lines = [attendee_lines] @@ -1281,7 +1269,7 @@ def strip_mailto(x): attendee_line.params.update(kwargs) cnt += 1 if not cnt: - raise error.NotFoundError("Participant %s not found in attendee list") + raise error.NotFoundError(f"Participant {attendee!r} not found in attendee list") error.assert_(cnt == 1) def save( @@ -1570,6 +1558,7 @@ def _set_data(self, data): self._data = vcal.fix(data) self._vobject_instance = None self._icalendar_instance = None + self._state = RawDataState(self._data) return self def _get_data(self): @@ -1940,7 +1929,7 @@ def _get_duration(self, i): start = datetime(start.year, start.month, start.day) end = datetime(end.year, end.month, end.day) return end - start - elif "DTSTART" in i and not isinstance(i["DTSTART"], datetime): + elif "DTSTART" in i and not isinstance(i["DTSTART"].dt, datetime): return timedelta(days=1) else: return timedelta(0) @@ -2119,55 +2108,53 @@ def _reduce_count(self, i=None) -> bool: i["RRULE"]["COUNT"][0] -= 1 return True - def _complete_recurring_safe(self, completion_timestamp): - """This mode will create a new independent task which is - marked as completed, and modify the existing recurring task. - It is probably the most safe way to handle the completion of a - recurrence of a recurring task, though the link between the - completed task and the original task is lost. + def _build_recurring_safe_completed(self, completion_timestamp) -> "Todo | None": + """Pure (no-I/O) part of the "safe" recurring-completion strategy. + + Advances ``self`` to its next occurrence in memory and returns a + freshly-built standalone copy marked as completed. Returns + ``None`` when the task is not (or no longer) recurring, in which + case the caller should fall back to a plain completion. The + caller is responsible for saving both ``self`` and the returned + copy (one PUT each). """ ## If count is one, then it is not really recurring if not self._reduce_count(): - return self.complete(handle_rrule=False) + return None next_dtstart = self._next(completion_timestamp) if not next_dtstart: - return self.complete(handle_rrule=False) + return None completed = self.copy() completed.url = self.parent.url.join(completed.id + ".ics") completed.icalendar_component.pop("RRULE") - completed.save() - completed.complete() + completed._complete_ical(completion_timestamp=completion_timestamp) duration = self.get_duration() i = self.icalendar_component i.pop("DTSTART", None) i.add("DTSTART", next_dtstart) self.set_duration(duration, movable_attr="DUE") + return completed + def _complete_recurring_safe(self, completion_timestamp): + """This mode will create a new independent task which is + marked as completed, and modify the existing recurring task. + It is probably the most safe way to handle the completion of a + recurrence of a recurring task, though the link between the + completed task and the original task is lost. + """ + completed = self._build_recurring_safe_completed(completion_timestamp) + if completed is None: + return self.complete(handle_rrule=False) + completed.save() self.save() - def _complete_recurring_thisandfuture(self, completion_timestamp) -> None: - """The RFC is not much helpful, a lot of guesswork is needed - to consider what the "right thing" to do wrg of a completion of - recurring tasks is ... but this is my shot at it. - - 1) The original, with rrule, will be kept as it is. The rrule - string is fetched from the first subcomponent of the - icalendar. - - 2) If there are multiple recurrence instances in subcomponents - and the last one is marked with RANGE=THISANDFUTURE, then - select this one. If it has the rrule property set, use this - rrule rather than the original one. Drop the RANGE parameter. - Calculate the next RECURRENCE-ID from the DTSTART of this - object. Mark task as completed. Increase SEQUENCE. - - 3) Create a new recurrence instance with RANGE=THISANDFUTURE, - without RRULE set (Ref - https://github.com/Kozea/Radicale/issues/1264). Set the - RECURRENCE-ID to the one calculated in #2. Calculate the - DTSTART based on rrule and completion timestamp/date. + def _prepare_recurring_thisandfuture(self, completion_timestamp) -> None: + """Pure (no-I/O) in-memory mutation behind + ``_complete_recurring_thisandfuture``; see that method for the + algorithm description. The caller does the single + ``save(increase_seqno=False)`` that follows. """ recurrences = self.icalendar_instance.subcomponents orig = recurrences[0] @@ -2219,7 +2206,6 @@ def _complete_recurring_thisandfuture(self, completion_timestamp) -> None: [x for x in recurrences if not self.is_pending(x)] ): self._complete_ical(recurrences[0], completion_timestamp=completion_timestamp) - self.save(increase_seqno=False) return rrule = rrule2 or rrule @@ -2231,6 +2217,30 @@ def _complete_recurring_thisandfuture(self, completion_timestamp) -> None: thisandfuture.add("DTSTART", next_dtstart) self._set_duration(i=thisandfuture, duration=duration, movable_attr="DUE") self.icalendar_instance.subcomponents.append(thisandfuture) + + def _complete_recurring_thisandfuture(self, completion_timestamp) -> None: + """The RFC is not much helpful, a lot of guesswork is needed + to consider what the "right thing" to do wrg of a completion of + recurring tasks is ... but this is my shot at it. + + 1) The original, with rrule, will be kept as it is. The rrule + string is fetched from the first subcomponent of the + icalendar. + + 2) If there are multiple recurrence instances in subcomponents + and the last one is marked with RANGE=THISANDFUTURE, then + select this one. If it has the rrule property set, use this + rrule rather than the original one. Drop the RANGE parameter. + Calculate the next RECURRENCE-ID from the DTSTART of this + object. Mark task as completed. Increase SEQUENCE. + + 3) Create a new recurrence instance with RANGE=THISANDFUTURE, + without RRULE set (Ref + https://github.com/Kozea/Radicale/issues/1264). Set the + RECURRENCE-ID to the one calculated in #2. Calculate the + DTSTART based on rrule and completion timestamp/date. + """ + self._prepare_recurring_thisandfuture(completion_timestamp) self.save(increase_seqno=False) def complete( @@ -2284,85 +2294,15 @@ async def _async_complete( async def _async_complete_recurring_safe(self, completion_timestamp: datetime) -> None: """Async version of _complete_recurring_safe.""" - if not self._reduce_count(): + completed = self._build_recurring_safe_completed(completion_timestamp) + if completed is None: return await self._async_complete(completion_timestamp, handle_rrule=False) - next_dtstart = self._next(completion_timestamp) - if not next_dtstart: - return await self._async_complete(completion_timestamp, handle_rrule=False) - - completed = self.copy() - completed.url = self.parent.url.join(completed.id + ".ics") - completed.icalendar_component.pop("RRULE") - await completed.save() - completed._complete_ical(completion_timestamp=completion_timestamp) await completed.save() - - duration = self.get_duration() - i = self.icalendar_component - i.pop("DTSTART", None) - i.add("DTSTART", next_dtstart) - self.set_duration(duration, movable_attr="DUE") await self.save() async def _async_complete_recurring_thisandfuture(self, completion_timestamp: datetime) -> None: """Async version of _complete_recurring_thisandfuture.""" - recurrences = self.icalendar_instance.subcomponents - orig = recurrences[0] - if "STATUS" not in orig: - orig["STATUS"] = "NEEDS-ACTION" - - if len(recurrences) == 1: - just_completed = orig.copy() - just_completed.pop("RRULE") - just_completed.add("RECURRENCE-ID", orig.get("DTSTART", completion_timestamp)) - seqno = just_completed.pop("SEQUENCE", 0) - just_completed.add("SEQUENCE", seqno + 1) - recurrences.append(just_completed) - - prev = recurrences[-1] - rrule = prev.get("RRULE", orig["RRULE"]) - thisandfuture = prev.copy() - seqno = thisandfuture.pop("SEQUENCE", 0) - thisandfuture.add("SEQUENCE", seqno + 1) - - if len(recurrences) > 2: - if prev["RECURRENCE-ID"].params.get("RANGE", None) == "THISANDFUTURE": - prev["RECURRENCE-ID"].params.pop("RANGE") - else: - raise NotImplementedError( - "multiple instances found, but last one is not of type THISANDFUTURE, possibly this has been created by some incompatible client, but we should deal with it" - ) - self._complete_ical(prev, completion_timestamp) - - thisandfuture.pop("RECURRENCE-ID", None) - thisandfuture.add("RECURRENCE-ID", self._next(i=prev, rrule=rrule)) - thisandfuture["RECURRENCE-ID"].params["RANGE"] = "THISANDFUTURE" - rrule2 = thisandfuture.pop("RRULE", None) - - if rrule2 is not None: - count = rrule2.get("COUNT", None) - if count is not None and count[0] in (0, 1): - for i in recurrences: - self._complete_ical(i, completion_timestamp=completion_timestamp) - thisandfuture.add("RRULE", rrule2) - else: - count = rrule.get("COUNT", None) - if count is not None and count[0] <= len( - [x for x in recurrences if not self.is_pending(x)] - ): - self._complete_ical(recurrences[0], completion_timestamp=completion_timestamp) - await self.save(increase_seqno=False) - return - - rrule = rrule2 or rrule - - duration = self._get_duration(i=prev) - thisandfuture.pop("DTSTART", None) - thisandfuture.pop("DUE", None) - next_dtstart = self._next(i=prev, rrule=rrule, ts=completion_timestamp) - thisandfuture.add("DTSTART", next_dtstart) - self._set_duration(i=thisandfuture, duration=duration, movable_attr="DUE") - self.icalendar_instance.subcomponents.append(thisandfuture) + self._prepare_recurring_thisandfuture(completion_timestamp) await self.save(increase_seqno=False) def _complete_ical(self, i=None, completion_timestamp=None) -> None: diff --git a/caldav/collection.py b/caldav/collection.py index 37b87b96..7455dd2a 100644 --- a/caldav/collection.py +++ b/caldav/collection.py @@ -10,6 +10,7 @@ A SynchronizableCalendarObjectCollection contains a local copy of objects from a calendar on the server. """ +import inspect import logging import uuid import warnings @@ -31,7 +32,7 @@ from collections.abc import Coroutine, Iterable, Iterator, Sequence from typing import Literal -from .base_client import ICALH +from .base_client import ICALH, _warn_unreadable_display_name from .calendarobjectresource import ( CalendarObjectResource, Event, @@ -78,6 +79,19 @@ def _extract_calendar_id_from_url(url: str) -> str | None: return None +def _safe_display_name(cal) -> str | None: + """Return ``cal``'s DAV:displayname, or None if it can't be read. + + Used when discovering a relocated calendar's canonical URL after creation + (see Calendar._adopt_canonical_url); a calendar that refuses to report its + display name simply isn't a match. + """ + try: + return cal.get_display_name() + except Exception: + return None + + def _quote_url_path(url: str) -> str: """Quote the path component of a URL to handle unencoded spaces (e.g. Zimbra).""" parsed = urlparse(url) @@ -184,6 +198,43 @@ def calendars(self) -> "list[Calendar] | Coroutine[Any, Any, list[Calendar]]": """ return self.get_calendars() + def _find_calendar_by_name( + self, calendars: "list[tuple[Calendar, str | None]]", name: str + ) -> "Calendar": + """Pick the calendar whose display name is ``name``. + + The display names are supplied already resolved, so that the sync and + async twins of :meth:`calendar` can share the matching and the error. + """ + for calendar, display_name in calendars: + if display_name == name: + return calendar + raise error.NotFoundError(f"No calendar with name {name} found under {self.url}") + + def _first_calendar(self, calendars: "list[Calendar]") -> "Calendar": + """Return the first calendar, or raise if there are none.""" + if not calendars: + raise error.NotFoundError("no calendars found") + return calendars[0] + + async def _async_calendar(self, name: str | None = None) -> "Calendar": + """Async twin of :meth:`calendar` for the lookups that need the server.""" + calendars = await self.get_calendars() + if not name: + return self._first_calendar(calendars) + named = [] + for calendar in calendars: + try: + display_name = await calendar.get_display_name() + except Exception as e: + ## Skip calendars whose display name can't be read; warn only + ## when the failure is unexpected (see helper). Continuing + ## ensures one unreadable calendar doesn't abort the lookup. + _warn_unreadable_display_name(self.client, calendar, name, e) + continue + named.append((calendar, display_name)) + return self._find_calendar_by_name(named, name) + def make_calendar( self, name: str | None = None, @@ -236,7 +287,9 @@ async def _async_make_calendar( ) return await calendar.save(method=method) - def calendar(self, name: str | None = None, cal_id: str | None = None) -> "Calendar": + def calendar( + self, name: str | None = None, cal_id: str | None = None + ) -> "Calendar | Coroutine[Any, Any, Calendar]": """ The calendar method will return a calendar object. If it gets a cal_id but no name, it will not initiate any communication with the server @@ -246,21 +299,31 @@ def calendar(self, name: str | None = None, cal_id: str | None = None) -> "Calen cal_id: return the calendar with this calendar id or URL Returns: - Calendar(...)-object - """ - # For name-based lookup, use calendars() which already uses async delegation + Calendar(...)-object. A lookup by ``name``, or with neither + argument, has to list the calendars on the server, so on an async + client it returns a coroutine that must be awaited. + """ + ## A lookup by name (or with no arguments at all) lists the calendars + ## and reads their display names - round-trips, so the async client + ## needs its own path. A cal_id is pure URL arithmetic and stays + ## synchronous for both. + if not cal_id and self.is_async_client: + return self._async_calendar(name) if name and not cal_id: + named = [] for calendar in self.get_calendars(): - display_name = calendar.get_display_name() - if display_name == name: - return calendar - if name and not cal_id: - raise error.NotFoundError(f"No calendar with name {name} found under {self.url}") + try: + display_name = calendar.get_display_name() + except Exception as e: + ## Skip calendars whose display name can't be read; warn only + ## when the failure is unexpected (see helper). Continuing + ## ensures one unreadable calendar doesn't abort the lookup. + _warn_unreadable_display_name(self.client, calendar, name, e) + continue + named.append((calendar, display_name)) + return self._find_calendar_by_name(named, name) if not cal_id and not name: - cals = self.get_calendars() - if not cals: - raise error.NotFoundError("no calendars found") - return cals[0] + return self._first_calendar(self.get_calendars()) if self.client is None: raise ValueError("Unexpected value None for self.client") @@ -450,10 +513,15 @@ def calendar( name: str | None = None, cal_id: str | None = None, cal_url: str | None = None, - ) -> "Calendar": + ) -> "Calendar | Coroutine[Any, Any, Calendar]": """ The calendar method will return a calendar object. - It will not initiate any communication with the server. + + For a full-URL ``cal_id`` or a ``cal_url`` it does not initiate any + communication with the server and returns the Calendar directly (also + for async clients). For a bare ``cal_id``/``name`` it needs the + calendar home set, which on an async client is resolved with a PROPFIND; + in that case it returns a coroutine that must be awaited. """ if not cal_url: ## For full-URL cal_id, skip calendar_home_set (which may be async-lazy) @@ -467,6 +535,11 @@ def calendar( if self.client is None: raise ValueError("Unexpected value None for self.client") return Calendar(self.client, url=URL.objectify(cal_id)) + ## A bare cal_id/name needs the calendar home set. On async clients + ## that resolution awaits a PROPFIND, so we must hand back a coroutine + ## rather than evaluating the (lazy, coroutine-valued) home set here. + if self.is_async_client: + return self._async_calendar(name, cal_id) return self.calendar_home_set.calendar(name, cal_id) else: if self.client is None: @@ -474,6 +547,20 @@ def calendar( return Calendar(self.client, url=self.client.url.join(cal_url)) + async def _async_calendar( + self, + name: str | None = None, + cal_id: str | None = None, + ) -> "Calendar": + """Async implementation of calendar() for a bare cal_id/name.""" + calendar_home_set = await self._async_get_calendar_home_set() + calendar = calendar_home_set.calendar(name, cal_id) + ## A bare cal_id resolves synchronously even on an async client; a + ## name lookup hands back a coroutine. + if inspect.isawaitable(calendar): + calendar = await calendar + return calendar + def get_vcal_address(self) -> "vCalAddress | Coroutine[Any, Any, vCalAddress]": """ Returns the principal, as an icalendar.vCalAddress object. @@ -598,22 +685,27 @@ def freebusy_request( freebusy_ical.add_component(freebusy_comp) outbox = self.schedule_outbox() caldavobj = FreeBusy(data=freebusy_ical, parent=self) - for attendee in attendees: - caldavobj.add_attendee(attendee, no_default_parameters=True) if self.is_async_client: - return self._async_freebusy_request(outbox, caldavobj) + return self._async_freebusy_request(outbox, caldavobj, attendees) + + for attendee in attendees: + caldavobj.add_attendee(attendee, no_default_parameters=True) caldavobj.add_organizer() response = self.client.post(outbox.url, caldavobj.data, headers=ICALH) return response._parse_scheduling_response_objects(parent=self) - async def _async_freebusy_request(self, outbox, fb_obj) -> dict: + async def _async_freebusy_request(self, outbox, fb_obj, attendees) -> dict: """Async implementation of freebusy_request() for async clients.""" ## TODO: could we have common headers as global variable? headers = ICALH outbox = await outbox + for attendee in attendees: + if isinstance(attendee, Principal): + attendee = await attendee.get_vcal_address() + fb_obj.add_attendee(attendee, no_default_parameters=True) ## TODO: it's really bad that arbitrary methods returns ## a coroutine in async mode. It's needed to make it much ## more clear what methods involves I/O and what methods @@ -747,16 +839,38 @@ def _create( prop = dav.Prop() display_name = None - # Some servers (e.g. Zimbra) use the DisplayName from the MKCALENDAR body - # as the calendar URL, ignoring the actual request path. When the server - # does not support setting a separate display name, omit it from the body so - # the request URL path is used as the calendar identifier. supports_displayname = not self.client or self.client.features.is_supported( "create-calendar.set-displayname" ) + stable_url = not self.client or self.client.features.is_supported( + "create-calendar.stable-url" + ) + # A few servers assign a calendar a canonical URL that differs from the + # requested cal_id when a display name is set: Zimbra relocates the + # collection to a display-name-derived path (a collection-level alias + # lingers at the cal_id and answers PROPFIND/REPORT, but a GET on a child + # object under it 404s, so the cal_id is not a usable address), while OX + # always exposes an opaque cal://0/NNN canonical URL. We still send the + # display name (it sticks); afterwards, for such servers + # (create-calendar.stable-url unsupported), we DISCOVER and ADOPT the + # canonical URL (see _adopt_canonical_url) so that self.url - and every + # later URL-based operation - points at the address that actually + # resolves. This replaces the older "drop the display name" workaround + # and behaves identically for Zimbra and OX. We only omit the display + # name when the server cannot set one at creation at all + # (create-calendar.set-displayname unsupported). if name and supports_displayname: display_name = dav.DisplayName(name) prop += [display_name] + elif name: # not supports_displayname + log.warning( + "Creating calendar %r without the requested display name %r: the " + "server does not support setting a display name when a calendar is " + "created (create-calendar.set-displayname). The calendar keeps its " + "requested URL but will have no display name.", + id, + name, + ) if supported_calendar_component_set: sccs = cdav.SupportedCalendarComponentSet() for scc in supported_calendar_component_set: @@ -769,7 +883,7 @@ def _create( mkcol = (dav.Mkcol() if method == "mkcol" else cdav.Mkcalendar()) + set if self.is_async_client: - return self._async_create(path, mkcol, method, name, display_name) + return self._async_create(path, mkcol, method, name, display_name, stable_url) self._query(root=mkcol, query_method=method, url=path, expected_return_value=201) @@ -792,7 +906,84 @@ def _create( exc_info=True, ) - async def _async_create(self, path, mkcol, method, name, display_name) -> None: + # On servers that don't keep the calendar at the requested cal_id when a + # display name is set (create-calendar.stable-url unsupported), re-point + # self.url to the canonical URL the server actually assigned. + if display_name and not stable_url: + self._adopt_canonical_url(name) + + def _adopt_canonical_url(self, name) -> None: + """Re-point ``self.url`` to the server's canonical URL for this calendar. + + Called only for servers where ``create-calendar.stable-url`` is + unsupported: the calendar just created is reachable under a canonical URL + that differs from the requested cal_id (Zimbra: a display-name-derived + path; OX: an opaque ``cal://0/NNN`` segment). The requested cal_id is not + a reliable address there (on Zimbra a collection alias answers + PROPFIND/REPORT but a GET on a child object 404s). We locate the calendar + by the display name we just set and adopt its URL so later URL-based + operations resolve. + + Best effort: if the calendar can't be located (or its name is ambiguous + because another calendar already shares it), ``self.url`` is left at the + requested URL. + """ + requested = self.url.canonical() + try: + relocated = [ + cal.url + for cal in self.parent.calendars() + if _safe_display_name(cal) == name and cal.url.canonical() != requested + ] + except Exception: + log.warning("Could not list calendars to discover canonical URL", exc_info=True) + return + self._adopt_relocated_url(name, relocated) + + async def _async_adopt_canonical_url(self, name) -> None: + """Async twin of :meth:`_adopt_canonical_url`.""" + try: + cals = await self.parent.calendars() + except Exception: + log.warning("Could not list calendars to discover canonical URL (async)", exc_info=True) + return + requested = self.url.canonical() + relocated = [] + for cal in cals: + try: + display_name = await cal.get_display_name() + except Exception: + ## a calendar that refuses to report its display name is not a match + continue + if display_name == name and cal.url.canonical() != requested: + relocated.append(cal.url) + self._adopt_relocated_url(name, relocated) + + def _adopt_relocated_url(self, name, relocated: list) -> None: + """Adopt the one relocated URL found, or keep the requested one. + + Shared by :meth:`_adopt_canonical_url` and its async twin. An + ambiguous display name is deliberately *not* resolved by picking the + first candidate: the other candidate is typically a pre-existing, + unrelated calendar, and adopting it would send every later + ``add_event()``, ``search()`` and ``delete()`` to the wrong calendar + while orphaning the one just created. + """ + if not relocated: + return + if len(relocated) > 1: + log.warning( + "%d calendars are named %r, so the canonical URL of the calendar just " + "created is ambiguous; keeping the requested URL (%s) rather than risk " + "adopting a pre-existing unrelated calendar", + len(relocated), + name, + self.url, + ) + return + self.url = relocated[0] + + async def _async_create(self, path, mkcol, method, name, display_name, stable_url) -> None: """Async implementation of _create (call via _create, not directly).""" await self._query(root=mkcol, query_method=method, url=path, expected_return_value=201) @@ -810,6 +1001,10 @@ async def _async_create(self, path, mkcol, method, name, display_name) -> None: exc_info=True, ) + # See _adopt_canonical_url (sync) - re-point self.url on unstable servers. + if display_name and not stable_url: + await self._async_adopt_canonical_url(name) + def delete(self, wipe=None): """Delete the calendar. @@ -1139,28 +1334,41 @@ async def _async_save(self, display_name, method=None): # def data2object_class - def _multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) -> Iterable[str]: - """ - get multiple events' data. - TODO: Does it overlap the _request_report_build_resultlist method - ## WARNING: async logic is duplicated in _async_multiget — mirror any changes there + def _build_multiget_root(self, event_urls: Iterable[URL]) -> cdav.CalendarMultiGet: + """Build the calendar-multiget REPORT body for the given hrefs. + + Pure (no I/O) — shared by the sync and async multiget twins. """ if self.url is None: raise ValueError("Unexpected value None for self.url") - prop = dav.Prop() + cdav.CalendarData() - root = cdav.CalendarMultiGet() + prop + [dav.Href(value=u.path) for u in event_urls] - # RFC 4791 section 7.9: "the 'Depth' header MUST be ignored by the - # server and SHOULD NOT be sent by the client" for calendar-multiget - response = self._query(root, None, "report") + return cdav.CalendarMultiGet() + prop + [dav.Href(value=u.path) for u in event_urls] + + def _extract_multiget_results( + self, response: Any, raise_notfound: bool + ) -> list[tuple[str, str]]: + """Turn a multiget REPORT response into ``(href, calendar_data)`` tuples. + + Pure (no I/O) — shared by the sync and async multiget twins. + """ results = response.expand_simple_props([cdav.CalendarData()]) if raise_notfound: - for href in response.statuses: - status = response.statuses[href] + for href, status in response.statuses.items(): if status and "404" in status: raise error.NotFoundError(f"Status {status} in {href}") - for r in results: - yield (r, results[r][cdav.CalendarData.tag]) + return [(r, results[r][cdav.CalendarData.tag]) for r in results] + + def _multiget( + self, event_urls: Iterable[URL], raise_notfound: bool = False + ) -> list[tuple[str, str]]: + """get multiple events' data. + + TODO: Does it overlap the _request_report_build_resultlist method? + """ + # RFC 4791 section 7.9: "the 'Depth' header MUST be ignored by the + # server and SHOULD NOT be sent by the client" for calendar-multiget + response = self._query(self._build_multiget_root(event_urls), None, "report") + return self._extract_multiget_results(response, raise_notfound) def _post_multiget(self, results: Iterable[tuple[str, str]]) -> list[_CC]: """Post-processing shared by multiget and _async_multiget_objects.""" @@ -1188,20 +1396,8 @@ def multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) -> I async def _async_multiget( self, event_urls: Iterable[URL], raise_notfound: bool = False ) -> list[tuple[str, str]]: - ## WARNING: sync logic is duplicated in _multiget — mirror any changes there - if self.url is None: - raise ValueError("Unexpected value None for self.url") - - prop = dav.Prop() + cdav.CalendarData() - root = cdav.CalendarMultiGet() + prop + [dav.Href(value=u.path) for u in event_urls] - response = await self._query(root, None, "report") - results = response.expand_simple_props([cdav.CalendarData()]) - if raise_notfound: - for href in response.statuses: - status = response.statuses[href] - if status and "404" in status: - raise error.NotFoundError(f"Status {status} in {href}") - return [(r, results[r][cdav.CalendarData.tag]) for r in results] + response = await self._query(self._build_multiget_root(event_urls), None, "report") + return self._extract_multiget_results(response, raise_notfound) async def _async_multiget_objects( self, event_urls: Iterable[URL], raise_notfound: bool = False @@ -1211,6 +1407,75 @@ async def _async_multiget_objects( await self._async_multiget(event_urls, raise_notfound=raise_notfound) ) + def _assign_multiget_data(self, unloaded: list, results: Iterable[tuple[str, str]]) -> None: + """Assign multiget (href, data) results onto the matching unloaded objects. + + Shared post-processing for _batch_load_objects and its async twin: index + the results by normalised URL (quoting to match servers that return + unencoded spaces, e.g. Zimbra) and set obj.data on each match. + + Both sides of the comparison are unquoted before matching. Servers + disagree on what to percent-encode, and the object URLs themselves + went through `quote()` with the default `safe="/"` -- so a UID of the + conventional `@` form ends up as `%40` on one side + and `@` on the other. Comparing the unquoted forms makes the two + spellings equal instead of silently dropping the object. + """ + url_to_data = { + unquote(str(self.url.join(quote(unquote(str(href)), safe="/:@")))): data + for href, data in results + } + for obj in unloaded: + key = unquote(str(obj.url)) + if key in url_to_data: + obj.data = url_to_data[key] + + def _batch_load_objects(self, objects: list) -> None: + """Load unloaded objects from the list in a single calendar-multiget REPORT. + + Already-loaded objects are skipped. If the REPORT fails, falls back to + individual obj.load(only_if_unloaded=True) calls per object, silently + swallowing per-object errors so callers can filter on is_loaded() afterward. + """ + unloaded = [o for o in objects if not o.is_loaded()] + if not unloaded: + return + try: + self._assign_multiget_data(unloaded, self._multiget([o.url for o in unloaded])) + except Exception: + logging.error("Batch multiget failed, falling back to individual loads", exc_info=True) + for obj in unloaded: + try: + obj.load(only_if_unloaded=True) + except Exception: + pass + + async def _async_batch_load_objects(self, objects: list) -> None: + """Async version of _batch_load_objects. + + The post-processing is shared via _assign_multiget_data(); the only + sync/async difference is the await on the multiget REPORT and on the + per-object fallback load(). + """ + unloaded = [o for o in objects if not o.is_loaded()] + if not unloaded: + return + try: + self._assign_multiget_data( + unloaded, await self._async_multiget([o.url for o in unloaded]) + ) + except Exception: + logging.error( + "Async batch multiget failed, falling back to individual loads", exc_info=True + ) + for obj in unloaded: + try: + load_result = obj.load(only_if_unloaded=True) + if inspect.isawaitable(load_result): + await load_result + except Exception: + pass + def calendar_multiget(self, *largs, **kwargs): """ get multiple events' data @@ -1417,6 +1682,7 @@ def search( filters=None, post_filter=None, _hacks=None, + compatibility_workarounds: bool | None = None, **searchargs, ) -> "list[_CC] | Coroutine[Any, Any, list[_CC]]": """Sends a search request towards the server, processes the @@ -1529,11 +1795,25 @@ def search( # For async clients, use async_search if self.is_async_client: return my_searcher.async_search( - self, server_expand, split_expanded, props, xml, post_filter, _hacks + self, + server_expand, + split_expanded, + props, + xml, + post_filter, + _hacks, + compatibility_workarounds, ) return my_searcher.search( - self, server_expand, split_expanded, props, xml, post_filter, _hacks + self, + server_expand, + split_expanded, + props, + xml, + post_filter, + _hacks, + compatibility_workarounds, ) def freebusy_request( @@ -1834,6 +2114,64 @@ def _generate_fake_sync_token(self, objects: list["CalendarObjectResource"]) -> hash_value = hashlib.md5(combined.encode(), usedforsecurity=False).hexdigest() return f"fake-{hash_value}" + ## The three helpers below carry the pure (no-I/O) logic shared between the + ## get_objects_by_sync_token sync/async twins, so only the awaited + ## server round-trips differ between them. + + def _should_use_sync_token(self, sync_token: Any, disable_fallback: bool) -> bool: + """Decide whether to attempt a real sync-collection REPORT. + + Raises ReportError when the server can't do sync-tokens and the caller + forbade the full-retrieval fallback. + """ + sync_support = self.client.features.is_supported("sync-token", return_type=dict) + if sync_support.get("support") == "unsupported": + if disable_fallback: + raise error.ReportError("Sync tokens are not supported by the server") + return False + ## A fake token means we emulated sync support last time; don't try a real one. + if sync_token and isinstance(sync_token, str) and sync_token.startswith("fake-"): + return False + return True + + def _apply_fallback_etags(self, response: Any, all_objects: list) -> None: + """Map ETags from a depth-1 PROPFIND response onto the given objects. + + ETags are crucial for detecting content changes in the fallback + mechanism (which can otherwise only see additions/deletions). + """ + etag_props = response.expand_simple_props([dav.GetEtag()]) + url_to_obj = {str(obj.url.canonical()): obj for obj in all_objects} + log.debug(f"Fallback: Fetching ETags for {len(url_to_obj)} objects") + for url_str, props in etag_props.items(): + canonical_url_str = str(self.url.join(url_str).canonical()) + if canonical_url_str in url_to_obj: + if not hasattr(url_to_obj[canonical_url_str], "props"): + url_to_obj[canonical_url_str].props = {} + url_to_obj[canonical_url_str].props.update(props) + log.debug(f"Fallback: Added ETag to {canonical_url_str}") + + def _build_fallback_sync_result( + self, all_objects: list, sync_token: Any + ) -> "SynchronizableCalendarObjectCollection": + """Build the fallback collection from a full object list, emulating + sync-token semantics: if the caller passed back our previous fake + token and nothing changed, return an empty collection. + """ + fake_sync_token = self._generate_fake_sync_token(all_objects) + if ( + sync_token + and isinstance(sync_token, str) + and sync_token.startswith("fake-") + and sync_token == fake_sync_token + ): + return SynchronizableCalendarObjectCollection( + calendar=self, objects=[], sync_token=fake_sync_token + ) + return SynchronizableCalendarObjectCollection( + calendar=self, objects=all_objects, sync_token=fake_sync_token + ) + def get_objects_by_sync_token( self, sync_token: Any | None = None, @@ -1869,23 +2207,9 @@ def get_objects_by_sync_token( the server truly supports sync tokens. """ if self.is_async_client: - ## TODO: lots of code duplication here. It's difficult, since there is a lot of - ## forth and back between the client and the server in this method. return self._async_get_objects_by_sync_token(sync_token, load_objects, disable_fallback) - ## Check if we should attempt to use sync tokens - ## (either server supports them, or we haven't checked yet, or this is a fake token) - use_sync_token = True - sync_support = self.client.features.is_supported("sync-token", return_type=dict) - if sync_support.get("support") == "unsupported": - if disable_fallback: - raise error.ReportError("Sync tokens are not supported by the server") - use_sync_token = False - ## If sync_token looks like a fake token, don't try real sync-collection - if sync_token and isinstance(sync_token, str) and sync_token.startswith("fake-"): - use_sync_token = False - - if use_sync_token: + if self._should_use_sync_token(sync_token, disable_fallback): try: root = self.client._build_sync_collection_body( sync_token=sync_token, props=["getetag"] @@ -1931,50 +2255,17 @@ def get_objects_by_sync_token( pass ## Fetch ETags for all objects if not already present - ## ETags are crucial for detecting changes in the fallback mechanism if all_objects and ( not hasattr(all_objects[0], "props") or dav.GetEtag.tag not in all_objects[0].props ): - ## Use PROPFIND to fetch ETags for all objects try: ## Do a depth-1 PROPFIND on the calendar to get all ETags response = self._query_properties([dav.GetEtag()], depth=1) - etag_props = response.expand_simple_props([dav.GetEtag()]) - - ## Map ETags to objects by URL (using string keys for reliable comparison) - url_to_obj = {str(obj.url.canonical()): obj for obj in all_objects} - log.debug(f"Fallback: Fetching ETags for {len(url_to_obj)} objects") - for url_str, props in etag_props.items(): - canonical_url_str = str(self.url.join(url_str).canonical()) - if canonical_url_str in url_to_obj: - if not hasattr(url_to_obj[canonical_url_str], "props"): - url_to_obj[canonical_url_str].props = {} - url_to_obj[canonical_url_str].props.update(props) - log.debug(f"Fallback: Added ETag to {canonical_url_str}") + self._apply_fallback_etags(response, all_objects) except Exception as e: - ## If fetching ETags fails, we'll fall back to URL-based tokens - ## which can't detect content changes, only additions/deletions log.debug(f"Failed to fetch ETags for fallback sync: {e}") - pass - - ## Generate a fake sync token based on current state - fake_sync_token = self._generate_fake_sync_token(all_objects) - ## If a sync_token was provided, check if anything has changed - if sync_token and isinstance(sync_token, str) and sync_token.startswith("fake-"): - ## Compare the provided token with the new token - if sync_token == fake_sync_token: - ## Nothing has changed, return empty collection - return SynchronizableCalendarObjectCollection( - calendar=self, objects=[], sync_token=fake_sync_token - ) - ## If tokens differ, return all objects (emulating a full sync) - ## In a real implementation, we'd return only changed objects, - ## but that requires storing previous state which we don't have - - return SynchronizableCalendarObjectCollection( - calendar=self, objects=all_objects, sync_token=fake_sync_token - ) + return self._build_fallback_sync_result(all_objects, sync_token) def objects_by_sync_token( self, *largs, **kwargs @@ -1996,20 +2287,7 @@ async def _async_get_objects_by_sync_token( disable_fallback: bool = False, ) -> "SynchronizableCalendarObjectCollection": """Async implementation of get_objects_by_sync_token.""" - - ## TODO: lots of code duplication here. It's difficult, since there is a lot of - ## forth and back between the client and the server in this method. - - use_sync_token = True - sync_support = self.client.features.is_supported("sync-token", return_type=dict) - if sync_support.get("support") == "unsupported": - if disable_fallback: - raise error.ReportError("Sync tokens are not supported by the server") - use_sync_token = False - if sync_token and isinstance(sync_token, str) and sync_token.startswith("fake-"): - use_sync_token = False - - if use_sync_token: + if self._should_use_sync_token(sync_token, disable_fallback): try: root = self.client._build_sync_collection_body( sync_token=sync_token, props=["getetag"] @@ -2048,30 +2326,11 @@ async def _async_get_objects_by_sync_token( ): try: response = await self._query_properties([dav.GetEtag()], depth=1) - etag_props = response.expand_simple_props([dav.GetEtag()]) - url_to_obj = {str(obj.url.canonical()): obj for obj in all_objects} - log.debug(f"Fallback: Fetching ETags for {len(url_to_obj)} objects") - for url_str, props in etag_props.items(): - canonical_url_str = str(self.url.join(url_str).canonical()) - if canonical_url_str in url_to_obj: - if not hasattr(url_to_obj[canonical_url_str], "props"): - url_to_obj[canonical_url_str].props = {} - url_to_obj[canonical_url_str].props.update(props) - log.debug(f"Fallback: Added ETag to {canonical_url_str}") + self._apply_fallback_etags(response, all_objects) except Exception as e: log.debug(f"Failed to fetch ETags for fallback sync: {e}") - fake_sync_token = self._generate_fake_sync_token(all_objects) - - if sync_token and isinstance(sync_token, str) and sync_token.startswith("fake-"): - if sync_token == fake_sync_token: - return SynchronizableCalendarObjectCollection( - calendar=self, objects=[], sync_token=fake_sync_token - ) - - return SynchronizableCalendarObjectCollection( - calendar=self, objects=all_objects, sync_token=fake_sync_token - ) + return self._build_fallback_sync_result(all_objects, sync_token) def get_journals(self) -> "list[Journal] | Coroutine[Any, Any, list[Journal]]": """ diff --git a/caldav/compatibility_hints.py b/caldav/compatibility_hints.py index 0ff4e690..059091fd 100644 --- a/caldav/compatibility_hints.py +++ b/caldav/compatibility_hints.py @@ -80,8 +80,17 @@ class FeatureSet: "url": { "type": "client-hints", }, + "well-known": { + "description": "Server handles /.well-known/caldav discovery as specified in RFC 6764 section 5. A conformant server should respond with a redirect (301/302/307/308) from /.well-known/caldav to the actual CalDAV endpoint. 'full' means a redirect was observed; 'unsupported' means the server returned 404 or similar; 'unknown' means the check was skipped (e.g. localhost or request failed). Note: well-known is often provided by infrastructure (reverse proxy/hosting) rather than the CalDAV server itself, so 'unknown' is the expected default for self-hosted or test setups.", + "default": {"support": "unknown"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc6764#section-5"], + }, "get-current-user-principal": { "description": "Support for RFC5397, current principal extension. Most CalDAV servers have this, but it is an extension to the DAV standard. Possibly observed missing on mail.ru, DavMail gateway and it is possible to configure the support in some sabre-based servers", + ## Independent feature (directly probed): the default marks it so the + ## node uses its own probed value rather than being derived from + ## subfeatures such as .has-calendar. + "default": {"support": "full"}, "links": ["https://datatracker.ietf.org/doc/html/rfc5397"], }, "get-current-user-principal.has-calendar": { @@ -91,9 +100,36 @@ class FeatureSet: "description": "Server returns the supported-calendar-component-set property (RFC 4791 section 5.2.3). The property is optional: when absent the RFC mandates that all component types are accepted, so 'unsupported' here is not a protocol violation, but the client cannot determine the actual supported set without trying.", "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-5.2.3"], }, + "propfind": { + "description": "Server supports the PROPFIND method (RFC4918 section 9.1): a PROPFIND for a named property returns a multistatus response. Independent feature (not just a grouping node) so that a server lacking a sub-feature like propfind.allprop.resourcetype is not mistaken for one that does not support PROPFIND at all.", + "default": {"support": "full"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4918#section-9.1"], + }, + "propfind.allprop": { + "description": "An PROPFIND returns a multistatus response. This is independent of whether resourcetype in particular is included (see propfind.allprop.resourcetype).", + "default": {"support": "full"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4918#section-9.1"], + }, + "propfind.allprop.resourcetype": { + "description": "An PROPFIND returns the DAV:resourcetype live property. RFC4918 section 9.1 lists resourcetype among the live properties an allprop request should return, so 'full' (the default) is the conformant behaviour; a few servers (Bedework) omit it.", + "default": {"support": "full"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4918#section-9.1"], + }, "create-calendar.with-supported-component-types": { "description": "Server honours the supported-calendar-component-set restriction set at MKCALENDAR time. When 'full', the server both advertises (or enforces) the restriction; when 'unsupported', the restriction is silently ignored (wrong-type objects can be saved to the calendar). When 'ungraceful', the MKCALENDAR request itself fails when a component set is specified.", }, + "calendar-color": { + "description": "Server stores the nonstandard Apple/Mozilla {http://apple.com/ns/ical/}calendar-color property (set with a colour name like 'blue') on a calendar collection. 'full' covers servers that normalise the name to a hex value (the set value still tracks the input); 'broken' is a read-only property (the same value comes back regardless of what is set). Not described by RFC4791/RFC5545, so a server that rejects or ignores it ('unsupported') is not breaching any RFC. The default is 'fragile' because the behaviour varies a lot between servers and is rarely worth asserting on.", + "default": {"support": "fragile"}, + }, + "calendar-color.hex": { + "description": "Like calendar-color, but the property is set with a hex value (e.g. '#FF0000FF') rather than a colour name. Some servers accept one form but not the other.", + "default": {"support": "fragile"}, + }, + "calendar-order": { + "description": "Server stores the nonstandard Apple/Mozilla {http://apple.com/ns/ical/}calendar-order property on a calendar collection (a get/set round-trip). 'broken' is a read-only property (e.g. the server returns the calendar's own position regardless of what is set). Not described by RFC4791/RFC5545, so a server that rejects or ignores it ('unsupported') is not breaching any RFC. The default is 'fragile' because the behaviour varies a lot between servers.", + "default": {"support": "fragile"}, + }, "rate-limit": { "type": "client-feature", "description": "client (or test code) must sleep a bit between requests. Pro-active rate limiting is done through interval and count, server-flagged rate-limiting is controlled through default_sleep/max_sleep", @@ -110,6 +146,15 @@ class FeatureSet: "delay": "after this number of seconds, we may be reasonably sure that the search results are updated", } }, + "write-delay": { + "type": "server-peculiarity", + "default": {"support": "full"}, + "description": "The server processes write operations (PUT/DELETE/MKCALENDAR/PROPPATCH/...) asynchronously: the request returns success before the change has fully taken effect, so an immediate read-back (of any kind, not just a search) may 404 or return stale data. A client must wait a bit after every write. This is the general, write-side counterpart of 'search-cache' (which only delays searches). 'full' (the default) means writes take effect synchronously.", + "extra_keys": { + "behaviour": "'delay' to enable the post-write sleep", + "delay": "sleep this number of seconds after every write request before relying on the change being visible", + } + }, "tests-cleanup-calendar": { "type": "tests-behaviour", "description": "Deleting a calendar does not delete the objects, or perhaps create/delete of calendars does not work at all. For each test run, every calendar resource object should be deleted for every test run", @@ -127,10 +172,38 @@ class FeatureSet: "description": "Accessing a calendar which does not exist automatically creates it", }, "create-calendar.set-displayname": { - "description": "It's possible to set the displayname on a calendar upon creation" + "description": "It's possible to set the displayname on a calendar upon creation", + ## Independent feature (directly probed). + "default": {"support": "full"}, + }, + "create-calendar.stable-url": { + "description": ( + "After a calendar is created it remains addressable at the URL derived from the " + "requested cal_id. 'full' (the normal case): the calendar's canonical URL is the " + "requested URL. 'unsupported': the server assigns a DIFFERENT canonical URL and the " + "requested cal_id is not a reliable address for the calendar's object resources, so " + "clients must discover and adopt the canonical URL after creation (the caldav library " + "does this automatically). Two known patterns are handled identically: Zimbra " + "relocates the collection to a display-name-derived path - a collection-level alias " + "may linger at the cal_id and answer PROPFIND/REPORT, but a GET on a child object " + "(...//.ics) 404s, so it is not a usable address (cf. save-load.get-by-url); " + "OX always exposes an opaque cal://0/NNN (base64-segment) canonical URL. Note: on " + "Zimbra the URL only becomes unstable when a display name is supplied at creation; a " + "nameless MKCALENDAR stays at the requested cal_id." + ), + "default": {"support": "full"}, + }, + "propfind.displayname": { + "description": "Server returns the DAV:displayname property for a calendar collection via PROPFIND (RFC4918 section 15.2). This is a standard live property; virtually all CalDAV servers support it. 'broken' means the property is absent from the PROPFIND response even though a displayname was supplied at creation time.", + "default": {"support": "full"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4918#section-15.2"], }, "delete-calendar": { "description": "RFC4791 says nothing about deletion of calendars, so the server implementation is free to choose weather this should be supported or not. Section 3.2.3.2 in RFC 6638 says that if a calendar is deleted, all the calendarobjectresources on the calendar should also be deleted - but it's a bit unclear if this only applies to scheduling objects or not. Some calendar servers moves the object to a trashcan rather than deleting it", + ## Independent feature (directly probed): the default marks it so the + ## node uses its own probed value rather than being derived from + ## .free-namespace. + "default": {"support": "full"}, "links": ["https://datatracker.ietf.org/doc/html/rfc6638#section-3.2.3.2"], }, "delete-calendar.free-namespace": { @@ -142,9 +215,12 @@ class FeatureSet: "default": { "support": "fragile" }, }, "save-load": { - "description": "it's possible to save and load objects to the calendar" + "description": "it's possible to save and load objects to the calendar", + }, + "save-load.event": { ## TODO: make this DRY + "description": "it's possible to save and load events to the calendar", + "default": { "support": "full" } }, - "save-load.event": {"description": "it's possible to save and load events to the calendar"}, "save-load.event.recurrences": {"description": "it's possible to save and load recurring events to the calendar - events with an RRULE property set, including recurrence sets", "default": {"support": "full"}}, "save-load.event.recurrences.count": {"description": "The server will receive and store a recurring event with a count set in the RRULE", "default": {"support": "full"}}, ## This was Claude's suggestion and it works as of today, the @@ -157,17 +233,32 @@ class FeatureSet: ## information was simply discarded, and the current search behaviour would in ## such a case be incorrect if the exception is simply discarded. "save-load.event.recurrences.exception": {"description": "When a VCALENDAR containing a master VEVENT (with RRULE) and exception VEVENT(s) (with RECURRENCE-ID) is stored, the server keeps them together as a single calendar object resource. When unsupported, the server splits exception VEVENTs into separate calendar objects, making client-side expansion unreliable (the master expands without knowing about its exceptions)."}, - "save-load.todo": {"description": "it's possible to save and load tasks to the calendar"}, - "save-load.todo.recurrences": {"description": "it's possible to save and load recurring tasks to the calendar"}, + "save-load.event.recurrences.exception.reschedule": {"description": "The server accepts a PUT that reschedules an entire recurring event - changing the master VEVENT's DTSTART (re-anchoring the whole series) while detached exception VEVENT(s) (with RECURRENCE-ID) are present and their RECURRENCE-IDs are shifted to line up with the new series. This is unsupported for Ox, the server rejects such a PUT with 409 Conflict even when a matching If-Match etag is supplied. Rescheduling a recurring event that has no exceptions still works. Exercised by save(all_recurrences=True) after changing dtstart/dtend.", "default": {"support": "full"}}, + "save-load.todo": { + "description": "it's possible to save and load tasks to the calendar", + "default": { "support": "full" } + }, + "save-load.todo.recurrences": {"description": "it's possible to save and load recurring tasks to the calendar", "default": {"support": "full"}}, "save-load.todo.recurrences.count": {"description": "The server will receive and store a recurring task with a count set in the RRULE", "default": {"support": "full"}}, "save-load.todo.recurrences.thisandfuture": {"description": "Completing a recurring task with rrule_mode='thisandfuture' works (modifies RRULE and saves back to server)", "default": {"support": "full"}}, "save-load.todo.mixed-calendar": {"description": "The same calendar may contain both events and tasks (Zimbra only allows tasks to be placed on special task lists)", "default": {"support": "full"}}, - "save-load.journal": {"description": "The server will even accept journals"}, + "save-load.journal": { + "description": "The server will even accept journals", + "default": { "support": "full" } + }, ## TODO: zimbra cannot mix events and tasks, but then davis surprised me by not allowing journals on the same calendar. But this may be a miss in the checking script - it may be that mixing is allowed, but that the calendar has to be set up from scratch with explicit support for both VJOURNAL and other things "save-load.journal.mixed-calendar": {"description": "The same calendar may contain events, tasks and journals (some servers require journals on a dedicated VJOURNAL calendar)", "default": {"support": "full"}}, "save-load.get-by-url": { "description": "GET requests to calendar object resource URLs work correctly. When unsupported, the server returns 404 on GET even for valid object URLs. The client works around this by falling back to UID-based lookup.", }, + "non-existing-raises-not-found": { + "description": "Looking up a non-existing calendar object resource raises NotFoundError (the server answers 404). 'full' (the default) is the expected behaviour; some servers answer 403 instead (raising AuthorizationError) - e.g. Robur, probably to avoid leaking whether a resource exists - which is a legitimate choice rather than an RFC breach, so it is recorded as 'unsupported' rather than 'broken'.", + "default": {"support": "full"}, + }, + "save-load.stable-url": { + "description": "The server reports a calendar object resource under the same URL the client used to store it. When 'unsupported', the server canonicalizes the URL: e.g. OX App Suite exposes a calendar both under its display name and under an internal 'cal://0/NNN' identifier, so an object looked up via a calendar-query REPORT (object_by_uid / search) is reported under a different calendar path than the PUT URL. A direct GET on the original URL still works (the server keeps an alias). Clients should therefore not assume that a searched object's URL equals the URL it was created at.", + "default": {"support": "full"}, + }, "save-load.reuse-deleted-uid": { "description": "After deleting an event, the server allows creating a new event with the same UID. When 'broken', the server keeps deleted events in a trashbin with a soft-delete flag, causing unique constraint violations on UID reuse. See https://github.com/nextcloud/server/issues/30096" }, @@ -184,6 +275,15 @@ class FeatureSet: "description": "A saved calendar object resource can be modified and PUT back to the server; the server accepts the update and returns the modified data on the next GET/REPORT. When 'unsupported', the server treats calendar objects as immutable after initial creation (e.g. Google Calendar's legacy CalDAV API). Replaces the old 'no_overwrite' compatibility flag.", "default": {"support": "full"}, }, + "save-load.mutable.attendee-partstat": { + "description": "A client can modify an attendee's PARTSTAT on an existing event and PUT it back directly to the calendar. When 'unsupported', the server forbids direct modification of attendee participation status via PUT (e.g. OX App Suite returns 403 Forbidden even with a matching If-Match etag) and expects the change to be made through iTIP scheduling instead. See https://github.com/python-caldav/caldav/issues/399", + "default": {"support": "full"}, + "links": ["https://github.com/python-caldav/caldav/issues/399"], + }, + "save-load.mutable.if-match-optional": { + "description": "The If-Match precondition is optional when overwriting an existing calendar object resource: the server accepts a PUT that carries no If-Match etag (i.e. add_event()/save() on an object that was not first fetched). When 'unsupported', the server requires an If-Match etag for updates and rejects a no-If-Match overwrite with 409 Conflict (e.g. OX App Suite enforces optimistic concurrency). Such servers still support save-load.mutable via a fetch-then-save (etag-conditional) update; only the blind-overwrite path is affected.", + "default": {"support": "full"}, + }, "search": { "description": "calendar MUST support searching for objects using the REPORT method, as specified in RFC4791, section 7", "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-7"], @@ -208,7 +308,17 @@ class FeatureSet: "description": "Time-range searches should only return events/todos that actually fall within the requested time range. Some servers incorrectly return recurring events whose recurrences fall outside (after) the search interval, or events with no recurrences in the requested time range at all. RFC4791 section 9.9 specifies that a VEVENT component overlaps a time range if the condition (start < search_end AND end > search_start) is true.", "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-9.9"], }, + "search.time-range.comp-type-optional": { + "description": "Whether the server accepts a calendar-query carrying a time-range filter but NOT specifying a component type. Per RFC4791 section 9.7 a CALDAV:time-range element is only valid inside a comp-filter for VEVENT/VTODO/VJOURNAL/VFREEBUSY/VALARM - never directly under the VCALENDAR comp-filter. A query without a component type therefore has nowhere RFC-legal to put the time-range. Consequently 'unsupported' (the default) is FULLY RFC-COMPLIANT and is NOT a server defect: SabreDAV-based servers (Baikal, Nextcloud, ...) correctly reject such queries with HTTP 400 'You cannot add time-range filters on the VCALENDAR component'. When unsupported, the library splits the search into one query per component type. See https://github.com/python-caldav/caldav/issues/681", + "default": {"support": "unsupported"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-9.7"], + }, "search.time-range.todo": {"description": "basic time range searches for tasks works", "default": {"support": "full"}}, + "search.time-range.todo.no-dtstart": { + "description": "A VTODO without DTSTART (but with DUE) is returned by a date-range search. RFC5545 and RFC4791 section 9.9 say such a task has a defined time span and should be found, so 'full' (the default) is the compliant behaviour; some servers (Davical, Stalwart, Synology) skip any task lacking DTSTART. Probed with a closed window; servers that skip such tasks only in closed ranges (returning them in open-ended ones) are instead tracked by the 'vtodo_datesearch_nodtstart_task_is_skipped_in_closed_date_range' flag.", + "default": {"support": "full"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-9.9"], + }, "search.time-range.todo.old-dates": {"description": "time range searches for tasks with old dates (e.g. year 2000) work - some servers enforce a min-date-time restriction"}, "search.time-range.todo.strict": { "description": "Bounded VTODO time-range searches do not return tasks whose time span falls entirely outside the searched range (no false positives).", @@ -264,6 +374,11 @@ class FeatureSet: "search.text": { "description": "Search for text attributes should work" }, + "search.text.comp-type-optional": { + "description": "Whether the server returns matching objects for a calendar-query that carries a prop-filter (CATEGORIES, SUMMARY, ...) but does NOT specify a component type. Such a prop-filter ends up directly under the VCALENDAR comp-filter, where it filters on VCALENDAR's own properties - which do not include component properties like CATEGORIES - so most servers (e.g. Xandikos, SabreDAV) match nothing. 'unsupported' (the default) is therefore the common, RFC-reasonable case; when unsupported the library splits the search into one query per component type. Analogous to search.time-range.comp-type-optional. See https://github.com/python-caldav/caldav/issues/681", + "default": {"support": "unsupported"}, + "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-9.7"], + }, "search.text.case-sensitive": { "description": "In RFC4791, section-9.7.5, a text-match may pass a collation, and i;ascii-casemap MUST be the default, this is not checked (yet - TODO) by the caldav-server-checker project. Section 7.5 describes that the servers also are REQUIRED to support i;octet. The definitions of those collations are given in RFC4790, i;octet is a case-sensitive byte-by-byte comparition (fastest). search.text.case-sensitive is supported if passing the i;octet collation to search causes the search to be case-sensitive.", "links": [ @@ -285,6 +400,10 @@ class FeatureSet: }, "search.text.category": { "description": "Search for category should work. This is not explicitly specified in RFC4791, but covered in section 9.7.5. No examples targets categories explicitly, but there are some text match examples in section 7.8.6 and following sections", + ## Independent feature (directly probed): the default marks it so the + ## node uses its own probed value rather than being derived from + ## .substring. + "default": {"support": "full"}, "links": [ "https://datatracker.ietf.org/doc/html/rfc4791#section-9.7.5", "https://datatracker.ietf.org/doc/html/rfc4791#section-7.8.6", @@ -301,7 +420,11 @@ class FeatureSet: "links": ["https://datatracker.ietf.org/doc/html/rfc4791#section-7.4"], }, "search.recurrences.includes-implicit.todo": { - "description": "tasks can also be recurring" + "description": "tasks can also be recurring", + ## Independent feature (directly probed): the default marks it so the + ## node uses its own probed value rather than being derived from + ## .pending. + "default": {"support": "full"}, }, "search.recurrences.includes-implicit.todo.pending": { "description": "a future recurrence of a pending task should always be pending and appear in searches for pending tasks", @@ -332,6 +455,10 @@ class FeatureSet: }, "sync-token": { "description": "RFC6578 sync-collection reports are supported. Server provides sync tokens that can be used to efficiently retrieve only changed objects since last sync. Support can be 'full', 'fragile' (occasionally returns more content than expected), or 'unsupported'. Behaviour 'time-based' indicates second-precision tokens requiring sleep(1) between operations", + ## Independent feature (directly probed): the default marks it so the + ## node uses its own probed value rather than being derived from + ## .delete. + "default": {"support": "full"}, "links": ["https://datatracker.ietf.org/doc/html/rfc6578"], }, "sync-token.delete": { @@ -339,6 +466,10 @@ class FeatureSet: }, "scheduling": { "description": "Server supports CalDAV Scheduling (RFC6638). Detected via the presence of 'calendar-auto-schedule' in the DAV response header.", + ## Independent feature (directly probed via the DAV header): the default + ## marks it so the node uses its own probed value rather than being + ## derived from subfeatures such as .calendar-user-address-set. + "default": {"support": "full"}, "links": ["https://datatracker.ietf.org/doc/html/rfc6638"], }, "scheduling.mailbox": { @@ -390,6 +521,11 @@ class FeatureSet: }, "principal-search": { "description": "Server supports searching for principals (CalDAV users). Principal search may be restricted for privacy/security reasons on many servers. (not to be confused with get-current-user-principal)" + ## NB: genuine grouping node - 'supported' iff at least one search + ## method (.by-name / .list-all) works. The checker sets it directly + ## because that OR-semantics cannot be expressed by the library's + ## all-children-agree derivation; it deliberately has NO default so + ## that when all sub-searches fail the node is unsupported. }, "principal-search.by-name": { "description": "Server supports searching for principals by display name. Testing this properly requires setting up another user with a known name, so this check is not yet implemented" @@ -408,6 +544,10 @@ class FeatureSet: "save.duplicate-uid.cross-calendar": { "description": "Server allows events with the same UID to exist in different calendars and treats them as separate entities. Support can be 'full' (allowed), 'ungraceful' (rejected with error), or 'unsupported' (silently ignored or moved). Behaviour 'silently-ignored' means the duplicate is not saved but no error is thrown. Behaviour 'moved-instead-of-copied' means the event is moved from the original calendar to the new calendar (Zimbra behavior)" }, + "save.duplicate-event": { + "description": "Server allows two events with identical content but different UIDs to coexist in the same calendar. Some servers reject or de-duplicate such an event ('duplicates not allowed even with a different UID'), in which case this is 'unsupported' (silently dropped) or 'ungraceful' (rejected with an error). The default 'full' is the usual behaviour.", + "default": {"support": "full"}, + }, ## TODO: as for now, the tests will run towards the first calendar it will find, and most of the tests will assume the calendar is empty. This is bad. "test-calendar": { "type": "tests-behaviour", @@ -493,13 +633,14 @@ def copyFeatureSet(self, feature_set, collapse=True): UserWarning, stacklevel=3, ) + continue value = feature_set[feature] if feature not in self._server_features: self._server_features[feature] = {} server_node = self._server_features[feature] if isinstance(value, bool): server_node['support'] = "full" if value else "unsupported" - elif isinstance(value, str) and 'support' not in server_node: + elif isinstance(value, str): self._validate_support_level(value, feature) server_node['support'] = value elif isinstance(value, dict): @@ -541,44 +682,76 @@ def _collapse_key(self, feature_dict): def collapse(self): """ - If all subfeatures are the same, it should be collapsed into the parent - - Messy and complex logic :-( + Compact the stored feature set: a *grouping* parent (one without its own + explicit default) whose grouping children are all explicitly set to the + same status is replaced by a single entry on the parent, and the children + are dropped. + + The parent status comes from the single derivation path, + is_supported() -> _derive_from_subfeatures(). That path already: + * treats a node with an explicit default as an independent feature - + never derived/collapsed from its children (so e.g. save-load.mutable + stays "full" even when every child is "unsupported"), and + * ignores independent children (those with their own default) when + deriving a grouping parent. + collapse() adds only a losslessness check on top: it folds the children + in solely when every grouping child is explicitly set and matches the + derived value, so no per-child information is lost. """ - features = list(self._server_features.keys()) parents = set() - for feature in features: + for feature in self._server_features: if '.' in feature: parents.add(feature[:feature.rfind('.')]) - parents = list(parents) - ## Parents needs to be ordered by the number of dots. We proceed those with most dots first. - parents.sort(key = lambda x: (-x.count('.'), x)) - for parent in parents: + ## Deepest parents first, so a freshly collapsed child can feed its parent. + for parent in sorted(parents, key=lambda x: (-x.count('.'), x)): parent_info = self.find_feature(parent) - if len(parent_info['subfeatures']): - foo = self.is_supported(parent, return_type=dict, return_defaults=False) - if len(parent_info['subfeatures']) > 1 or foo is not None: - dont_collapse = False - foo_key = self._collapse_key(foo) if foo is not None else None - for sub in parent_info['subfeatures']: - bar = self._server_features.get(f"{parent}.{sub}") - if bar is None: - dont_collapse = True - break - bar_key = self._collapse_key(bar) - if foo is None: - foo = bar - foo_key = bar_key - elif bar_key != foo_key: - dont_collapse = True - break - if not dont_collapse: - if parent not in self._server_features: - self._server_features[parent] = {} - for sub in parent_info['subfeatures']: - self._server_features.pop(f"{parent}.{sub}") - self.copyFeatureSet({parent: foo}) + ## Independent node (its own explicit default) is never collapsed. + if 'default' in parent_info: + continue + + ## Independent children (their own default) are separate features: + ## neither folded in nor required to match. + grouping_children = [ + sub + for sub in parent_info['subfeatures'] + if 'default' not in self.find_feature(f"{parent}.{sub}") + ] + if not grouping_children: + continue + + derived = self.is_supported(parent, return_type=dict, return_defaults=False) + if derived is None: + continue + derived_key = self._collapse_key(derived) + + ## Lossless only if every grouping child is explicitly set and matches. + child_nodes = [self._server_features.get(f"{parent}.{sub}") for sub in grouping_children] + if any(node is None or self._collapse_key(node) != derived_key for node in child_nodes): + continue + + ## Folding sets the (previously unset) parent explicitly, which an + ## *independent* child (its own default) that is not itself explicitly + ## set would then inherit - changing its resolved status whenever its + ## default differs from the derived value. Skip the fold in that case + ## so is_supported() stays invariant under collapse(). (e.g. folding + ## save.duplicate-uid into save must not flip the independent sibling + ## save.duplicate-event from its default "full" to "ungraceful".) + independent_children = [ + sub + for sub in parent_info['subfeatures'] + if 'default' in self.find_feature(f"{parent}.{sub}") + ] + if any( + f"{parent}.{sub}" not in self._server_features + and self._collapse_key(self._default(f"{parent}.{sub}")) != derived_key + for sub in independent_children + ): + continue + + for sub in grouping_children: + self._server_features.pop(f"{parent}.{sub}", None) + self.copyFeatureSet({parent: derived}) def _default(self, feature_info): if isinstance(feature_info, str): @@ -619,7 +792,12 @@ def is_supported(self, feature, return_type=bool, return_defaults=True, accept_f if 'default' not in current_info: derived = self._derive_from_subfeatures(feature_, current_info, return_type, accept_fragile) if derived is not None: - return derived + # When visiting an ancestor node (feature_ != feature), only propagate + # the derived status downward if the *original* queried feature is also + # a grouping node (no explicit default). Independent features have their + # own explicit default and must not be overridden by a derived ancestor. + if feature_ == feature or 'default' not in feature_info: + return derived if '.' not in feature_: if not return_defaults: return None @@ -689,8 +867,11 @@ def _derive_from_subfeatures(self, feature, feature_info, return_type, accept_fr if has_positive: if all_same: derived_status = subfeature_statuses[0] + elif not is_complete: + # Incomplete mixed set: unset siblings might be unsupported; inconclusive + return None else: - # Mixed positive/negative → unknown + # All relevant children seen, but mixed positive/negative → unknown derived_status = 'unknown' elif is_complete and all_same: # All relevant subfeatures set, all the same negative status @@ -804,6 +985,67 @@ def dotted_feature_set_list(self, compact=False): ret[x] = feature.copy() return ret + ## Feature types that the server-tester cannot reliably probe and that + ## therefore must not be cross-checked against the declared config. + _UNCHECKABLE_FEATURE_TYPES = ( + "client-feature", + "server-observation", + "tests-behaviour", + "client-hints", + "server-peculiarity", + ) + + def compare(self, observed): + """Compare this *declared* (expected) feature set against an *observed* + feature set and return the list of mismatches. + + Each mismatch is a dict with keys ``feature``, ``expected`` and + ``observed`` holding the resolved (string) support levels that disagree. + + Only server-features are compared; anything resolving to ``fragile`` or + ``unknown`` on either side, and feature types the tester cannot probe + reliably (see ``_UNCHECKABLE_FEATURE_TYPES``), are ignored. + """ + ## Snapshot what the tester explicitly probed *before* compact=True + ## calls collapse(), which mutates _server_features by folding + ## subfeatures into their parent - making probed features look + ## untested. is_supported() still resolves the collapsed values + ## correctly afterwards via the parent. + checked_features = set(observed._server_features.keys()) + observed_dotted = observed.dotted_feature_set_list(compact=True) + expected_dotted = self.dotted_feature_set_list(compact=True) + + mismatches = [] + ## Iterate everything either side made an explicit statement about: + ## the compacted dotted dicts plus every feature the tester probed. + ## Probed features whose observed value equals the default are absent + ## from observed_dotted, yet may still conflict with a non-default + ## status the declared config inherits from a parent (e.g. Infomaniak + ## search.comp-type.optional vs an unsupported search.comp-type). + for feature in set(observed_dotted).union(expected_dotted).union(checked_features): + observation = observed.is_supported(feature, str) + expectation = self.is_supported(feature, str) + if "fragile" in (observation, expectation): + continue + if "unknown" in (observation, expectation): + continue + ## Skip features the tester never explicitly probed - the + ## observation would just be a default, not a real result. + if feature not in observed_dotted and feature not in checked_features: + continue + type_ = observed.find_feature(feature).get("type", "server-feature") + if type_ in self._UNCHECKABLE_FEATURE_TYPES: + continue + if expectation != observation: + mismatches.append( + { + "feature": feature, + "expected": expectation, + "observed": observation, + } + ) + return mismatches + #### OLD STYLE ## THE LIST BELOW IS TO BE REMOVED COMPLETELY. DO NOT USE IT. @@ -825,28 +1067,9 @@ def dotted_feature_set_list(self, compact=False): ## * Perhaps some more readable format should be considered (yaml?). ## * Consider how to get this into the documentation incompatibility_description = { - 'calendar_order': - """Server supports (nonstandard) calendar ordering property""", - - 'calendar_color': - """Server supports (nonstandard) calendar color property""", - - 'duplicates_not_allowed': - """Duplication of an event in the same calendar not allowed """ - """(even with different uid)""", - - 'event_by_url_is_broken': """A GET towards a valid calendar object resource URL will yield 404 (wtf?)""", - 'propfind_allprop_failure': - """The propfind test fails ... """ - """it asserts DAV:allprop response contains the text 'resourcetype', """ - """possibly this assert is wrong""", - - 'vtodo_datesearch_nodtstart_task_is_skipped': - """date searches for todo-items will not find tasks without a dtstart""", - 'vtodo_datesearch_nodtstart_task_is_skipped_in_closed_date_range': """only open-ended date searches for todo-items will find tasks without a dtstart""", @@ -866,24 +1089,21 @@ def dotted_feature_set_list(self, compact=False): """Events should be deleted before the calendar is deleted, """ """and/or deleting a calendar may not have immediate effect""", - 'no_overwrite': - """events cannot be edited""", - 'dav_not_supported': """when asked, the server may claim it doesn't support the DAV protocol. Observed by one baikal server, should be investigated more (TODO) and robur""", 'fastmail_buggy_noexpand_date_search': """The 'blissful anniversary' recurrent example event is returned when asked for a no-expand date search for some timestamps covering a completely different date""", - 'non_existing_raises_other': - """Robur raises AuthorizationError when trying to access a non-existing resource (while 404 is expected). Probably so one shouldn't probe a public name space?""", - 'robur_rrule_freq_yearly_expands_monthly': """Robur expands a yearly event into a monthly event. I believe I've reported this one upstream at some point, but can't find back to it""", } xandikos = { + ## Genuinely returns matching objects for a comp-type-less query that carries + ## a time-range (verified: the event is returned, not just "no error"). + "search.time-range.comp-type-optional": {"support": "full"}, ## Principal property search returns 403 (not implemented) "principal-search": "ungraceful", @@ -900,6 +1120,9 @@ def dotted_feature_set_list(self, compact=False): ## There is much development going on at Radicale as of summar 2025, ## so I'm expecting this list to shrink a lot soon. radicale = { + ## Genuinely returns matching objects for a comp-type-less query that carries + ## a time-range (verified: the event is returned, not just "no error"). + "search.time-range.comp-type-optional": {"support": "full"}, "search.is-not-defined": {"support": "full"}, "search.text.case-sensitive": {"support": "unsupported"}, "search.recurrences.includes-implicit.todo.pending": {"support": "fragile", "behaviour": "inconsistent results between runs"}, @@ -909,11 +1132,9 @@ def dotted_feature_set_list(self, compact=False): ## this only applies for very simple installations "auto-connect.url": {"domain": "localhost", "scheme": "http", "basepath": "/"}, "scheduling": {"support": "unsupported"}, - 'old_flags': [ - ## extra features not specified in RFC4791 - "calendar_order", - "calendar_color" - ] + ## extra properties not specified in RFC4791/RFC5545 + "calendar-color": {"support": "full"}, + "calendar-order": {"support": "full"}, } ## Be aware that nextcloud by default have different rate limits, including how often a user is allowed to create a new calendar. This may break test runs badly. @@ -921,8 +1142,15 @@ def dotted_feature_set_list(self, compact=False): 'auto-connect.url': { 'basepath': '/remote.php/dav', }, - ## I'm surprised, I'm quite sure this was reported ungraceful earlier. Passed with caldav commit a98d50490b872e9b9d8e93e2e401c936ad193003, caldav server checker commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad 2026-02-15. The commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad was however development done on the wrong branch and has been force-pushed awway. It was again observed ungraceful at commits be26d42b1ca3ff3b4fd183761b4a9b024ce12b84 / 537a23b145487006bb987dee5ab9e00cdebb0492 - 'search.comp-type.optional': {'support': 'ungraceful'}, + ## Historically this flip-flopped between "ungraceful" and "full" - that + ## instability was a checker bug (https://github.com/python-caldav/caldav/issues/681): + ## the comp-type.optional probe used to send a comp-type-less query carrying a + ## time-range, which SabreDAV rejects (the time-range belongs in a VEVENT/... + ## comp-filter, not under VCALENDAR). Now that the probe omits the time-range, + ## Nextcloud correctly accepts the bare comp-type-less query. The time-range + ## variant is tracked separately as search.time-range.comp-type-optional + ## (unsupported on SabreDAV, the default). + 'search.comp-type.optional': {'support': 'full'}, 'search.recurrences.expanded.todo': {'support': 'unsupported'}, "search.recurrences.includes-implicit.infinite-scope": False, 'delete-calendar': { @@ -970,13 +1198,42 @@ def dotted_feature_set_list(self, compact=False): ## Zimbra is not very good at it's caldav support zimbra = { 'auto-connect.url': {'basepath': '/dav/'}, + ## Genuinely returns matching objects for a comp-type-less query that carries + ## a time-range (verified: the event is returned, not just "no error"). + 'search.time-range.comp-type-optional': {'support': 'full'}, 'delete-calendar': {'support': 'fragile', 'behaviour': 'may move to trashbin instead of deleting immediately'}, ## This is a zimbra bug when creating calendars with a display ## name. Now mitigated in the calendar creation code. #'save-load.get-by-url': {'support': 'fragile', 'behaviour': '404 most of the time - but sometimes 200. Weird, should be investigated more'}, ## Zimbra treats same-UID events across calendars as aliases of the same event 'save.duplicate-uid.cross-calendar': {'support': 'unsupported'}, - 'create-calendar.set-displayname': {'support': 'unsupported'}, + ## Zimbra DOES apply a display name set at creation (the name sticks, so + ## set-displayname is 'full') - but it couples the display name to the + ## calendar URL. MKCALENDAR lands the calendar at the requested cal_id path; + ## the display name is then applied by a follow-up PROPPATCH, which Zimbra + ## implements as a rename that MOVES the collection: the canonical URL + ## relocates to a display-name-derived path (verified deterministic with a + ## unique name against zcs-foss:latest). + ## + ## So create-calendar.stable-url is 'unsupported': is_supported() returns + ## False, and Calendar._create() therefore discovers and adopts the canonical + ## URL after creation (re-pointing self.url), instead of dropping the display + ## name. This keeps the calendar fully usable (name retained AND object URLs + ## resolve) on both Zimbra and OX with no per-server branching. + ## + ## Two Zimbra quirks worth recording (and someday probing for explicitly), + ## mirrored in caldav-server-tester's CheckMakeDeleteCalendar: + ## * The URL is only unstable when a display name is supplied at creation; + ## a nameless MKCALENDAR stays put at the requested cal_id. + ## * Zimbra keeps a collection-level ALIAS at the original cal_id (PROPFIND/ + ## REPORT on it succeed), yet a GET on a child object under that alias + ## (...//.ics) 404s - the object is only retrievable under + ## the canonical relocated URL. So "the calendar collection is reachable + ## at cal_id" does NOT imply "objects are reachable at cal_id"; the canonical + ## URL must be used. (This also explains the old save-load.get-by-url + ## "404 most of the time but sometimes 200" observation.) + 'create-calendar.set-displayname': {'support': 'full'}, + 'create-calendar.stable-url': {'support': 'unsupported', 'behaviour': 'a display name set at creation relocates the collection to a display-name-derived canonical URL; a collection alias lingers at the requested cal_id but child object GETs under it 404'}, 'save-load.todo.mixed-calendar': {'support': 'unsupported'}, 'save-load.todo.recurrences.count': {'support': 'unsupported'}, ## This is a new problem? 'save-load.journal': {'support': 'ungraceful'}, @@ -987,7 +1244,10 @@ def dotted_feature_set_list(self, compact=False): # sometimes throws a 500 'search.text.category': {'support': 'ungraceful'}, 'search.recurrences.expanded.todo': { "support": "unsupported" }, - 'search.comp-type.optional': {'support': 'fragile'}, ## TODO: more research on this, looks like a bug in the checker, + ## was 'fragile' - that was the checker bug (it compared a comp-type-less + ## search against cnt, which counts objects stored in a separate + ## task/journal calendar). Confirmed full 2026-06-06. + 'search.comp-type.optional': {'support': 'full'}, 'search.time-range.alarm': {'support': 'unsupported'}, 'principal-search': "unsupported", ## Zimbra implements server-side automatic scheduling: invitations are @@ -1011,11 +1271,15 @@ def dotted_feature_set_list(self, compact=False): ## TODO: I just discovered that when searching for a date some ## years after a recurring daily event was made, the event does ## not appear. - - ## extra features not specified in RFC5545 - "calendar_order", - "calendar_color" - ] + ], + ## extra properties not specified in RFC4791/RFC5545. Zimbra stores + ## calendar-order, and stores calendar-color only when set as a hex value - + ## it rejects/ignores a colour name like "blue". (The old 'calendar_color' + ## flag was never actually exercised, because testSetCalendarProperties skips + ## on Zimbra: setting a display name relocates the calendar.) + "calendar-color": {"support": "unsupported"}, + "calendar-color.hex": {"support": "full"}, + "calendar-order": {"support": "full"}, } bedework = { @@ -1040,7 +1304,14 @@ def dotted_feature_set_list(self, compact=False): "search.recurrences": False, "sync-token": { "support": "fragile" }, 'search.comp-type': {'support': 'broken', 'behaviour': 'Server returns everything when searching for events and nothing when searching for todos'}, - 'search.comp-type.optional': {'support': 'ungraceful'}, + ## was 'ungraceful' - that was the checker bug (cnt counted the separately + ## stored journal); confirmed full 2026-06-06. + 'search.comp-type.optional': {'support': 'full'}, + ## Flaps between full and unsupported across runs - the comp-type-less + ## time-range query intermittently returns the in-range object vs nothing, + ## most likely the search-cache delay above. Marked fragile so the checker + ## skips it. Observed 2026-06-06. + 'search.time-range.comp-type-optional': {'support': 'fragile'}, 'search.is-not-defined.dtend': False, "principal-search": { "support": "ungraceful" }, ## Bedework hides past non-recurring events from REPORT without a time-range filter, @@ -1056,11 +1327,11 @@ def dotted_feature_set_list(self, compact=False): ## TODO: play with this and see if it's needed 'save-load.icalendar.related-to': {'support': 'broken', 'behaviour': 'first RELATED-TO line is preserved but subsequent RELATED-TO lines are stripped'}, - 'old_flags': [ - 'propfind_allprop_failure', - 'duplicates_not_allowed', - ], - + ## Bedework omits DAV:resourcetype from an allprop PROPFIND response. + "propfind.allprop.resourcetype": {"support": "unsupported"}, + ## (The old 'duplicates_not_allowed' flag was stale: Bedework does store a + ## second event with the same content under a different UID, so + ## save.duplicate-event is left at the default "full".) } synology = { @@ -1071,7 +1342,8 @@ def dotted_feature_set_list(self, compact=False): 'search.is-not-defined': {'support': 'fragile', 'behaviour': 'works for CLASS but not for CATEGORIES'}, 'search.text.case-sensitive': {'support': 'unsupported'}, 'search.time-range.alarm': {'support': 'unsupported'}, - 'old_flags': ['vtodo_datesearch_nodtstart_task_is_skipped'], + ## Synology skips VTODOs without DTSTART in date-range searches. + 'search.time-range.todo.no-dtstart': {'support': 'unsupported'}, 'test-calendar': {'cleanup-regime': 'wipe-calendar'}, 'scheduling.schedule-tag': False, 'scheduling.mailbox.inbox-delivery': False, @@ -1082,7 +1354,9 @@ def dotted_feature_set_list(self, compact=False): # into their calendar. "scheduling.schedule-tag": False, "http.multiplexing": "fragile", ## ref https://github.com/python-caldav/caldav/issues/564 - 'search.comp-type.optional': {'support': 'ungraceful'}, + ## was 'ungraceful' - that was the checker bug (cnt counted the journal that + ## SabreDAV stores in a separate calendar); confirmed full 2026-06-06. + 'search.comp-type.optional': {'support': 'full'}, 'search.recurrences.expanded.todo': {'support': 'unsupported'}, 'search.recurrences.includes-implicit.todo': {'support': 'unsupported'}, "search.recurrences.includes-implicit.infinite-scope": False, @@ -1091,11 +1365,9 @@ def dotted_feature_set_list(self, compact=False): 'principal-search.by-name.self': {'support': 'unsupported'}, 'principal-search.list-all': {'support': 'ungraceful'}, #'sync-token.delete': {'support': 'unsupported'}, ## Perhaps on some older servers? - 'old_flags': [ - ## extra features not specified in RFC5545 - "calendar_order", - "calendar_color", - ], + ## extra properties not specified in RFC4791/RFC5545 + "calendar-color": {"support": "full"}, + "calendar-order": {"support": "full"}, ## I'm surprised, I'm quite sure this was passing earlier. Caldav commit a98d50490b872e9b9d8e93e2e401c936ad193003, caldav server checker commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad 'search.combined-is-logical-and': False } ## TODO: testPrincipals, testWrongAuthType, testTodoDatesearch fails @@ -1107,7 +1379,10 @@ def dotted_feature_set_list(self, compact=False): } cyrus = { - "search.comp-type.optional": {"support": "ungraceful"}, + ## A bare comp-type-less query is accepted; the previous "ungraceful" was a + ## checker bug where the probe carried a time-range + ## (https://github.com/python-caldav/caldav/issues/681). + "search.comp-type.optional": {"support": "full"}, "search.recurrences.includes-implicit.infinite-scope": False, "search.time-range.alarm": {"support": "ungraceful"}, 'principal-search': {'support': 'ungraceful'}, @@ -1146,29 +1421,41 @@ def dotted_feature_set_list(self, compact=False): # DAViCal delivers iTIP notifications to the attendee inbox AND auto-schedules # into their calendar. "scheduling.schedule-tag": False, - "search.comp-type.optional": { "support": "fragile" }, + ## was 'fragile' - that was the checker bug (cnt mismatch); confirmed full 2026-06-06. + "search.comp-type.optional": { "support": "full" }, + ## Genuinely returns matching objects for a comp-type-less query that carries + ## a time-range (verified: the event is returned, not just "no error"). + "search.time-range.comp-type-optional": { "support": "full" }, "search.time-range.alarm": { "support": "unsupported" }, 'sync-token': {'support': 'fragile'}, 'principal-search': {'support': 'unsupported'}, 'principal-search.list-all': {'support': 'unsupported'}, + ## DAViCal skips VTODOs without DTSTART in date-range searches. + 'search.time-range.todo.no-dtstart': {'support': 'unsupported'}, "old_flags": [ #'no_journal', ## it threw a 500 internal server error! ## for old versions #'nofreebusy', ## for old versions ## 'fragile_sync_tokens' removed - covered by 'sync-token': {'support': 'fragile'} - 'vtodo_datesearch_nodtstart_task_is_skipped', ## no issue raised yet - 'calendar_color', - 'calendar_order', 'vtodo_datesearch_notime_task_is_skipped', ], + ## extra properties not specified in RFC4791/RFC5545 + "calendar-color": {"support": "full"}, + "calendar-order": {"support": "full"}, } sogo = { "scheduling.schedule-tag": False, "scheduling.mailbox.inbox-delivery": False, + ## SOGo rejects the calendar-color property with an error (left at the + ## default "fragile" - rejecting a nonstandard extension is fine). It + ## accepts calendar-order but echoes back a server-computed position rather + ## than the value that was set, so that property is effectively read-only. + "calendar-order": {"support": "broken", "behaviour": "read-only; server returns its own calendar position rather than the value set"}, ## I'm surprised, I'm quite sure this was passing earlier. reported unsupported with caldav commit a98d50490b872e9b9d8e93e2e401c936ad193003, caldav server checker commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad 2026-02-15 "search.text.category": False, - "search.time-range.event.old-dates": False, - "search.time-range.todo.old-dates": False, + ## old-date time-range search works (probe found, definite-future object + ## correctly excluded); the earlier "False" was an artifact of the old + ## count==1 check, which a next-year open-start DUE-only task inflated. "save-load.journal": {"support": "ungraceful"}, "search.is-not-defined": {"support": "unsupported"}, "search.text.case-sensitive": { @@ -1180,9 +1467,13 @@ def dotted_feature_set_list(self, compact=False): "search.time-range.alarm": { "support": "unsupported" }, - ## was unsupported. reported ungraceful with caldav commit a98d50490b872e9b9d8e93e2e401c936ad193003, caldav server checker commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad 2026-02-15 + ## A comp-type-less query returns nothing - with or without a time-range - + ## so both search.comp-type.optional and search.time-range.comp-type-optional + ## are unsupported (the latter is the default). The previous "ungraceful" was + ## a checker bug where the comp-type.optional probe carried a time-range that + ## SabreDAV-likes reject (https://github.com/python-caldav/caldav/issues/681). "search.comp-type.optional": { - "support": "ungraceful" + "support": "unsupported" }, ## includes-implicit.todo has been observed as both supported and unsupported ## across different test runs. Other includes-implicit children are unsupported. @@ -1260,9 +1551,12 @@ def dotted_feature_set_list(self, compact=False): 'principal-search': {'support': 'ungraceful'}, 'freebusy-query': {'support': 'ungraceful'}, "scheduling": {"support": "unsupported"}, - 'old_flags': [ - 'non_existing_raises_other', ## AuthorizationError instead of NotFoundError - ], + ## Robur answers 403 (AuthorizationError) instead of 404 (NotFoundError) when + ## looking up a non-existing resource - probably to avoid leaking whether a + ## resource exists. (Not re-probed during this migration: the Robur test + ## server was down; value carried over from the old 'non_existing_raises_other' + ## flag.) + 'non-existing-raises-not-found': {'support': 'unsupported', 'behaviour': 'raises AuthorizationError (403) instead of NotFoundError (404)'}, 'save-load.icalendar.related-to': {'support': 'unsupported'}, 'test-calendar': {'cleanup-regime': 'wipe-calendar'}, "sync-token": {"support": "ungraceful"}, @@ -1330,11 +1624,12 @@ def dotted_feature_set_list(self, compact=False): "principal-search.by-name.self": {"support": "unsupported"}, "principal-search": {"support": "ungraceful"}, "save-load.journal.mixed-calendar": {"support": "unsupported"}, - "search.comp-type.optional": {"support": "ungraceful"}, - "old_flags": [ - "calendar_order", - "calendar_color", - ], + ## was 'ungraceful' - that was the checker bug (cnt counted the journal that + ## SabreDAV stores in a separate calendar); confirmed full 2026-06-06. + "search.comp-type.optional": {"support": "full"}, + ## extra properties not specified in RFC4791/RFC5545 + "calendar-color": {"support": "full"}, + "calendar-order": {"support": "full"}, ## I'm surprised, I'm quite sure this was passing earlier. Caldav commit a98d50490b872e9b9d8e93e2e401c936ad193003, caldav server checker commit 3cae24cf99da1702b851b5a74a9b88c8e5317dad 'search.combined-is-logical-and': False } @@ -1356,25 +1651,35 @@ def dotted_feature_set_list(self, compact=False): "save.duplicate-uid.cross-calendar": {"support": "ungraceful"}, # CCS rejects multi-instance VTODOs (thisandfuture recurring completion) "save-load.todo.recurrences.thisandfuture": {"support": "unsupported"}, - "search.comp-type.optional": {"support": "ungraceful"}, - ## "full" observed, 70938dc1cbb6a839978eee4315699746d38ee5f0/3cae24cf99da1702b851b5a74a9b88c8e5317dad, 2026-02-17. - ## However, this may be due to mess with the caldav-server-checker branches. "unsupported" again at be26d42b1ca3ff3b4fd183761b4a9b024ce12b84 / 537a23b145487006bb987dee5ab9e00cdebb0492 + ## was 'ungraceful' - that was the checker bug (cnt mismatch: it counted a + ## journal object that CCS could not store, so the comp-type-less count never + ## matched). Confirmed full 2026-06-06. + ## ("full" had also been observed 2026-02-17, then "unsupported"/"ungraceful" + ## - all that flapping was the same checker bug, now fixed.) + "search.comp-type.optional": {"support": "full"}, "search.text.case-sensitive": {"support": "unsupported"}, "search.time-range.event": {"support": "full"}, "search.time-range.event.old-dates": {"support": "ungraceful"}, "search.time-range.todo": {"support": "full"}, "search.time-range.todo.old-dates": {"support": "ungraceful"}, - "search.time-range.open": {"support": "ungraceful"}, + ## open-ended time-range searches work with the near-future fixtures; CCS only + ## rejected them (ungraceful) for the old year-2000 range, so the leaves default + ## to "full" (a grouping "search.time-range.open: ungraceful" was removed here). "search.time-range.alarm": {"support": "unsupported"}, - "search.recurrences": {"support": "unsupported"}, + ## Recurrence expansion actually works within the (near-future) search window; + ## this was previously reported "unsupported" only because the test fixtures + ## lived in year 2000, which CCS's min-date-time restriction hid. Only infinite + ## scope (far-future) and server-side VTODO expansion remain unsupported. + "search.recurrences.includes-implicit.infinite-scope": {"support": "unsupported"}, + "search.recurrences.expanded.todo": {"support": "unsupported"}, "principal-search": {"support": "unsupported"}, # Ephemeral Docker container: wipe objects (avoids UID conflicts across calendars) "test-calendar": {"cleanup-regime": "wipe-calendar"}, - ## Did pass earlier, ungraceful at be26d42b1ca3ff3b4fd183761b4a9b024ce12b84 / 537a23b145487006bb987dee5ab9e00cdebb0492 - 'freebusy-query': {'support': 'ungraceful'}, - "old_flags": [ - "propfind_allprop_failure", - ], + ## freebusy-query works with the near-future fixtures; CCS rejected the + ## year-2000 range with an error, so this defaults to "full" now. + ## (The old 'propfind_allprop_failure' flag was stale: CCS does return + ## DAV:resourcetype in an allprop PROPFIND, so propfind.allprop.resourcetype + ## is left at the default "full".) } ## Stalwart - all-in-one mail & collaboration server (CalDAV added 2024/2025) @@ -1390,24 +1695,39 @@ def dotted_feature_set_list(self, compact=False): 'create-calendar.auto': True, 'principal-search': {'support': 'ungraceful'}, 'search.time-range.alarm': False, + ## Stalwart accepts comp-type-less queries fully, including the time-range + ## and prop-filter variants (both unsupported on most other servers). + ## Confirmed 2026-06-06. + 'search.time-range.comp-type-optional': {'support': 'full'}, + 'search.text.comp-type-optional': {'support': 'full'}, ## Stalwart supports implicit recurrence for datetime events but not for ## all-day (VALUE=DATE) recurring events in time-range searches. 'search.recurrences.includes-implicit.event': {'support': 'fragile', 'behaviour': 'broken for all-day (VALUE=DATE) events'}, ## Stalwart returns the recurring todo in search results but doesn't return the ## RRULE intact, so client-side expansion can't expand it to specific occurrences. 'search.recurrences.includes-implicit.todo': {'support': 'fragile'}, - ## Stalwart correctly handles exceptions in server-side CALDAV:expand (observed supported). - ## Stalwart stores master+exception VEVENTs as a single resource with 2 VEVENTs. + ## Stalwart stores master+exception VEVENTs as a single resource with 2 VEVENTs, + ## so client-side expand of the recurrence set works. 'save-load.event.recurrences.exception': {'support': 'full'}, + ## ...but server-side CALDAV:expand only suppresses the exception-overridden + ## occurrence when SEQUENCE is absent. With SEQUENCE present (as real clients + ## always emit) it returns both the original occurrence and the override. + ## Detected by the server-tester's csc_monthly_recurring_with_exception_seq fixture. + 'search.recurrences.expanded.exception': { + 'support': 'fragile', + 'behaviour': 'server-side expand fails to suppress the exception-overridden occurrence when SEQUENCE is present', + }, 'search.time-range.open': True, ## Stalwart delivers iTIP notifications to the attendee inbox AND auto-schedules ## into their calendar (verified by running CheckSchedulingInboxDelivery). "scheduling.mailbox.inbox-delivery": True, "scheduling.auto-schedule": True, - 'old_flags': [ - ## Stalwart does not return VTODO items without DTSTART in date searches - 'vtodo_datesearch_nodtstart_task_is_skipped', - ], + ## Stalwart's handling of DTSTART-less VTODOs in date searches is date + ## dependent: a near-future DUE-only task is returned (the server-tester + ## probe sees 'full'), but the old-date fixtures used by testTodoDatesearch + ## are skipped. Marked 'fragile' so the checker skips it and the integration + ## test (is_supported -> False) still treats the old-date task as skipped. + 'search.time-range.todo.no-dtstart': {'support': 'fragile'}, } ## Lots of transient problems with purelymail @@ -1494,12 +1814,24 @@ def dotted_feature_set_list(self, compact=False): ] } -## https://www.open-xchange.com/ +## https://ox.io/ ## OX App Suite CalDAV served at /caldav/ (Apache proxies to /servlet/dav/caldav on port 8009). ## The Docker image must be built locally before use (see tests/docker-test-servers/ox/build.sh). ox = { - ## Renaming a calendar after creation via PROPPATCH is not supported - 'create-calendar.set-displayname': {'support': 'unsupported'}, + ## Renaming a calendar after creation via PROPPATCH is not supported, but + ## setting the display name AT creation time is - and that's what the probe + ## tests. Was 'unsupported' (conflated the two operations, and masked by the + ## checker's display-name-lookup bug). Confirmed full 2026-06-07. + 'create-calendar.set-displayname': {'support': 'full'}, + ## OX gives EVERY calendar an opaque internal 'cal://0/NNN' canonical URL + ## (base64-encoded in the path, e.g. /caldav/Y2FsOi8vMC8xMzYw/), whether or + ## not a display name is set. The requested cal_id does resolve as a usable + ## alias (object GETs under it work, unlike Zimbra), but the canonical URL + ## still differs from the requested URL - so under the URL-stability semantics + ## this is 'unsupported', exactly like Zimbra and with no special-casing: the + ## library discovers and adopts the canonical URL after creation. (The + ## display name itself sticks, so create-calendar.set-displayname is 'full'.) + 'create-calendar.stable-url': {'support': 'unsupported', 'behaviour': "the calendar's canonical URL is an opaque cal://0/NNN (base64 path segment) that differs from the requested cal_id; the cal_id alias is usable but clients should adopt the canonical URL"}, ## VTODOs must be in a dedicated VTODO-only calendar; mixed calendars not supported 'save-load.todo.mixed-calendar': {'support': 'unsupported'}, ## Basic VTODO support works fine; only recurrences are broken @@ -1508,20 +1840,63 @@ def dotted_feature_set_list(self, compact=False): 'save-load.todo.recurrences': {'support': 'ungraceful'}, ## VJOURNAL is not supported 'save-load.journal': {'support': 'unsupported'}, + ## OX exposes the calendar both under its display name and under an internal + ## "cal://0/NNN" id, so objects looked up via REPORT come back under a + ## different calendar URL than the one used to PUT them (GET on the original + ## URL still works via an alias). + 'save-load.stable-url': {'support': 'unsupported'}, + ## OX enforces optimistic concurrency: a no-If-Match overwrite PUT is rejected + ## with 409 Conflict (etag-conditional save() still works). + 'save-load.mutable.if-match-optional': {'support': 'unsupported'}, + ## OX forbids changing an attendee's PARTSTAT via a direct PUT (403 Forbidden + ## even with a matching etag); it must go through iTIP scheduling. + 'save-load.mutable.attendee-partstat': {'support': 'unsupported'}, ## Search limitations 'search.time-range.event.old-dates': {'support': 'unsupported'}, 'search.time-range.todo.old-dates': {'support': 'unsupported'}, 'search.time-range.alarm': {'support': 'unsupported'}, 'search.unlimited-time-range': {'support': 'broken'}, - 'search.comp-type.optional': {'support': 'ungraceful'}, - 'search.text': {'support': 'unsupported'}, + ## was 'ungraceful' - that was the checker bug (cnt mismatch across the + ## separate VTODO calendar); confirmed full 2026-06-06. + 'search.comp-type.optional': {'support': 'full'}, + ## OX silently ignores the CALDAV comp-filter: a calendar-query that + ## specifies a component type returns the calendar's whole contents + ## regardless of the requested type (a VEVENT-calendar answers a VTODO query + ## with its VEVENT, and vice versa). No right-typed objects are dropped, so + ## the library recovers the correct result by post-filtering - hence + ## "unsupported" (silently ignored), not "broken". Confirmed by direct probe + ## 2026-06-09. Contrast bedework, which drops the todos (data loss = broken). + 'search.comp-type': {'support': 'unsupported'}, + ## Text search (case-sensitive, case-insensitive, substring) now works in OX. + ## Confirmed full 2026-06-13. Category search remains unsupported. + 'search.text': {'support': 'full'}, 'search.text.category': {'support': 'unsupported'}, - 'search.text.case-sensitive': {'support': 'unsupported'}, - 'search.text.case-insensitive': {'support': 'unsupported'}, - ## Recurrence searching broken (sliding window + old-dates limitation) - 'search.recurrences.includes-implicit': {'support': 'unsupported'}, + ## Recurrence searching: the sliding window hides far-past/far-future + ## occurrences, but implicit expansion of *datetime* events and server-side + ## expansion of exceptions work within the window (detectable now that the + ## fixtures are in the near future rather than year 2000). VTODO recurrence, + ## datetime-event server-side expansion, and infinite scope remain unsupported. + ## (event and exception expansion are left at the default "full".) + 'search.recurrences.includes-implicit.todo': {'support': 'unsupported'}, 'search.recurrences.includes-implicit.todo.pending': {'support': 'unsupported'}, - 'search.recurrences.expanded': {'support': 'unsupported'}, + 'search.recurrences.includes-implicit.infinite-scope': {'support': 'unsupported'}, + 'search.recurrences.expanded.event': {'support': 'unsupported'}, + 'search.recurrences.expanded.todo': {'support': 'unsupported'}, + ## Rescheduling the whole series (changing the master DTSTART) is rejected with + ## 409 Conflict once detached exceptions exist - even with a matching If-Match + ## etag. Shifting the DTSTART of an exception-free recurring event still works. + ## Confirmed by direct probe 2026-06-14. + 'save-load.event.recurrences.exception.reschedule': {'support': 'unsupported'}, + ## OX ignores the time-range on VTODO queries and returns every task + 'search.time-range.todo.strict': {'support': 'broken'}, + ## OX silently ignores the is-not-defined prop-filter and returns the whole + ## calendar regardless (confirmed by direct probe 2026-06-09: a no_category + ## search still returns the categorised event; a no_class search still + ## returns the CONFIDENTIAL event). Same "filter ignored" behaviour as + ## search.comp-type above - silently ignored, hence unsupported. + 'search.is-not-defined': {'support': 'unsupported'}, + 'search.is-not-defined.category': {'support': 'unsupported'}, + 'search.is-not-defined.class': {'support': 'unsupported'}, ## is-not-defined for DTEND is not supported 'search.is-not-defined.dtend': {'support': 'unsupported'}, ## Freebusy queries are not supported (returns 400) @@ -1538,9 +1913,63 @@ def dotted_feature_set_list(self, compact=False): "scheduling.freebusy-query": "ungraceful", 'search.time-range.open.start': "broken", 'search.time-range.open.end': True, - ## time-range.open is "broken", while time-range.open.start.duration is "unsupported"? - ## this may possibly be some problems with the checker rather than with Ox - 'search.time-range.open.start.duration': "unsupported" + ## DTSTART+DURATION components ARE found by an overlapping time-range search: + ## confirmed by direct probe 2026-06-09 for VEVENT, and the VTODO duration + ## fixture is returned too. The VTODO time-range is not honoured strictly + ## (out-of-range tasks leak in - tracked separately as + ## search.time-range.todo.strict=broken), so the checker now treats the VTODO + ## duration probe as inconclusive rather than a failure and judges this + ## feature from the conclusive VEVENT result. (Previously mis-reported as a + ## VTODO/VEVENT asymmetry; see the old "checker problem" note here.) + 'search.time-range.open.start.duration': {'support': 'full'}, +} + +## Infomaniak (https://www.infomaniak.com/) - kSuite calendar, CalDAV served at +## https://sync.infomaniak.com/ (/.well-known/caldav redirects there). Runs +## SabreDAV 4.3.1. Profiled 2026-06-15 against a freshly created dedicated +## calendar; save-load and most search features work well. +infomaniak = { + ## SabreDAV processes writes asynchronously - MKCALENDAR/PUT/DELETE return + ## before the change is queryable, so an immediate read-back 404s or returns + ## stale data for several seconds. This is server-wide (not just searches), + ## so we sleep after every write rather than only before searches. + 'write-delay': {'behaviour': 'delay', 'delay': 16}, + ## VJOURNAL is not supported. + 'save-load.journal': {'support': 'unsupported'}, + ## Calendar colour/order work once the post-write delay is honoured (the + ## hex form is normalised, e.g. '#FF0000FF' is stored as '#ff0000'). These + ## previously looked 'broken' (read-only): a read-back issued too soon + ## returned the stale value, an artifact of the asynchronous writes above. + ## Set explicitly to 'full' since the feature default is the weaker 'fragile'. + 'calendar-color': {'support': 'full'}, + 'calendar-color.hex': {'support': 'full'}, + 'calendar-order': {'support': 'full'}, + ## The CALDAV comp-filter is silently ignored: a calendar-query that requests + ## one component type returns the calendar's whole contents regardless (a + ## VJOURNAL query returned a VEVENT). No right-typed objects are dropped, so + ## the library recovers by post-filtering - hence "unsupported", not "broken". + 'search.comp-type': {'support': 'unsupported', 'behaviour': 'comp-filter silently ignored - returns the whole calendar regardless of requested component type'}, + ## Because the comp-filter is ignored, omitting it (which the RFC permits) + ## also returns the whole calendar - so the "optional comp-type" behaviour + ## works. The parent is 'unsupported', so this child must say so explicitly, + ## otherwise it inherits 'unsupported' and disagrees with the observation. + 'search.comp-type.optional': {'support': 'full'}, + ## A combined (logical-AND) filter is not honoured. + 'search.combined-is-logical-and': {'support': 'unsupported'}, + ## VTODO recurrence searching is not supported (datetime VEVENT recurrence + ## search, including server-side expand and infinite scope, works fine). + 'search.recurrences.includes-implicit.todo': {'support': 'unsupported'}, + 'search.recurrences.includes-implicit.todo.pending': {'support': 'unsupported'}, + 'search.recurrences.expanded.todo': {'support': 'unsupported'}, + ## Scheduling is advertised and the calendar-user-address-set and scheduling + ## mailbox are present, but the server never returns a Schedule-Tag (neither + ## on GET nor via PROPFIND). + 'scheduling.schedule-tag': {'support': 'unsupported', 'behaviour': 'no Schedule-Tag returned on GET or via PROPFIND'}, + 'scheduling.schedule-tag.stable-partstat': {'support': 'unsupported'}, + ## Principal search is effectively unsupported (lists nothing / errors out). + 'principal-search': {'support': 'ungraceful'}, + 'principal-search.by-name.self': {'support': 'unsupported'}, + 'principal-search.list-all': {'support': 'ungraceful'}, } # fmt: on diff --git a/caldav/config.py b/caldav/config.py index 05c8af72..07fc585a 100644 --- a/caldav/config.py +++ b/caldav/config.py @@ -33,6 +33,8 @@ def expand_config_section(config, section="default", blacklist=None): ## If it's not a glob-pattern ... if set(section).isdisjoint(set("[*?")): + if section not in config: + return [] ## If it's referring to a "meta section" with the "contains" keyword if "contains" in config[section]: results = [] @@ -47,7 +49,7 @@ def expand_config_section(config, section="default", blacklist=None): return results else: ## Disabled sections should be ignored - if config.get("section", {}).get("disable", False): + if config.get(section, {}).get("disable", False): return [] ## NORMAL CASE - return [ section ] @@ -181,7 +183,7 @@ def resolve_features(features): feature_name = features if feature_name.startswith("compatibility_hints."): feature_name = feature_name[len("compatibility_hints.") :] - return getattr(caldav.compatibility_hints, feature_name) + return copy.deepcopy(getattr(caldav.compatibility_hints, feature_name)) if isinstance(features, dict) and "base" in features: base_name = features["base"] if isinstance(base_name, str): @@ -262,16 +264,24 @@ def get_connection_params( Dict with connection parameters (url, username, password, etc.) or None if no configuration found. """ - # 1. Explicit parameters take highest priority - if explicit_params: - # Filter to valid connection keys - conn_params = {k: v for k, v in explicit_params.items() if k in CONNKEYS} - if conn_params.get("url") or conn_params.get("features"): - # Return when URL is given, or when features are given (the - # client constructor resolves URL from auto-connect.url hints - # via _auto_url()). Don't fall through to env vars/config - # files when the caller explicitly provided connection info. - return conn_params + # 1. Explicit parameters take highest priority. + # A kwarg whose value is None counts as "not supplied" rather than + # "unset it" - the common CLI wrapper get_davclient(url=args.url, + # username=args.user, password=args.password) passes None for every + # option the user left out, and overlaying those on the winning source + # would wipe CALDAV_URL and friends. An empty string is kept: it is + # meaningful for servers with no authentication. + explicit_conn = ( + {k: v for k, v in explicit_params.items() if k in CONNKEYS and v is not None} + if explicit_params + else {} + ) + if explicit_conn.get("url") or explicit_conn.get("features"): + # Return when URL is given, or when features are given (the + # client constructor resolves URL from auto-connect.url hints + # via _auto_url()). Don't fall through to env vars/config + # files when the caller explicitly provided connection info. + return explicit_conn # Check for config file path from environment early (needed for test server config too) if environment: @@ -284,6 +294,9 @@ def get_connection_params( if testconfig or (environment and os.environ.get("PYTHON_CALDAV_USE_TEST_SERVER")): conn = _get_test_server_config(name, environment, config_file) if conn is not None: + # Explicit kwargs outrank the discovered test server, same as for + # the environment and config-file sources below. + conn.update(explicit_conn) return conn # In test mode, don't fall through to regular config - return None # This prevents accidentally using personal/production servers for testing @@ -297,14 +310,19 @@ def get_connection_params( if environment: conn_params = _get_env_config() if conn_params: + conn_params.update(explicit_conn) return conn_params # 4. Config file if check_config_file: conn_params = _get_file_config(config_file, config_section) if conn_params: + conn_params.update(explicit_conn) return conn_params + # No env/config source matched. At this point explicit_conn has neither + # 'url' nor 'features' (those return early above), so it cannot produce a + # connectable client — treat it as "no configuration found". return None @@ -334,7 +352,7 @@ def _get_file_config(file_path: str | None, section_name: str | None) -> dict[st return None section_data = config_section(cfg, section_name) - return _extract_conn_params_from_section(section_data) + return extract_conn_params_from_section(section_data) def _get_test_server_config( @@ -496,14 +514,22 @@ def _test_server_to_params(server: Any, was_already_started: bool) -> dict[str, return params -def _extract_conn_params_from_section(section_data: dict[str, Any]) -> dict[str, Any] | None: +def extract_conn_params_from_section(section_data: dict[str, Any]) -> dict[str, Any] | None: """Extract connection parameters from a config section dict. - Returns a dict containing only CONNKEYS entries. Returns ``None`` if no - server URL is present. Calendar filter keys (``calendar_name``, - ``calendar_url``) are intentionally excluded — callers that need them - (e.g. :func:`get_all_file_connection_params`) read ``section_data`` - directly. + Keys prefixed with ``caldav_`` are mapped to client constructor parameters + (with ``caldav_user``/``caldav_pass`` accepted as aliases for + username/password), environment variable references are expanded, and a + ``features`` key is resolved through :func:`resolve_features`. Public so + that downstream tools (e.g. plann) can reuse it on plann-style config + sections. + + Returns a dict containing only CONNKEYS entries. Returns ``None`` if + neither a server URL nor features are present (with features, the client + constructor can resolve the URL from auto-connect.url hints). Calendar + filter keys (``calendar_name``, ``calendar_url``) are intentionally + excluded — callers that need them (e.g. + :func:`get_all_file_connection_params`) read ``section_data`` directly. """ conn_params: dict[str, Any] = {} for k in section_data: @@ -522,7 +548,7 @@ def _extract_conn_params_from_section(section_data: dict[str, Any]) -> dict[str, elif k == "features" and section_data[k]: conn_params["features"] = resolve_features(section_data[k]) - return conn_params if conn_params.get("url") else None + return conn_params if (conn_params.get("url") or conn_params.get("features")) else None def get_all_file_connection_params( @@ -540,7 +566,7 @@ def get_all_file_connection_params( ``calendar_url`` calendar-filter keys read from the config section. Returns an empty list when the config file is absent or the section has - no usable URL. + neither a usable URL nor features to derive one from. """ if not section_name: section_name = "default" @@ -553,7 +579,7 @@ def get_all_file_connection_params( result: list[dict[str, Any]] = [] for s in sections: section_data = config_section(cfg, s) - params = _extract_conn_params_from_section(section_data) + params = extract_conn_params_from_section(section_data) if params: # Add calendar filter keys — these must NOT flow into DAVClient() for k in ("calendar_name", "calendar_url"): @@ -588,7 +614,7 @@ def get_all_test_servers( for section_name in cfg: section_data = config_section(cfg, section_name) if section_data.get("testing_allowed"): - conn_params = _extract_conn_params_from_section(section_data) + conn_params = extract_conn_params_from_section(section_data) if conn_params: # Also copy the raw section data for keys not in CONNKEYS # (e.g., testing_allowed itself, or custom keys) diff --git a/caldav/datastate.py b/caldav/datastate.py index 72c89dc2..85836cb4 100644 --- a/caldav/datastate.py +++ b/caldav/datastate.py @@ -64,18 +64,18 @@ def get_uid(self) -> str | None: """ cal = self.get_icalendar_copy() for comp in cal.subcomponents: - if comp.name in ("VEVENT", "VTODO", "VJOURNAL", "FREEBUSY") and "UID" in comp: + if comp.name in ("VEVENT", "VTODO", "VJOURNAL", "VFREEBUSY") and "UID" in comp: return str(comp["UID"]) return None def get_component_type(self) -> str | None: - """Get the component type (VEVENT, VTODO, VJOURNAL, FREEBUSY) without full parsing. + """Get the component type (VEVENT, VTODO, VJOURNAL, VFREEBUSY) without full parsing. Default implementation parses the data, but subclasses can optimize. """ cal = self.get_icalendar_copy() for comp in cal.subcomponents: - if comp.name in ("VEVENT", "VTODO", "VJOURNAL", "FREEBUSY"): + if comp.name in ("VEVENT", "VTODO", "VJOURNAL", "VFREEBUSY"): return comp.name return None @@ -149,7 +149,7 @@ def get_component_type(self) -> str | None: return "VTODO" elif "BEGIN:VJOURNAL" in self._data: return "VJOURNAL" - elif "BEGIN:FREEBUSY" in self._data: + elif "BEGIN:VFREEBUSY" in self._data: return "VFREEBUSY" return None diff --git a/caldav/davclient.py b/caldav/davclient.py index 74d37803..ee86009b 100644 --- a/caldav/davclient.py +++ b/caldav/davclient.py @@ -223,7 +223,12 @@ def __init__( preventing DNS-based downgrade attacks where malicious DNS could redirect to unencrypted HTTP. Set to False ONLY if you need to support non-TLS servers and trust your DNS infrastructure. - This parameter has no effect if enable_rfc6764=False. + SCOPE: this only gates the RFC6764 discovery path. It has no + effect when enable_rfc6764=False, and does NOT reject an + explicitly-passed http:// URL (e.g. url="http://your.server.example.com/dav/" + still connects over plaintext despite require_tls=True). + Making enforcement global is deferred to 4.0 — see + https://github.com/python-caldav/caldav/issues/687 rate_limit_handle: boolean, whether to automatically sleep and retry when the server responds with 429 Too Many Requests or 503 Service Unavailable. Default: False (raise RateLimitError immediately). @@ -297,9 +302,16 @@ def __init__( } ) self.headers.update(headers or CaseInsensitiveDict()) - if self.url.username is not None: + ## An explicit username discards the URL credentials wholesale: they + ## belong to a different account, and merging them field by field + ## would let DAVClient(url="https://bob:hunter2@cal.example.com/", + ## username="alice") ship alice's login with bob's password. + ## Overriding only the password is a different thing - the username + ## still comes from the URL, so the pair stays coherent. + if self.url.username is not None and username is None: username = unquote(self.url.username) - password = unquote(self.url.password) + if password is None and self.url.password is not None: + password = unquote(self.url.password) # Use discovered username if no explicit username was provided if username is None and discovered_username is not None: @@ -331,19 +343,9 @@ def __init__( self._principal = None - rate_limit = self.features.is_supported("rate-limit", dict) - if rate_limit_handle is None: - if rate_limit and rate_limit.get("enable"): - rate_limit_handle = True - if "default_sleep" in rate_limit: - rate_limit_default_sleep = rate_limit["default_sleep"] - if "max_sleep" in rate_limit: - rate_limit_max_sleep = rate_limit["max_sleep"] - else: - rate_limit_handle = False - self.rate_limit_handle = rate_limit_handle - self.rate_limit_default_sleep = rate_limit_default_sleep - self.rate_limit_max_sleep = rate_limit_max_sleep + self._init_rate_limit_config( + rate_limit_handle, rate_limit_default_sleep, rate_limit_max_sleep + ) def __enter__(self) -> Self: ## Used for tests, to set up a temporarily test server @@ -466,13 +468,6 @@ def get_calendars(self, principal: Principal | None = None) -> list[Calendar]: for cal in calendars: print(f"Calendar: {cal.get_display_name()}") """ - from caldav.collection import ( - _extract_calendar_home_set_from_results as extract_home_set, - ) - from caldav.collection import ( - _extract_calendars_from_propfind_results as extract_calendars, - ) - if principal is None: principal = self.principal() @@ -482,14 +477,7 @@ def get_calendars(self, principal: Principal | None = None) -> list[Calendar]: props=self.CALENDAR_HOME_SET_PROPS, depth=0, ) - calendar_home_url = extract_home_set(response.results) - if not calendar_home_url: - # Fall back to the principal URL as calendar home - # (some servers like GMX don't support calendar-home-set) - calendar_home_url = str(principal.url) - - # Make URL absolute if relative - calendar_home_url = self._make_absolute_url(calendar_home_url) + calendar_home_url = self._calendar_home_url(response, principal) # Fetch calendars via PROPFIND response = self.propfind( @@ -498,14 +486,7 @@ def get_calendars(self, principal: Principal | None = None) -> list[Calendar]: depth=1, ) - # Process results using shared helper - calendar_infos = extract_calendars(response.results) - - # Convert CalendarInfo objects to Calendar objects - return [ - Calendar(client=self, url=info.url, name=info.name, id=info.cal_id) - for info in calendar_infos - ] + return self._build_calendars_from_propfind(response) def search_calendar( self, @@ -825,20 +806,7 @@ def request( try: return self._sync_request(url, method, body, headers) except error.RateLimitError as e: - if not self.rate_limit_handle: - raise - sleep_seconds = error.compute_sleep_seconds( - e.retry_after_seconds, - self.rate_limit_default_sleep, - self.rate_limit_max_sleep, - ) - if rate_limit_time_slept: - sleep_seconds += rate_limit_time_slept / 2 - if sleep_seconds is None or ( - self.rate_limit_max_sleep is not None - and rate_limit_time_slept > self.rate_limit_max_sleep - ): - raise + sleep_seconds = self._rate_limit_sleep_seconds(e, rate_limit_time_slept) time.sleep(sleep_seconds) return self.request(url, method, body, headers, rate_limit_time_slept + sleep_seconds) diff --git a/caldav/davobject.py b/caldav/davobject.py index f2a2383b..e25e5de2 100644 --- a/caldav/davobject.py +++ b/caldav/davobject.py @@ -407,6 +407,13 @@ def _post_get_properties(self, response, props, parse_response_xml, parse_props) if not parse_response_xml: return response + ## A 207 whose responses are all bare 404s means the resource we + ## asked about is not there. Without this the propstat-oriented + ## parsing below finds nothing and hands the caller a dict of None + ## values instead - see DAVResponse.all_responses_not_found(). + if response.all_responses_not_found(): + raise error.NotFoundError(f"{self.url} not found on the server") + # Use protocol layer results when available and parse_props=True if parse_props and response.results: # Convert results to the expected {href: {tag: value}} format diff --git a/caldav/discovery.py b/caldav/discovery.py index c240858b..08c74387 100644 --- a/caldav/discovery.py +++ b/caldav/discovery.py @@ -393,6 +393,10 @@ def discover_service( DNS-based downgrade attacks to plaintext HTTP. Set to False only if you explicitly need to support non-TLS servers and trust your DNS infrastructure. + NOTE: this gates the discovery path only; the client does not + enforce TLS on explicitly-passed URLs. Global enforcement is + deferred to 4.0 — see + https://github.com/python-caldav/caldav/issues/687 Returns: ServiceInfo object with discovered service details, or None if discovery fails @@ -481,10 +485,16 @@ def discover_service( well_known_info = _well_known_lookup(domain, service_type, timeout, ssl_verify_cert) if well_known_info: - # Preserve username from email address - well_known_info.username = username - log.info(f"Discovered {service_type} service via well-known URI: {well_known_info.url}") - return well_known_info + if require_tls and not well_known_info.tls: + log.warning( + f"require_tls=True: Rejecting well-known redirect to non-TLS URL " + f"{well_known_info.url!r} — possible misconfiguration or downgrade attack" + ) + else: + # Preserve username from email address + well_known_info.username = username + log.info(f"Discovered {service_type} service via well-known URI: {well_known_info.url}") + return well_known_info # All discovery methods failed log.warning(f"Failed to discover {service_type} service for {domain}") diff --git a/caldav/jmap/async_client.py b/caldav/jmap/async_client.py index d87fa682..d9bc6c06 100644 --- a/caldav/jmap/async_client.py +++ b/caldav/jmap/async_client.py @@ -3,25 +3,26 @@ Mirrors JMAPClient with all public methods as coroutines. Uses niquests.AsyncSession for HTTP — niquests is a core dependency. + +All response-parsing logic lives in _JMAPClientBase (client.py); each method +here is a ~3-line async wrapper: get session, send request, delegate to parser. """ from __future__ import annotations import logging import uuid +import warnings from niquests import AsyncSession -from caldav.jmap._methods.calendar import build_calendar_get, parse_calendar_get +from caldav.jmap._methods.calendar import build_calendar_get from caldav.jmap._methods.event import ( build_event_changes, build_event_get, build_event_set_create, build_event_set_destroy, build_event_set_update, - parse_event_changes, - parse_event_get, - parse_event_set, ) from caldav.jmap._methods.task import ( build_task_get, @@ -29,8 +30,6 @@ build_task_set_create, build_task_set_destroy, build_task_set_update, - parse_task_list_get, - parse_task_set, ) from caldav.jmap.client import _DEFAULT_USING, _TASK_USING, _JMAPClientBase from caldav.jmap.convert import ical_to_jscal @@ -64,11 +63,46 @@ class AsyncJMAPClient(_JMAPClientBase): timeout: HTTP request timeout in seconds. """ + def _get_http_session(self) -> AsyncSession: + """Return the persistent async HTTP session, creating it on first call.""" + if self._http_session is None: + sess = AsyncSession() + sess.auth = self._auth + sess.headers.update({"Content-Type": "application/json", "Accept": "application/json"}) + self._http_session = sess + return self._http_session + + async def aclose(self) -> None: + """Release the persistent HTTP session and its connection pool. + + Only needed when the client was not used as an async context manager + -- the documented Quick Start builds one directly. Idempotent; the + session is recreated on the next request. + """ + if self._http_session is not None: + await self._http_session.close() + self._http_session = None + async def __aenter__(self) -> AsyncJMAPClient: + self._get_http_session() return self async def __aexit__(self, exc_type, exc_val, exc_tb) -> None: - return None + await self.aclose() + + def __del__(self) -> None: + ## Closing an async session needs an event loop, which is long gone + ## by the time __del__ runs, so all we can do is say so. + try: + if self._http_session is not None: + warnings.warn( + f"{type(self).__name__} was garbage collected with an open HTTP " + "session; use 'async with' or await aclose()", + ResourceWarning, + stacklevel=2, + ) + except Exception: + pass async def _get_session(self) -> Session: """Return the cached Session, fetching it on first call.""" @@ -103,14 +137,11 @@ async def _request(self, method_calls: list[tuple], using: list[str] | None = No log.debug("JMAP POST to %s: %d method call(s)", session.api_url, len(method_calls)) - async with AsyncSession() as http: - response = await http.post( - session.api_url, - json=payload, - auth=self._auth, - headers={"Content-Type": "application/json", "Accept": "application/json"}, - timeout=self.timeout, - ) + response = await self._get_http_session().post( + session.api_url, + json=payload, + timeout=self.timeout, + ) if response.status_code in (401, 403): raise JMAPAuthError( @@ -142,18 +173,8 @@ async def get_calendars(self) -> list[JMAPCalendar]: List of :class:`~caldav.jmap.objects.calendar.JMAPCalendar` objects. """ session = await self._get_session() - call = build_calendar_get(session.account_id) - responses = await self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "Calendar/get": - calendars = parse_calendar_get(resp_args) - for cal in calendars: - cal._client = self - cal._is_async = True - return calendars - - return [] + responses = await self._request([build_calendar_get(session.account_id)]) + return self._parse_get_calendars(responses, self, True) async def create_event(self, calendar_id: str, ical_str: str) -> str: """Create a calendar event from an iCalendar string. @@ -172,20 +193,7 @@ async def create_event(self, calendar_id: str, ical_str: str) -> str: jscal = ical_to_jscal(ical_str, calendar_id=calendar_id) call = build_event_set_create(session.account_id, {"new-0": jscal}) responses = await self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - created, _, _, not_created, _, _ = parse_event_set(resp_args) - if "new-0" in not_created: - self._raise_set_error(session, not_created["new-0"]) - if "new-0" not in created: - raise JMAPMethodError( - url=session.api_url, - reason="CalendarEvent/set response missing created entry for new-0", - ) - return created["new-0"]["id"] - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + return self._parse_create_event_response(responses, session.api_url) async def get_event(self, event_id: str) -> JMAPCalendarObject: """Fetch a calendar event as an iCalendar string. @@ -203,21 +211,8 @@ async def get_event(self, event_id: str) -> JMAPCalendarObject: JMAPMethodError: If the event is not found. """ session = await self._get_session() - call = build_event_get(session.account_id, ids=[event_id]) - responses = await self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - items = parse_event_get(resp_args) - if not items: - raise JMAPMethodError( - url=session.api_url, - reason=f"Event not found: {event_id}", - error_type="notFound", - ) - return JMAPCalendarObject(data=items[0], parent=None) - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/get response") + responses = await self._request([build_event_get(session.account_id, ids=[event_id])]) + return self._parse_get_event_response(responses, session.api_url, event_id) async def update_event(self, event_id: str, ical_str: str) -> None: """Update a calendar event from an iCalendar string. @@ -230,19 +225,17 @@ async def update_event(self, event_id: str, ical_str: str) -> None: JMAPMethodError: If the server rejects the update. """ session = await self._get_session() - patch = ical_to_jscal(ical_str) - patch.pop("uid", None) # uid is server-immutable after creation; patch must omit it - call = build_event_set_update(session.account_id, {event_id: patch}) - responses = await self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - _, _, _, _, not_updated, _ = parse_event_set(resp_args) - if event_id in not_updated: - self._raise_set_error(session, not_updated[event_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + patch, nulled = self._build_event_update_patch(ical_str) + while True: + responses = await self._request( + [build_event_set_update(session.account_id, {event_id: patch})] + ) + drop = self._unsupported_null_keys(responses, event_id, patch, nulled) + if not drop: + break + for key in drop: + patch.pop(key, None) + self._parse_update_event_response(responses, session.api_url, event_id) async def _search( self, @@ -255,15 +248,7 @@ async def _search( session = await self._get_session() calls = self._build_event_search_calls(session.account_id, calendar_id, start, end, text) responses = await self._request(calls) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - return [ - JMAPCalendarObject(data=item, parent=parent) - for item in parse_event_get(resp_args) - ] - - return [] + return self._parse_search_response(responses, parent) async def search_events( self, @@ -302,16 +287,12 @@ async def get_sync_token(self) -> str: retrieve only what changed since this point. """ session = await self._get_session() - call = build_event_get(session.account_id, ids=[]) - responses = await self._request([call]) - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - return resp_args.get("state", "") - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/get response") + responses = await self._request([build_event_get(session.account_id, ids=[])]) + return self._parse_get_sync_token_response(responses, session.api_url) async def get_objects_by_sync_token( self, sync_token: str - ) -> tuple[list[JMAPCalendarObject], list[JMAPCalendarObject], list[str]]: + ) -> tuple[list[JMAPCalendarObject], list[JMAPCalendarObject], list[str], str]: """Fetch events changed since a previous sync token. Calls ``CalendarEvent/changes`` to discover which events were created, @@ -325,53 +306,28 @@ async def get_objects_by_sync_token( or by a prior call to this method. Returns: - A 3-tuple ``(added, modified, deleted)``: + A 4-tuple ``(added, modified, deleted, new_sync_token)``: - ``added``: objects for newly created events (``parent`` is ``None``). - ``modified``: objects for updated events (``parent`` is ``None``). - ``deleted``: Event IDs that were destroyed. + - ``new_sync_token``: Pass to the next call to this method as ``sync_token``. Raises: JMAPMethodError: If the server reports ``hasMoreChanges: true``. """ session = await self._get_session() - changes_call = build_event_changes(session.account_id, sync_token) - responses = await self._request([changes_call]) - - created_ids: list[str] = [] - updated_ids: list[str] = [] - destroyed: list[str] = [] - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/changes": - _, _, has_more, created_ids, updated_ids, destroyed = parse_event_changes(resp_args) - if has_more: - raise JMAPMethodError( - url=session.api_url, - reason=( - "CalendarEvent/changes response was truncated by the server " - "(hasMoreChanges=true). Call get_sync_token() to obtain a " - "fresh baseline and re-sync." - ), - error_type="serverPartialFail", - ) - + responses = await self._request([build_event_changes(session.account_id, sync_token)]) + created_ids, updated_ids, destroyed, new_sync_token = self._parse_event_changes_response( + responses, session.api_url + ) fetch_ids = created_ids + updated_ids if not fetch_ids: - return [], [], destroyed - - get_call = build_event_get(session.account_id, ids=fetch_ids) - get_responses = await self._request([get_call]) - - events_by_id: dict[str, JMAPCalendarObject] = {} - for method_name, resp_args, _ in get_responses: - if method_name == "CalendarEvent/get": - for item in parse_event_get(resp_args): - events_by_id[item["id"]] = JMAPCalendarObject(data=item, parent=None) - - added = [events_by_id[i] for i in created_ids if i in events_by_id] - modified = [events_by_id[i] for i in updated_ids if i in events_by_id] - return added, modified, destroyed + return [], [], destroyed, new_sync_token + get_responses = await self._request([build_event_get(session.account_id, ids=fetch_ids)]) + return self._assemble_sync_token_result( + get_responses, created_ids, updated_ids, destroyed, new_sync_token + ) async def delete_event(self, event_id: str) -> None: """Delete a calendar event. @@ -383,17 +339,8 @@ async def delete_event(self, event_id: str) -> None: JMAPMethodError: If the server rejects the delete. """ session = await self._get_session() - call = build_event_set_destroy(session.account_id, [event_id]) - responses = await self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - _, _, _, _, _, not_destroyed = parse_event_set(resp_args) - if event_id in not_destroyed: - self._raise_set_error(session, not_destroyed[event_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + responses = await self._request([build_event_set_destroy(session.account_id, [event_id])]) + self._parse_delete_event_response(responses, session.api_url, event_id) async def _get_object_by_uid( self, uid: str, calendar_id: str | None = None, parent: JMAPCalendar | None = None @@ -414,14 +361,10 @@ async def get_task_lists(self) -> list[dict]: List of raw JMAP TaskList dicts as returned by the server. """ session = await self._get_session() - call = build_task_list_get(session.account_id) - responses = await self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "TaskList/get": - return parse_task_list_get(resp_args) - - return [] + responses = await self._request( + [build_task_list_get(session.account_id)], using=_TASK_USING + ) + return self._parse_get_task_lists_response(responses) async def create_task(self, task_list_id: str, title: str, **kwargs) -> str: """Create a task in a task list. @@ -452,15 +395,7 @@ async def create_task(self, task_list_id: str, title: str, **kwargs) -> str: task_dict.update(kwargs) call = build_task_set_create(session.account_id, {"new-0": task_dict}) responses = await self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - created, _, _, not_created, _, _ = parse_task_set(resp_args) - if "new-0" in not_created: - self._raise_set_error(session, not_created["new-0"]) - return created["new-0"]["id"] - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + return self._parse_create_task_response(responses, session.api_url) async def get_task(self, task_id: str) -> dict: """Fetch a task by ID. @@ -475,21 +410,10 @@ async def get_task(self, task_id: str) -> dict: JMAPMethodError: If the task is not found. """ session = await self._get_session() - call = build_task_get(session.account_id, ids=[task_id]) - responses = await self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/get": - items = resp_args.get("list", []) - if not items: - raise JMAPMethodError( - url=session.api_url, - reason=f"Task not found: {task_id}", - error_type="notFound", - ) - return items[0] - - raise JMAPMethodError(url=session.api_url, reason="No Task/get response") + responses = await self._request( + [build_task_get(session.account_id, ids=[task_id])], using=_TASK_USING + ) + return self._parse_get_task_response(responses, session.api_url, task_id) async def update_task(self, task_id: str, patch: dict) -> None: """Update a task with a partial patch. @@ -504,15 +428,7 @@ async def update_task(self, task_id: str, patch: dict) -> None: session = await self._get_session() call = build_task_set_update(session.account_id, {task_id: patch}) responses = await self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - _, _, _, _, not_updated, _ = parse_task_set(resp_args) - if task_id in not_updated: - self._raise_set_error(session, not_updated[task_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + self._parse_update_task_response(responses, session.api_url, task_id) async def delete_task(self, task_id: str) -> None: """Delete a task. @@ -524,14 +440,7 @@ async def delete_task(self, task_id: str) -> None: JMAPMethodError: If the server rejects the delete. """ session = await self._get_session() - call = build_task_set_destroy(session.account_id, [task_id]) - responses = await self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - _, _, _, _, _, not_destroyed = parse_task_set(resp_args) - if task_id in not_destroyed: - self._raise_set_error(session, not_destroyed[task_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + responses = await self._request( + [build_task_set_destroy(session.account_id, [task_id])], using=_TASK_USING + ) + self._parse_delete_task_response(responses, session.api_url, task_id) diff --git a/caldav/jmap/client.py b/caldav/jmap/client.py index 59cd333b..89e60452 100644 --- a/caldav/jmap/client.py +++ b/caldav/jmap/client.py @@ -43,6 +43,7 @@ ) from caldav.jmap.constants import CALENDAR_CAPABILITY, CORE_CAPABILITY, TASK_CAPABILITY from caldav.jmap.convert import ical_to_jscal +from caldav.jmap.convert._patch import _NULL_FOR_UPDATE from caldav.jmap.error import JMAPAuthError, JMAPMethodError from caldav.jmap.objects.calendar import JMAPCalendar from caldav.jmap.objects.calendar_object import JMAPCalendarObject @@ -70,6 +71,7 @@ def __init__( self.password = password self.timeout = timeout self._session_cache: Session | None = None + self._http_session = None if auth is not None: self._auth = auth @@ -118,9 +120,10 @@ def _build_auth(self, auth_type: str | None): reason=f"Unsupported auth_type {effective_type!r}. Use 'basic' or 'bearer'.", ) - def _raise_set_error(self, session: Session, err: dict) -> None: + @staticmethod + def _raise_set_error(api_url: str, err: dict) -> None: raise JMAPMethodError( - url=session.api_url, + url=api_url, reason=f"set failed: {err}", error_type=err.get("type", "serverError"), ) @@ -158,6 +161,256 @@ def _build_event_search_calls( ) return [query_call, get_call] + @staticmethod + def _build_event_update_patch(ical_str: str) -> tuple[dict, frozenset[str]]: + """Build a JSCalendar PatchObject for a ``CalendarEvent/set`` update. + + RFC 8620 merge semantics preserve properties absent from the patch, so + any optional property removed client-side must be explicitly nulled to + actually clear it server-side. Returns the patch together with the set + of keys that were null-injected purely for this cleanup (i.e. were not + present in the converted iCalendar) so the caller can drop them if the + server refuses to null a property it does not support. + """ + patch = ical_to_jscal(ical_str) + patch.pop("uid", None) # uid is server-immutable after creation; patch must omit it + nulled: set[str] = set() + for key in _NULL_FOR_UPDATE: + if key not in patch: + patch[key] = None + nulled.add(key) + return patch, frozenset(nulled) + + @staticmethod + def _unsupported_null_keys( + responses: list, event_id: str, patch: dict, nulled: frozenset[str] + ) -> set[str] | None: + """Detect an update that failed *only* because the server rejects + null-clearing of properties it does not support. + + Some servers (e.g. Stalwart for ``recurrenceRules``) reject a property + outright in ``CalendarEvent/set``, even when it is being set to ``null``. + Nulling such a property is harmless cleanup — it was absent from the new + iCalendar — so we report it as droppable, letting the caller retry the + update without it. + + Returns the set of droppable keys when the failure is exactly this case, + or ``None`` when the update succeeded or failed for a genuine reason (in + which case the caller proceeds to :meth:`_parse_update_event_response`, + which raises the real error). Some servers report only one offending + property per response, so the caller retries in a loop, dropping the + reported keys until the update succeeds or hits a genuine error; each + returned key is guaranteed still present in ``patch``, so the loop + strictly shrinks the patch and terminates. + """ + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/set": + _, _, _, _, not_updated, _ = parse_event_set(resp_args) + err = not_updated.get(event_id) + if not err or err.get("type") != "invalidProperties": + return None + props = set(err.get("properties") or []) + droppable = {p for p in props if p in nulled and p in patch and patch[p] is None} + # Only retry when every offending property is null-cleanup we can + # safely omit; if the client actually set one of them to a value, + # the rejection is genuine and must surface. + if props and props == droppable: + return droppable + return None + return None + + # --------------------------------------------------------------------------- + # Shared response parsers — pure synchronous; used by both sync and async + # clients. Each method takes the raw ``methodResponses`` list returned by + # ``_request()`` plus whatever extra context is needed to build the result + # or raise an informative error, and returns/raises exactly what the public + # method should return/raise. + # --------------------------------------------------------------------------- + + @staticmethod + def _parse_get_calendars(responses: list, client, is_async: bool) -> list[JMAPCalendar]: + for method_name, resp_args, _ in responses: + if method_name == "Calendar/get": + calendars = parse_calendar_get(resp_args) + for cal in calendars: + cal._client = client + cal._is_async = is_async + return calendars + return [] + + @staticmethod + def _parse_create_event_response(responses: list, api_url: str) -> str: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/set": + created, _, _, not_created, _, _ = parse_event_set(resp_args) + if "new-0" in not_created: + _JMAPClientBase._raise_set_error(api_url, not_created["new-0"]) + if "new-0" not in created: + raise JMAPMethodError( + url=api_url, + reason="CalendarEvent/set response missing created entry for new-0", + ) + return created["new-0"]["id"] + raise JMAPMethodError(url=api_url, reason="No CalendarEvent/set response") + + @staticmethod + def _parse_get_event_response( + responses: list, api_url: str, event_id: str + ) -> JMAPCalendarObject: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/get": + items = parse_event_get(resp_args) + if not items: + raise JMAPMethodError( + url=api_url, + reason=f"Event not found: {event_id}", + error_type="notFound", + ) + return JMAPCalendarObject(data=items[0], parent=None) + raise JMAPMethodError(url=api_url, reason="No CalendarEvent/get response") + + @staticmethod + def _parse_update_event_response(responses: list, api_url: str, event_id: str) -> None: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/set": + _, _, _, _, not_updated, _ = parse_event_set(resp_args) + if event_id in not_updated: + _JMAPClientBase._raise_set_error(api_url, not_updated[event_id]) + return + raise JMAPMethodError(url=api_url, reason="No CalendarEvent/set response") + + @staticmethod + def _parse_search_response( + responses: list, parent: JMAPCalendar | None + ) -> list[JMAPCalendarObject]: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/get": + return [ + JMAPCalendarObject(data=item, parent=parent) + for item in parse_event_get(resp_args) + ] + return [] + + @staticmethod + def _parse_get_sync_token_response(responses: list, api_url: str) -> str: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/get": + return resp_args.get("state", "") + raise JMAPMethodError(url=api_url, reason="No CalendarEvent/get response") + + @staticmethod + def _parse_event_changes_response( + responses: list, api_url: str + ) -> tuple[list[str], list[str], list[str], str]: + """Parse a CalendarEvent/changes response. + + Returns ``(created_ids, updated_ids, destroyed_ids, new_sync_token)``. + Raises :class:`JMAPMethodError` when the server truncated the result. + """ + created_ids: list[str] = [] + updated_ids: list[str] = [] + destroyed: list[str] = [] + new_sync_token: str = "" + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/changes": + _, new_sync_token, has_more, created_ids, updated_ids, destroyed = ( + parse_event_changes(resp_args) + ) + if has_more: + raise JMAPMethodError( + url=api_url, + reason=( + "CalendarEvent/changes response was truncated by the server " + "(hasMoreChanges=true). Call get_sync_token() to obtain a " + "fresh baseline and re-sync." + ), + error_type="serverPartialFail", + ) + return created_ids, updated_ids, destroyed, new_sync_token + + @staticmethod + def _assemble_sync_token_result( + get_responses: list, + created_ids: list[str], + updated_ids: list[str], + destroyed: list[str], + new_sync_token: str, + ) -> tuple[list[JMAPCalendarObject], list[JMAPCalendarObject], list[str], str]: + events_by_id: dict[str, JMAPCalendarObject] = {} + for method_name, resp_args, _ in get_responses: + if method_name == "CalendarEvent/get": + for item in parse_event_get(resp_args): + events_by_id[item["id"]] = JMAPCalendarObject(data=item, parent=None) + added = [events_by_id[i] for i in created_ids if i in events_by_id] + modified = [events_by_id[i] for i in updated_ids if i in events_by_id] + return added, modified, destroyed, new_sync_token + + @staticmethod + def _parse_delete_event_response(responses: list, api_url: str, event_id: str) -> None: + for method_name, resp_args, _ in responses: + if method_name == "CalendarEvent/set": + _, _, _, _, _, not_destroyed = parse_event_set(resp_args) + if event_id in not_destroyed: + _JMAPClientBase._raise_set_error(api_url, not_destroyed[event_id]) + return + raise JMAPMethodError(url=api_url, reason="No CalendarEvent/set response") + + @staticmethod + def _parse_get_task_lists_response(responses: list) -> list[dict]: + for method_name, resp_args, _ in responses: + if method_name == "TaskList/get": + return parse_task_list_get(resp_args) + return [] + + @staticmethod + def _parse_create_task_response(responses: list, api_url: str) -> str: + for method_name, resp_args, _ in responses: + if method_name == "Task/set": + created, _, _, not_created, _, _ = parse_task_set(resp_args) + if "new-0" in not_created: + _JMAPClientBase._raise_set_error(api_url, not_created["new-0"]) + if "new-0" not in created: + raise JMAPMethodError( + url=api_url, + reason="Task/set response missing created entry for new-0", + ) + return created["new-0"]["id"] + raise JMAPMethodError(url=api_url, reason="No Task/set response") + + @staticmethod + def _parse_get_task_response(responses: list, api_url: str, task_id: str) -> dict: + for method_name, resp_args, _ in responses: + if method_name == "Task/get": + items = resp_args.get("list", []) + if not items: + raise JMAPMethodError( + url=api_url, + reason=f"Task not found: {task_id}", + error_type="notFound", + ) + return items[0] + raise JMAPMethodError(url=api_url, reason="No Task/get response") + + @staticmethod + def _parse_update_task_response(responses: list, api_url: str, task_id: str) -> None: + for method_name, resp_args, _ in responses: + if method_name == "Task/set": + _, _, _, _, not_updated, _ = parse_task_set(resp_args) + if task_id in not_updated: + _JMAPClientBase._raise_set_error(api_url, not_updated[task_id]) + return + raise JMAPMethodError(url=api_url, reason="No Task/set response") + + @staticmethod + def _parse_delete_task_response(responses: list, api_url: str, task_id: str) -> None: + for method_name, resp_args, _ in responses: + if method_name == "Task/set": + _, _, _, _, _, not_destroyed = parse_task_set(resp_args) + if task_id in not_destroyed: + _JMAPClientBase._raise_set_error(api_url, not_destroyed[task_id]) + return + raise JMAPMethodError(url=api_url, reason="No Task/set response") + class JMAPClient(_JMAPClientBase): """Synchronous JMAP client for calendar operations. @@ -179,11 +432,42 @@ class JMAPClient(_JMAPClientBase): timeout: HTTP request timeout in seconds. """ + def _get_http_session(self): + """Return the persistent HTTP session, creating it on first call.""" + if self._http_session is None: + sess = requests.Session() + sess.auth = self._auth + sess.headers.update({"Content-Type": "application/json", "Accept": "application/json"}) + self._http_session = sess + return self._http_session + + def close(self) -> None: + """Release the persistent HTTP session and its connection pool. + + Only needed when the client was not used as a context manager -- the + documented Quick Start builds one directly, and without this there + was no way to hand the sockets back. Idempotent; the session is + recreated on the next request. + """ + if self._http_session is not None: + self._http_session.close() + self._http_session = None + def __enter__(self) -> JMAPClient: + self._get_http_session() return self def __exit__(self, exc_type, exc_val, exc_tb) -> None: - return None + self.close() + + def __del__(self) -> None: + ## Last-resort net for a client that was neither closed nor used as a + ## context manager. Interpreter shutdown can have torn down enough + ## for this to fail, and an exception here is unraisable noise. + try: + self.close() + except Exception: + pass def _get_session(self) -> Session: """Return the cached Session, fetching it on first call.""" @@ -217,11 +501,9 @@ def _request(self, method_calls: list[tuple], using: list[str] | None = None) -> log.debug("JMAP POST to %s: %d method call(s)", session.api_url, len(method_calls)) - response = requests.post( + response = self._get_http_session().post( session.api_url, json=payload, - auth=self._auth, - headers={"Content-Type": "application/json", "Accept": "application/json"}, timeout=self.timeout, ) @@ -255,18 +537,8 @@ def get_calendars(self) -> list[JMAPCalendar]: List of :class:`~caldav.jmap.objects.calendar.JMAPCalendar` objects. """ session = self._get_session() - call = build_calendar_get(session.account_id) - responses = self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "Calendar/get": - calendars = parse_calendar_get(resp_args) - for cal in calendars: - cal._client = self - cal._is_async = False - return calendars - - return [] + responses = self._request([build_calendar_get(session.account_id)]) + return self._parse_get_calendars(responses, self, False) def create_event(self, calendar_id: str, ical_str: str) -> str: """Create a calendar event from an iCalendar string. @@ -285,20 +557,7 @@ def create_event(self, calendar_id: str, ical_str: str) -> str: jscal = ical_to_jscal(ical_str, calendar_id=calendar_id) call = build_event_set_create(session.account_id, {"new-0": jscal}) responses = self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - created, _, _, not_created, _, _ = parse_event_set(resp_args) - if "new-0" in not_created: - self._raise_set_error(session, not_created["new-0"]) - if "new-0" not in created: - raise JMAPMethodError( - url=session.api_url, - reason="CalendarEvent/set response missing created entry for new-0", - ) - return created["new-0"]["id"] - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + return self._parse_create_event_response(responses, session.api_url) def get_event(self, event_id: str) -> JMAPCalendarObject: """Fetch a calendar event by JMAP event ID. @@ -316,21 +575,8 @@ def get_event(self, event_id: str) -> JMAPCalendarObject: JMAPMethodError: If the event is not found. """ session = self._get_session() - call = build_event_get(session.account_id, ids=[event_id]) - responses = self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - items = parse_event_get(resp_args) - if not items: - raise JMAPMethodError( - url=session.api_url, - reason=f"Event not found: {event_id}", - error_type="notFound", - ) - return JMAPCalendarObject(data=items[0], parent=None) - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/get response") + responses = self._request([build_event_get(session.account_id, ids=[event_id])]) + return self._parse_get_event_response(responses, session.api_url, event_id) def update_event(self, event_id: str, ical_str: str) -> None: """Update a calendar event from an iCalendar string. @@ -343,19 +589,17 @@ def update_event(self, event_id: str, ical_str: str) -> None: JMAPMethodError: If the server rejects the update. """ session = self._get_session() - patch = ical_to_jscal(ical_str) - patch.pop("uid", None) # uid is server-immutable after creation; patch must omit it - call = build_event_set_update(session.account_id, {event_id: patch}) - responses = self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - _, _, _, _, not_updated, _ = parse_event_set(resp_args) - if event_id in not_updated: - self._raise_set_error(session, not_updated[event_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + patch, nulled = self._build_event_update_patch(ical_str) + while True: + responses = self._request( + [build_event_set_update(session.account_id, {event_id: patch})] + ) + drop = self._unsupported_null_keys(responses, event_id, patch, nulled) + if not drop: + break + for key in drop: + patch.pop(key, None) + self._parse_update_event_response(responses, session.api_url, event_id) def _search( self, @@ -368,15 +612,7 @@ def _search( session = self._get_session() calls = self._build_event_search_calls(session.account_id, calendar_id, start, end, text) responses = self._request(calls) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - return [ - JMAPCalendarObject(data=item, parent=parent) - for item in parse_event_get(resp_args) - ] - - return [] + return self._parse_search_response(responses, parent) def search_events( self, @@ -417,16 +653,12 @@ def get_sync_token(self) -> str: retrieve only what changed since this point. """ session = self._get_session() - call = build_event_get(session.account_id, ids=[]) - responses = self._request([call]) - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/get": - return resp_args.get("state", "") - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/get response") + responses = self._request([build_event_get(session.account_id, ids=[])]) + return self._parse_get_sync_token_response(responses, session.api_url) def get_objects_by_sync_token( self, sync_token: str - ) -> tuple[list[JMAPCalendarObject], list[JMAPCalendarObject], list[str]]: + ) -> tuple[list[JMAPCalendarObject], list[JMAPCalendarObject], list[str], str]: """Fetch events changed since a previous sync token. Calls ``CalendarEvent/changes`` to discover which events were created, @@ -440,53 +672,28 @@ def get_objects_by_sync_token( or by a prior call to this method. Returns: - A 3-tuple ``(added, modified, deleted)``: + A 4-tuple ``(added, modified, deleted, new_sync_token)``: - ``added``: objects for newly created events (``parent`` is ``None``). - ``modified``: objects for updated events (``parent`` is ``None``). - ``deleted``: Event IDs that were destroyed. + - ``new_sync_token``: Pass to the next call to this method as ``sync_token``. Raises: JMAPMethodError: If the server reports ``hasMoreChanges: true``. """ session = self._get_session() - changes_call = build_event_changes(session.account_id, sync_token) - responses = self._request([changes_call]) - - created_ids: list[str] = [] - updated_ids: list[str] = [] - destroyed: list[str] = [] - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/changes": - _, _, has_more, created_ids, updated_ids, destroyed = parse_event_changes(resp_args) - if has_more: - raise JMAPMethodError( - url=session.api_url, - reason=( - "CalendarEvent/changes response was truncated by the server " - "(hasMoreChanges=true). Call get_sync_token() to obtain a " - "fresh baseline and re-sync." - ), - error_type="serverPartialFail", - ) - + responses = self._request([build_event_changes(session.account_id, sync_token)]) + created_ids, updated_ids, destroyed, new_sync_token = self._parse_event_changes_response( + responses, session.api_url + ) fetch_ids = created_ids + updated_ids if not fetch_ids: - return [], [], destroyed - - get_call = build_event_get(session.account_id, ids=fetch_ids) - get_responses = self._request([get_call]) - - events_by_id: dict[str, JMAPCalendarObject] = {} - for method_name, resp_args, _ in get_responses: - if method_name == "CalendarEvent/get": - for item in parse_event_get(resp_args): - events_by_id[item["id"]] = JMAPCalendarObject(data=item, parent=None) - - added = [events_by_id[i] for i in created_ids if i in events_by_id] - modified = [events_by_id[i] for i in updated_ids if i in events_by_id] - return added, modified, destroyed + return [], [], destroyed, new_sync_token + get_responses = self._request([build_event_get(session.account_id, ids=fetch_ids)]) + return self._assemble_sync_token_result( + get_responses, created_ids, updated_ids, destroyed, new_sync_token + ) def delete_event(self, event_id: str) -> None: """Delete a calendar event. @@ -498,17 +705,8 @@ def delete_event(self, event_id: str) -> None: JMAPMethodError: If the server rejects the delete. """ session = self._get_session() - call = build_event_set_destroy(session.account_id, [event_id]) - responses = self._request([call]) - - for method_name, resp_args, _ in responses: - if method_name == "CalendarEvent/set": - _, _, _, _, _, not_destroyed = parse_event_set(resp_args) - if event_id in not_destroyed: - self._raise_set_error(session, not_destroyed[event_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No CalendarEvent/set response") + responses = self._request([build_event_set_destroy(session.account_id, [event_id])]) + self._parse_delete_event_response(responses, session.api_url, event_id) def _get_object_by_uid( self, uid: str, calendar_id: str | None = None, parent: JMAPCalendar | None = None @@ -529,14 +727,8 @@ def get_task_lists(self) -> list[dict]: List of raw JMAP TaskList dicts as returned by the server. """ session = self._get_session() - call = build_task_list_get(session.account_id) - responses = self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "TaskList/get": - return parse_task_list_get(resp_args) - - return [] + responses = self._request([build_task_list_get(session.account_id)], using=_TASK_USING) + return self._parse_get_task_lists_response(responses) def create_task(self, task_list_id: str, title: str, **kwargs) -> str: """Create a task in a task list. @@ -567,15 +759,7 @@ def create_task(self, task_list_id: str, title: str, **kwargs) -> str: task_dict.update(kwargs) call = build_task_set_create(session.account_id, {"new-0": task_dict}) responses = self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - created, _, _, not_created, _, _ = parse_task_set(resp_args) - if "new-0" in not_created: - self._raise_set_error(session, not_created["new-0"]) - return created["new-0"]["id"] - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + return self._parse_create_task_response(responses, session.api_url) def get_task(self, task_id: str) -> dict: """Fetch a task by ID. @@ -590,21 +774,10 @@ def get_task(self, task_id: str) -> dict: JMAPMethodError: If the task is not found. """ session = self._get_session() - call = build_task_get(session.account_id, ids=[task_id]) - responses = self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/get": - items = resp_args.get("list", []) - if not items: - raise JMAPMethodError( - url=session.api_url, - reason=f"Task not found: {task_id}", - error_type="notFound", - ) - return items[0] - - raise JMAPMethodError(url=session.api_url, reason="No Task/get response") + responses = self._request( + [build_task_get(session.account_id, ids=[task_id])], using=_TASK_USING + ) + return self._parse_get_task_response(responses, session.api_url, task_id) def update_task(self, task_id: str, patch: dict) -> None: """Update a task with a partial patch. @@ -619,15 +792,7 @@ def update_task(self, task_id: str, patch: dict) -> None: session = self._get_session() call = build_task_set_update(session.account_id, {task_id: patch}) responses = self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - _, _, _, _, not_updated, _ = parse_task_set(resp_args) - if task_id in not_updated: - self._raise_set_error(session, not_updated[task_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + self._parse_update_task_response(responses, session.api_url, task_id) def delete_task(self, task_id: str) -> None: """Delete a task. @@ -639,14 +804,7 @@ def delete_task(self, task_id: str) -> None: JMAPMethodError: If the server rejects the delete. """ session = self._get_session() - call = build_task_set_destroy(session.account_id, [task_id]) - responses = self._request([call], using=_TASK_USING) - - for method_name, resp_args, _ in responses: - if method_name == "Task/set": - _, _, _, _, _, not_destroyed = parse_task_set(resp_args) - if task_id in not_destroyed: - self._raise_set_error(session, not_destroyed[task_id]) - return - - raise JMAPMethodError(url=session.api_url, reason="No Task/set response") + responses = self._request( + [build_task_set_destroy(session.account_id, [task_id])], using=_TASK_USING + ) + self._parse_delete_task_response(responses, session.api_url, task_id) diff --git a/caldav/jmap/convert/_patch.py b/caldav/jmap/convert/_patch.py new file mode 100644 index 00000000..db31a2ab --- /dev/null +++ b/caldav/jmap/convert/_patch.py @@ -0,0 +1,33 @@ +""" +RFC 8620 PatchObject helpers for CalendarEvent/set update calls. + +When updating an event, absent keys preserve the server's current value. +To delete an optional property the patch must set it to null explicitly. +""" + +from __future__ import annotations + +# Optional JSCalendar top-level properties that must be explicitly nulled in +# a CalendarEvent/set update when they are absent from the converted result. +# This ensures properties removed client-side (e.g. LOCATION deleted from +# the iCalendar) are actually removed on the server, not silently preserved. +_NULL_FOR_UPDATE: frozenset[str] = frozenset( + { + "description", + "color", + "locations", + "keywords", + "priority", + "privacy", + "freeBusyStatus", + "status", + "sequence", + "showWithoutTime", + "timeZone", + "recurrenceRules", + "excludedRecurrenceRules", + "recurrenceOverrides", + "participants", + "alerts", + } +) diff --git a/caldav/jmap/convert/_utils.py b/caldav/jmap/convert/_utils.py index 12b12263..8f19b493 100644 --- a/caldav/jmap/convert/_utils.py +++ b/caldav/jmap/convert/_utils.py @@ -5,6 +5,7 @@ from __future__ import annotations from datetime import date, datetime, timedelta +from datetime import tzinfo as tzinfo_t def _timedelta_to_duration(td: timedelta) -> str: @@ -110,23 +111,31 @@ def _duration_to_timedelta(duration_str: str) -> timedelta: return sign * td -def _format_local_dt(dt: datetime | date) -> str: - """Format a datetime or date as a JSCalendar LocalDateTime or UTCDateTime string. +def _format_local_dt(dt: datetime | date, tzinfo: tzinfo_t | None = None) -> str: + """Format a datetime or date as a JSCalendar LocalDateTime string. - JSCalendar uses: - - LocalDateTime: "2024-03-15T09:00:00" (no TZ suffix) - - UTCDateTime: "2024-03-15T09:00:00Z" (uppercase Z) + RFC 8984 requires LocalDateTime (no Z suffix) for override keys and RRULE + ``until`` values, and those are expressed in the *event's* timezone. An + aware datetime is therefore converted into ``tzinfo`` before the offset is + dropped; merely stripping it would shift the value by the UTC offset, and + a floating ``UNTIL`` against a TZID ``DTSTART`` is forbidden outright by + RFC 5545 3.3.10. + + ``tzinfo`` is the event's timezone, normally ``DTSTART.dt.tzinfo``. When + it is None the event is floating or all-day: there is nothing to convert + into, so the value is passed through as-is. For date objects (all-day), uses T00:00:00 suffix. Args: dt: A datetime (with or without tzinfo) or a date. + tzinfo: The event's timezone, or None for a floating/all-day event. Returns: - Formatted string suitable for use as a JSCalendar override key or datetime value. + Formatted string suitable for use as a JSCalendar override key or RRULE until. """ if isinstance(dt, datetime): - if dt.tzinfo is not None and dt.utcoffset() == timedelta(0): - return dt.strftime("%Y-%m-%dT%H:%M:%SZ") + if tzinfo is not None and dt.tzinfo is not None: + dt = dt.astimezone(tzinfo) return dt.strftime("%Y-%m-%dT%H:%M:%S") return f"{dt.isoformat()}T00:00:00" diff --git a/caldav/jmap/convert/ical_to_jscal.py b/caldav/jmap/convert/ical_to_jscal.py index 00e9ce59..8471a396 100644 --- a/caldav/jmap/convert/ical_to_jscal.py +++ b/caldav/jmap/convert/ical_to_jscal.py @@ -68,11 +68,14 @@ def _dtstart_to_jscal(dtstart_prop) -> tuple[str, str | None, bool]: return dt.strftime("%Y-%m-%dT%H:%M:%S"), None, False -def _rrule_to_jscal(rrule_prop) -> dict: +def _rrule_to_jscal(rrule_prop, tzinfo=None) -> dict: """Convert an iCalendar RRULE property to a JSCalendar RecurrenceRule dict. Always emits @type, interval, rscale, skip, firstDayOfWeek to match the fields Cyrus returns — makes round-trip comparison predictable. + + ``tzinfo`` is the event's timezone; ``until`` is a LocalDateTime in that + zone, so a UTC ``UNTIL`` off the wire has to be converted, not truncated. """ rule: dict = { "@type": "RecurrenceRule", @@ -97,7 +100,7 @@ def _rrule_to_jscal(rrule_prop) -> dict: until_list = rrule_prop.get("UNTIL", []) if until_list: - rule["until"] = _format_local_dt(until_list[0]) + rule["until"] = _format_local_dt(until_list[0], tzinfo) byday_list = rrule_prop.get("BYDAY", []) if byday_list: @@ -145,9 +148,11 @@ def _rrule_to_jscal(rrule_prop) -> dict: return rule -def _exdate_to_overrides(exdate_prop) -> dict: +def _exdate_to_overrides(exdate_prop, tzinfo=None) -> dict: """Convert an EXDATE property (single or list) to recurrenceOverrides entries. + ``tzinfo`` is the event's timezone — see :func:`_format_local_dt`. + Returns: Dict mapping LocalDateTime/UTCDateTime string → {"excluded": True} """ @@ -160,7 +165,7 @@ def _exdate_to_overrides(exdate_prop) -> dict: dts = getattr(ex, "dts", [ex]) for dt_prop in dts: dt = getattr(dt_prop, "dt", dt_prop) - overrides[_format_local_dt(dt)] = {"excluded": True} + overrides[_format_local_dt(dt, tzinfo)] = {"excluded": True} return overrides @@ -302,15 +307,13 @@ def ical_to_jscal(ical_str: str, calendar_id: str | None = None) -> dict: # Split subcomponents into master VEVENTs and override VEVENTs master: icalendar.Event | None = None - overrides_by_recurrence_id: dict[str, icalendar.Event] = {} + override_components: list[icalendar.Event] = [] for component in cal.subcomponents: if not isinstance(component, icalendar.Event): continue if component.get("RECURRENCE-ID") is not None: - # Override instance — key by its recurrence-id datetime - rid = _format_local_dt(component["RECURRENCE-ID"].dt) - overrides_by_recurrence_id[rid] = component + override_components.append(component) elif master is None: master = component @@ -323,6 +326,17 @@ def ical_to_jscal(ical_str: str, calendar_id: str | None = None) -> dict: dtstart_prop = master["DTSTART"] start, time_zone, show_without_time = _dtstart_to_jscal(dtstart_prop) + ## The event's own timezone. Every LocalDateTime slot below (RRULE + ## until, EXDATE keys, RECURRENCE-ID keys) is expressed in it, so it has + ## to be known before any of them can be formatted — which is why the + ## override keys cannot be built in the loop above. + event_tzinfo = getattr(getattr(dtstart_prop, "dt", None), "tzinfo", None) + + overrides_by_recurrence_id: dict[str, icalendar.Event] = { + _format_local_dt(component["RECURRENCE-ID"].dt, event_tzinfo): component + for component in override_components + } + if master.get("DURATION"): duration = _timedelta_to_duration(master["DURATION"].dt) elif master.get("DTEND"): @@ -386,6 +400,17 @@ def ical_to_jscal(ical_str: str, calendar_id: str | None = None) -> dict: if location: jscal["locations"] = _location_str_to_jscal(str(location)) + status = master.get("STATUS") + if status: + _STATUS_ICAL_TO_JSCAL = { + "CONFIRMED": "confirmed", + "TENTATIVE": "tentative", + "CANCELLED": "cancelled", + } + jscal_status = _STATUS_ICAL_TO_JSCAL.get(str(status).upper()) + if jscal_status: + jscal["status"] = jscal_status + participants: dict = {} organizer = master.get("ORGANIZER") if organizer is not None: @@ -411,19 +436,19 @@ def ical_to_jscal(ical_str: str, calendar_id: str | None = None) -> dict: if rrules is not None: if not isinstance(rrules, list): rrules = [rrules] - jscal["recurrenceRules"] = [_rrule_to_jscal(r) for r in rrules] + jscal["recurrenceRules"] = [_rrule_to_jscal(r, event_tzinfo) for r in rrules] exrules = master.get("EXRULE") if exrules is not None: if not isinstance(exrules, list): exrules = [exrules] - jscal["excludedRecurrenceRules"] = [_rrule_to_jscal(r) for r in exrules] + jscal["excludedRecurrenceRules"] = [_rrule_to_jscal(r, event_tzinfo) for r in exrules] recurrence_overrides: dict = {} exdate = master.get("EXDATE") if exdate is not None: - recurrence_overrides.update(_exdate_to_overrides(exdate)) + recurrence_overrides.update(_exdate_to_overrides(exdate, event_tzinfo)) for rid_key, child in overrides_by_recurrence_id.items(): # Build a patch: only fields that differ from the master diff --git a/caldav/jmap/convert/jscal_to_ical.py b/caldav/jmap/convert/jscal_to_ical.py index 2a449d1b..4ed4c03d 100644 --- a/caldav/jmap/convert/jscal_to_ical.py +++ b/caldav/jmap/convert/jscal_to_ical.py @@ -353,6 +353,17 @@ def jscal_to_ical(jscal: dict) -> str: if loc_name: event.add("location", loc_name) + status = jscal.get("status") + if status: + _STATUS_JSCAL_TO_ICAL = { + "confirmed": "CONFIRMED", + "tentative": "TENTATIVE", + "cancelled": "CANCELLED", + } + ical_status = _STATUS_JSCAL_TO_ICAL.get(status) + if ical_status: + event.add("status", ical_status) + for rule in jscal.get("recurrenceRules") or []: ical_rule = _jscal_rrule_to_rrule(rule) if ical_rule: @@ -371,6 +382,15 @@ def jscal_to_ical(jscal: dict) -> str: rid_dt: datetime | date = datetime.strptime(override_key, "%Y-%m-%dT%H:%M:%SZ").replace( tzinfo=timezone.utc ) + elif show_without_time: + rid_dt = date.fromisoformat(override_key[:10]) + elif time_zone: + try: + rid_dt = datetime.strptime(override_key[:19], "%Y-%m-%dT%H:%M:%S").replace( + tzinfo=ZoneInfo(time_zone) + ) + except ZoneInfoNotFoundError: + rid_dt = datetime.strptime(override_key[:19], "%Y-%m-%dT%H:%M:%S") else: rid_dt = datetime.strptime(override_key[:19], "%Y-%m-%dT%H:%M:%S") @@ -381,7 +401,8 @@ def jscal_to_ical(jscal: dict) -> str: child.add("uid", uid) child.add("dtstamp", datetime.now(tz=timezone.utc)) child.add("recurrence-id", rid_dt) - child_start = patch.get("start", start_str) + # Default child start to the occurrence time (override key), not the master start. + child_start = patch.get("start", override_key) child_tz = patch.get("timeZone", time_zone) child_swt = patch.get("showWithoutTime", show_without_time) if child_start: diff --git a/caldav/jmap/objects/calendar.py b/caldav/jmap/objects/calendar.py index 28812ac9..6be47bd7 100644 --- a/caldav/jmap/objects/calendar.py +++ b/caldav/jmap/objects/calendar.py @@ -8,7 +8,7 @@ from __future__ import annotations from dataclasses import dataclass, field -from datetime import datetime +from datetime import datetime, timezone from typing import TYPE_CHECKING from caldav.jmap.objects.calendar_object import JMAPCalendarObject @@ -18,6 +18,17 @@ from caldav.jmap.client import JMAPClient +def _to_utcdate(dt: datetime) -> str: + """Convert a datetime to JMAP UTCDate format (YYYY-MM-DDTHH:MM:SSZ). + + Naive datetimes are assumed to be UTC. Aware datetimes are converted to + UTC before formatting. Microseconds are dropped as JMAP does not allow them. + """ + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + @dataclass class JMAPCalendar: """A JMAP Calendar object. @@ -111,9 +122,9 @@ def search(self, **searchargs): start = searchargs.get("start") end = searchargs.get("end") if isinstance(start, datetime): - start = start.isoformat() + start = _to_utcdate(start) if isinstance(end, datetime): - end = end.isoformat() + end = _to_utcdate(end) return self._client._search( calendar_id=self.id, start=start, @@ -126,9 +137,9 @@ async def _async_search(self, **searchargs) -> list[JMAPCalendarObject]: start = searchargs.get("start") end = searchargs.get("end") if isinstance(start, datetime): - start = start.isoformat() + start = _to_utcdate(start) if isinstance(end, datetime): - end = end.isoformat() + end = _to_utcdate(end) return await self._client._search( calendar_id=self.id, start=start, diff --git a/caldav/lib/auth.py b/caldav/lib/auth.py index fa4d351e..c15b9ef1 100644 --- a/caldav/lib/auth.py +++ b/caldav/lib/auth.py @@ -28,7 +28,7 @@ def extract_auth_types(header: str) -> set[str]: Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate#syntax """ - return {h.split()[0] for h in header.lower().split(",")} + return {h.split()[0] for h in header.lower().split(",") if h.strip()} def select_auth_type( diff --git a/caldav/lib/error.py b/caldav/lib/error.py index 16d79883..2822c0b7 100644 --- a/caldav/lib/error.py +++ b/caldav/lib/error.py @@ -13,6 +13,12 @@ ## Environmental variables prepended with "PYTHON_CALDAV" are used for debug purposes, ## environmental variables prepended with "CALDAV_" are for connection parameters debug_dump_communication = os.environ.get("PYTHON_CALDAV_COMMDUMP", False) + if debug_dump_communication: + logging.getLogger("caldav").warning( + "PYTHON_CALDAV_COMMDUMP is set: request/response bodies and headers " + "(including credentials and calendar PII) will be written to uniquely-named " + "files under /tmp. These files accumulate indefinitely — remove them when done." + ) ## one of DEBUG_PDB, DEBUG, DEVELOPMENT, PRODUCTION debugmode = os.environ["PYTHON_CALDAV_DEBUGMODE"] except KeyError: diff --git a/caldav/lib/url.py b/caldav/lib/url.py index c2b426e0..3390371b 100644 --- a/caldav/lib/url.py +++ b/caldav/lib/url.py @@ -140,7 +140,13 @@ def canonical(self) -> "URL": """ url = self.unauth() - arr = list(cast(urllib.parse.ParseResult, self.url_parsed)) + # Use url's parsed form (credentials already stripped), not self's. + # Also always build a fresh URL so self is never mutated — unauth() + # returns self when there are no credentials, and the old code then + # overwrote url.url_raw/url_parsed which are the same object as self. + if url.url_parsed is None: + url.url_parsed = cast(urllib.parse.ParseResult, urlparse(str(url))) + arr = list(url.url_parsed) ## quoting path and removing double slashes arr[2] = quote(unquote(url.path.replace("//", "/"))) ## sensible defaults @@ -155,11 +161,7 @@ def canonical(self) -> "URL": portpart = "" arr[1] += portpart - # make sure to delete the string version - url.url_raw = urlunparse(arr) - url.url_parsed = None - - return url + return URL(urlunparse(arr)) def join(self, path: Any) -> "URL": """ diff --git a/caldav/lib/vcal.py b/caldav/lib/vcal.py index fb29f7cd..246dd281 100644 --- a/caldav/lib/vcal.py +++ b/caldav/lib/vcal.py @@ -77,20 +77,28 @@ def fix(event): ## TODO: add ^ before COMPLETED and CREATED? ## 1) Add an arbitrary time if completed is given as date - fixed = re.sub(r"COMPLETED(?:;VALUE=DATE)?:(\d+)\s", r"COMPLETED:\g<1>T120000Z", event) + fixed = re.sub(r"COMPLETED(?:;VALUE=DATE)?:(\d+)(?=\s)", r"COMPLETED:\g<1>T120000Z", event) ## 2) CREATED timestamps prior to epoch does not make sense, ## change from year 0001 to epoch. fixed = re.sub("CREATED:00001231T000000Z", "CREATED:19700101T000000Z", fixed) - fixed = re.sub(r"\\+('\")", r"\1", fixed) + fixed = re.sub(r"\\+(['\"])", r"\1", fixed) - ## 4) trailing whitespace probably never makes sense - fixed = re.sub(" *$", "", fixed) + ## 4) trailing whitespace probably never makes sense -- but only on a + ## line that is not continued by a folded line. RFC 5545 3.1 folds + ## blind at 75 octets, so the fold may land right after a space that is + ## part of the value; stripping it would join two words together. The + ## negative lookahead is what keeps that whitespace alone. + fixed = re.sub(r"[ \t]+$(?!\n[ \t])", "", fixed, flags=re.MULTILINE) ## 6) add DTSTAMP if not given ## (corner case that DTSTAMP is given in one but not all the recurrences is ignored) if "\nDTSTAMP:" not in fixed: - assert "\nEND" in fixed + if "\nEND" not in fixed: + logging.getLogger(__name__).warning( + "vcal.fix(): truncated iCalendar data (no END: line) — skipping DTSTAMP fixup" + ) + return fixed dtstamp = datetime.datetime.now(tz=datetime.timezone.utc).strftime("%Y%m%dT%H%M%SZ") fixed = re.sub("(\nEND:(VTODO|VEVENT|VJOURNAL))", f"\nDTSTAMP:{dtstamp}\\1", fixed) @@ -240,8 +248,8 @@ def create_ical(ical_fragment=None, objtype=None, language="en_DK", **props): ret = to_normal_str(my_instance.to_ical()) if ical_fragment and ical_fragment.strip(): ret = re.sub( - "^END:V", - ical_fragment.strip() + "\nEND:V", + "^(END:V(?:EVENT|TODO|JOURNAL))", + ical_fragment.strip() + "\n\\1", ret, flags=re.MULTILINE, count=1, diff --git a/caldav/response.py b/caldav/response.py index 0b27d5b9..a8b1205e 100644 --- a/caldav/response.py +++ b/caldav/response.py @@ -116,22 +116,38 @@ def _strip_to_multistatus(tree: _Element) -> "_Element | list[_Element]": return [tree] -def _extract_properties(propstats: "list[_Element]") -> "dict[str, Any]": - """Extract properties from propstat elements into a flat dict.""" - properties: dict[str, Any] = {} +def _collect_prop_elements(propstats: "list[_Element]") -> "dict[str, _Element]": + """Collect ``{proptag: element}`` from a list of propstat elements. + + Each propstat status is validated first: anything but 200/201/207/404 + raises :class:`error.ResponseError`, since a ``500``/``403``/``507`` + propstat means the server failed to answer, not that the property is + unset. Swallowing it would hand the caller a silently-empty value. + + Propstats whose status reports 404 are then skipped — that is the single, + shared expression of the "a 404 propstat means the property is absent on + the resource" quirk. This helper is the one place both the dataclass + parsers (via :func:`_extract_properties`) and the legacy + :meth:`DAVResponse._find_objects_and_props` collect prop children, so both + the validation and the quirk stop having to be maintained in two parallel + loops (code-review §5.7). + """ + collected: dict[str, _Element] = {} for propstat in propstats: status_elem = propstat.find(dav.Status.tag) - if status_elem is not None and status_elem.text and " 404 " in status_elem.text: - continue - prop = propstat.find(dav.Prop.tag) - if prop is None: - continue - for child in prop: - if len(child) == 0: - properties[child.tag] = child.text - else: - properties[child.tag] = _element_to_value(child) - return properties + if status_elem is not None and status_elem.text: + _validate_status(status_elem.text) + if " 404 " in status_elem.text: + continue + for prop in propstat.iterfind(dav.Prop.tag): + for child in prop: + collected[child.tag] = child + return collected + + +def _extract_properties(propstats: "list[_Element]") -> "dict[str, Any]": + """Extract properties from propstat elements into a flat dict of parsed values.""" + return {tag: _element_to_value(el) for tag, el in _collect_prop_elements(propstats).items()} def _element_to_value(elem: _Element) -> Any: @@ -274,7 +290,12 @@ def _init_from_response(self, response: "Response", davclient: Any = None) -> No # We'll try to parse the content as XML no matter the content type. self.tree = etree.XML( self._raw, - parser=etree.XMLParser(remove_blank_text=True, huge_tree=self.huge_tree), + parser=etree.XMLParser( + remove_blank_text=True, + huge_tree=self.huge_tree, + resolve_entities=False, + no_network=True, + ), ) except Exception: # Content wasn't XML. What does the content-type say? @@ -474,28 +495,23 @@ def _parse_response(self, response: _Element) -> tuple[str, list[_Element], Any href = _normalize_href(elem.text or "") elif elem.tag == dav.PropStat.tag: propstats.append(elem) - elif elem.tag == "{DAV:}responsedescription": - ## This happens with Stalwart on a 404. - ## This code is mostly moot, but in debug - ## mode I want to be sure we do not toss away any data - error.assert_(elem.text == "No resources found") - check_404 = True - elif elem.tag == "{DAV:}error": - ## This happens with purelymail on a 404. - ## This code is mostly moot, but in debug - ## mode I want to be sure we do not toss away any data - children = elem.getchildren() - error.assert_(len(children) == 1) - error.assert_(children[0].tag == "{https://purelymail.com}does-not-exist") + elif elem.tag in ("{DAV:}responsedescription", "{DAV:}error"): + ## Both are optional children of per RFC 4918 + ## and carry server-defined content. We've seen them on + ## 404s (Stalwart sends No resources + ## found, purelymail sends + ## <…:does-not-exist/>). check_404 = True else: - ## i.e. purelymail may contain one more tag, ... - ## This is probably not a breach of the standard. It may - ## probably be ignored. But it's something we may want to - ## know. + ## A tag we don't recognise at all (e.g. a server inventing + ## an element). Not necessarily a standards + ## breach and probably ignorable, but worth surfacing. error.weirdness("unexpected element found in response", elem) error.assert_(href) - if check_404: + if check_404 and status: + ## We've only ever observed / on + ## 404s; flag it in debug mode if a server pairs them with some + ## other status so we notice and revisit this handling. error.assert_("404" in status) return (cast(str, href), propstats, status) @@ -579,6 +595,35 @@ def sync_token(self): ## protocol.xml_parsers layer is a better approach. Look for more ## cases of old code that was is still remaining after the ## protocol layer refactoring + def all_responses_not_found(self) -> bool: + """True if the multistatus consists solely of response-level 404s. + + RFC 4918 §14.24 lets a ```` carry a bare ```` + instead of one or more ```` elements, so a server may + report "this resource does not exist" inside a 207 Multi-Status + rather than as a transport-level 404. Xandikos answers PROPFIND on + a missing collection that way (while answering REPORT on the very + same URL with a plain 404). + + A 404 for one href among several is normal on ``Depth: 1`` and must + not be treated as "the resource is gone", hence the requirement that + *every* response reports 404 and none carries properties. + """ + if self.tree is None: + return False + responses = [r for r in self._strip_to_multistatus() if r.tag == dav.Response.tag] + if not responses: + return False + for response in responses: + ## a direct-child ; the ones nested inside + ## are a different thing and handled by _collect_prop_elements + if response.find(dav.PropStat.tag) is not None: + return False + status = response.find(dav.Status.tag) + if status is None or "404" not in (status.text or ""): + return False + return True + def _find_objects_and_props(self) -> dict[str, dict[str, _Element]]: """Internal implementation of find_objects_and_props without deprecation warning.""" self.objects: dict[str, dict[str, _Element]] = {} @@ -606,27 +651,11 @@ def _find_objects_and_props(self) -> dict[str, dict[str, _Element]]: self.objects[href] = {} self.statuses[href] = status - ## The properties may be delivered either in one - ## propstat with multiple props or in multiple - ## propstat - for propstat in propstats: - cnt = 0 - status = propstat.find(dav.Status.tag) - error.assert_(status is not None) - if status is not None and status.text is not None: - error.assert_(len(status) == 0) - cnt += 1 - self.validate_status(status.text) - ## if a prop was not found, ignore it - if " 404 " in status.text: - continue - for prop in propstat.iterfind(dav.Prop.tag): - cnt += 1 - for theprop in prop: - self.objects[href][theprop.tag] = theprop - - ## there shouldn't be any more elements except for status and prop - error.assert_(cnt == len(propstat)) + ## The properties may be delivered either in one propstat + ## with multiple props or in multiple propstats; the 404-skip + ## quirk is shared with the dataclass parsers via + ## _collect_prop_elements (code-review §5.7). + self.objects[href].update(_collect_prop_elements(propstats)) return self.objects diff --git a/caldav/search.py b/caldav/search.py index 5b853382..061a336c 100644 --- a/caldav/search.py +++ b/caldav/search.py @@ -17,6 +17,8 @@ from .lib import error if TYPE_CHECKING: + from collections.abc import Generator + from .calendarobjectresource import ( CalendarObjectResource as AsyncCalendarObjectResource, ) @@ -190,7 +192,8 @@ def _build_search_xml_query( for property in searcher._property_operator: if searcher._property_operator[property] == "undef": match = cdav.NotDefined() - filters.append(cdav.PropFilter(property.upper()) + match) + prop_name = "CATEGORIES" if property.lower() == "category" else property.upper() + filters.append(cdav.PropFilter(prop_name) + match) else: value = searcher._property_filters[property] property_ = property.upper() @@ -232,6 +235,23 @@ def _build_search_xml_query( return (root, comp_class) +def _dedup_by_url(matches: list) -> list: + """Drop repeated resources, keeping the first occurrence and the order. + + A search that is split into several server queries can return the same + resource more than once: the include-completed split issues overlapping + queries, and in a comp-type split a resource that legally holds both a + VEVENT and a VTODO matches two of the three queries. + """ + objects = [] + seen = set() + for item in matches: + if item.url not in seen: + seen.add(item.url) + objects.append(item) + return objects + + def _is_not_defined_supported(features: Any, prop: str) -> bool: """Check if is-not-defined search is supported for a specific property. @@ -267,9 +287,39 @@ class SearchAction(Enum): SEARCH_WITH_COMPTYPES = auto() # (args) -> search with all comp types REQUEST_REPORT = auto() # (xml, comp_class, props) -> make CalDAV request LOAD_OBJECT = auto() # (obj) -> load object data + LOAD_OBJECTS_BATCH = ( + auto() + ) # (calendar, objects) -> batch-load via calendar._batch_load_objects RETURN = auto() # (result) -> return this value +def _advance_search_gen( + gen: "Generator[tuple[SearchAction, Any], Any, None]", + result: Any = None, + exc: BaseException | None = None, +) -> "tuple[SearchAction, Any] | None": + """Phase 2 of the search driver protocol, shared by sync and async drivers. + + Feed the Phase-1 ``result`` (or the ``exc`` raised while executing the + yielded action) back into the search generator. Feeding an exception via + ``gen.throw()`` lets the search logic's own try/except blocks act on it + (the issue #681 time-range fallback, per-object load error handling, ...); + if the generator does not handle it, ``gen.throw()`` re-raises it out of + here, which is the correct propagation. + + Passing ``result=None, exc=None`` on a fresh generator primes it. + + :return: the next ``(action, data)`` to execute, or ``None`` when the + generator is exhausted (StopIteration → the driver returns ``[]``). + """ + try: + if exc is not None: + return gen.throw(exc) + return gen.send(result) + except StopIteration: + return None + + @dataclass class CalDAVSearcher(Searcher): """The baseclass (which is generic, and not CalDAV-specific) @@ -316,6 +366,12 @@ class CalDAVSearcher(Searcher): comp_class: Optional["CalendarObjectResource"] = None _explicit_operators: set = field(default_factory=set) _calendar: Optional["Calendar"] = field(default=None, repr=False) + ## When False, all server-compatibility workarounds in _search_impl are + ## disabled and the query the searcher describes is sent verbatim (a single + ## REPORT, no comp-type splitting, no filter rewriting, no fallback retries). + ## Used by the server-compatibility checker to observe raw server behaviour. + ## Propagates to clones automatically via dataclasses.replace(). + _compatibility_workarounds: bool = True def add_property_filter( self, @@ -455,12 +511,18 @@ def _search_impl( "create the searcher via calendar.searcher()" ) + ## When disabled, every server-compatibility workaround below is skipped + ## and the query is sent verbatim (used by the compatibility checker to + ## observe raw server behaviour). + cw = self._compatibility_workarounds + ## Workaround for servers where REPORT without a time range only returns ## objects within a sliding window (search.unlimited-time-range: broken). ## Inject a wide time range covering 1970–2126 so that year-2000 test ## objects and other old data are returned. if ( - not self.start + cw + and not self.start and not self.end and not (self.expand or server_expand) and not calendar.client.features.is_supported("search.unlimited-time-range") @@ -480,7 +542,8 @@ def _search_impl( ## Handle servers with broken component-type filtering (e.g., Bedework) comp_type_support = calendar.client.features.is_supported("search.comp-type", str) no_comp_filter = ( - (self.comp_class or self.todo or self.event or self.journal) + cw + and (self.comp_class or self.todo or self.event or self.journal) and comp_type_support == "broken" and post_filter is not False ) @@ -490,13 +553,18 @@ def _search_impl( post_filter = True ## Setting default value for post_filter - if post_filter is None and ( - (self.todo and not self.include_completed) - or self.expand - or "categories" in self._property_filters - or "category" in self._property_filters - or not calendar.client.features.is_supported("search.text.case-sensitive") - or not calendar.client.features.is_supported("search.time-range.accurate") + if ( + cw + and post_filter is None + and ( + (self.todo and not self.include_completed) + or self.expand + or "categories" in self._property_filters + or "category" in self._property_filters + or any(op == "==" for op in self._property_operator.values()) + or not calendar.client.features.is_supported("search.text.case-sensitive") + or not calendar.client.features.is_supported("search.time-range.accurate") + ) ): post_filter = True @@ -508,7 +576,8 @@ def _search_impl( ## expansion is unreliable (the master expands without knowing its exceptions, yielding ## duplicate occurrences). Fall back to server-side expansion when it handles exceptions. if ( - self.expand + cw + and self.expand and not server_expand and not calendar.client.features.is_supported("save-load.event.recurrences.exception") and calendar.client.features.is_supported("search.recurrences.expanded.exception") @@ -523,7 +592,8 @@ def _search_impl( ## (e.g. purelymail where both i;octet and i;ascii-casemap collations are unsupported). ## Remove all text-value filters and rely on client-side post_filter instead. if ( - not calendar.client.features.is_supported("search.text") + cw + and not calendar.client.features.is_supported("search.text") and self._property_filters and post_filter is not False ): @@ -545,7 +615,8 @@ def _search_impl( ## special compatbility-case for servers that does not ## support category search properly if ( - not calendar.client.features.is_supported("search.text.category") + cw + and not calendar.client.features.is_supported("search.text.category") and ("categories" in self._property_filters or "category" in self._property_filters) and post_filter is not False ): @@ -562,7 +633,7 @@ def _search_impl( ## special compatibility-case for servers that do not support is-not-defined ## for specific properties (e.g. search.is-not-defined.category or .dtend) - if post_filter is not False: + if cw and post_filter is not False: undef_props_without_support = [ prop for prop, op in self._property_operator.items() @@ -587,7 +658,8 @@ def _search_impl( ## special compatibility-case for servers that do not support substring search if ( - not calendar.client.features.is_supported("search.text.substring") + cw + and not calendar.client.features.is_supported("search.text.substring") and post_filter is not False ): explicit_contains = [ @@ -614,7 +686,7 @@ def _search_impl( ## special compatibility-case for servers that does not ## support combined searches very well - if not calendar.client.features.is_supported("search.combined-is-logical-and"): + if cw and not calendar.client.features.is_supported("search.combined-is-logical-and"): if self.start or self.end: if self._property_filters: clone = self._clone_without_filters(clear_all_filters=True) @@ -624,7 +696,7 @@ def _search_impl( ) yield ( SearchAction.RETURN, - self.filter(objects, post_filter, split_expanded, server_expand), + self.filter(objects, True, split_expanded, server_expand), ) return @@ -657,7 +729,7 @@ def _search_impl( ## TODO: consider if not ignore_completed3 is sufficient, ## then the recursive part of the query here is moot, and ## we wouldn't waste so much time on repeated queries - if self.todo and self.include_completed is False: + if cw and self.todo and self.include_completed is False: clone = replace(self, include_completed=True) clone.include_completed = True ## Why? Isn't this redundant? clone.expand = False @@ -688,13 +760,7 @@ def _search_impl( (clone, calendar, server_expand, False, props, xml, None, _hacks), ) - # Deduplicate by URL - objects = [] - match_set = set() - for item in matches: - if item.url not in match_set: - match_set.add(item.url) - objects.append(item) + objects = _dedup_by_url(matches) else: orig_xml = xml @@ -703,12 +769,47 @@ def _search_impl( server_expand, props=props, filters=xml, _hacks=_hacks ) - if not self.comp_class and not calendar.client.features.is_supported( - "search.comp-type.optional" - ): - if self.include_completed is None: - self.include_completed = True - + ## A CALDAV:time-range (and VALARM) filter is a component-level filter: + ## RFC4791 section 9.7 only allows it inside a comp-filter for + ## VEVENT/VTODO/VJOURNAL/VFREEBUSY/VALARM, never directly under VCALENDAR. + ## So when no component type is given we cannot place such a filter in an + ## RFC-legal way - we must split the search into one query per component + ## type (search.time-range.comp-type-optional). This is independent of + ## search.comp-type.optional, which only governs comp-type-less queries + ## WITHOUT any filter. + ## The same applies to a prop-filter (CATEGORIES, SUMMARY, ...): under + ## VCALENDAR it would filter on VCALENDAR's own properties (which lack + ## component properties), so servers match nothing + ## (search.text.comp-type-optional). + ## See https://github.com/python-caldav/caldav/issues/681 + has_component_level_filter = bool( + self.start or self.end or self.alarm_start or self.alarm_end + ) + has_property_filter = bool(self._property_filters) + needs_comptype_split = ( + cw + and not self.comp_class + and ( + not calendar.client.features.is_supported("search.comp-type.optional") + or ( + has_component_level_filter + and not calendar.client.features.is_supported( + "search.time-range.comp-type-optional" + ) + ) + or ( + has_property_filter + and not calendar.client.features.is_supported( + "search.text.comp-type-optional" + ) + ) + ) + ) + if needs_comptype_split: + ## The include_completed default for the split is resolved + ## inside _search_with_comptypes, on a clone. Setting it on + ## self here would permanently change the meaning of the + ## caller's searcher - the issue-#650 class of bug. result = yield ( SearchAction.SEARCH_WITH_COMPTYPES, (calendar, server_expand, split_expanded, props, orig_xml, _hacks, post_filter), @@ -722,8 +823,37 @@ def _search_impl( (calendar, xml, self.comp_class, props), ) except error.ReportError as err: + ## Reactive workaround for https://github.com/python-caldav/caldav/issues/681: + ## if the server was (optimistically) configured as supporting + ## search.time-range.comp-type-optional but actually rejects the + ## comp-type-less time-range query (e.g. SabreDAV's HTTP 400 "You cannot + ## add time-range filters on the VCALENDAR component"), retry by splitting + ## into one query per component type. Also covers prop-filters + ## (search.text.comp-type-optional). orig_xml must be empty - if the + ## caller passed a full calendar-query we cannot rebuild it per comp-type. if ( - calendar.client.features.backward_compatibility_mode + cw + and not self.comp_class + and not orig_xml + and (has_component_level_filter or has_property_filter) + ): + result = yield ( + SearchAction.SEARCH_WITH_COMPTYPES, + ( + calendar, + server_expand, + split_expanded, + props, + orig_xml, + _hacks, + post_filter, + ), + ) + yield (SearchAction.RETURN, result) + return + if ( + cw + and calendar.client.features.backward_compatibility_mode and not self.comp_class and "400" not in err.reason ): @@ -780,29 +910,17 @@ def _search_impl( ) return - # Post-process: load objects - obj2 = [] - for o in objects: - try: - yield (SearchAction.LOAD_OBJECT, o) - obj2.append(o) - except Exception: - logging.error( - "Server does not want to reveal details about the calendar object", - exc_info=True, - ) - objects = obj2 + # Post-process: batch-load unloaded objects in one REPORT instead of N GETs + yield (SearchAction.LOAD_OBJECTS_BATCH, (calendar, objects)) + objects = [o for o in objects if o.is_loaded() or o.has_component()] # Google sometimes returns empty objects objects = [o for o in objects if o.has_component()] objects = self.filter(objects, post_filter, split_expanded, server_expand) # Partial workaround for https://github.com/python-caldav/caldav/issues/201 - for obj in objects: - try: - yield (SearchAction.LOAD_OBJECT, obj) - except Exception: - pass + # Re-issue a batch load in case any objects need a second fetch + yield (SearchAction.LOAD_OBJECTS_BATCH, (calendar, objects)) yield (SearchAction.RETURN, self.sort(objects)) @@ -815,6 +933,7 @@ def search( xml: str = None, post_filter=None, _hacks: str = None, + compatibility_workarounds: bool | None = None, ) -> list[CalendarObjectResource]: """Do the search on a CalDAV calendar. @@ -831,6 +950,13 @@ def search( :param xml: XML query to be sent to the server (string or elements) :param post_filter: Do client-side filtering after querying the server :param _hacks: Please don't ask! + :param compatibility_workarounds: When ``False``, all server-compatibility + workarounds are disabled and the query is sent verbatim + (single REPORT, no comp-type splitting, no filter + rewriting, no fallback retries). Mainly for the + server-compatibility checker, to observe raw server + behaviour. ``None`` (the default) leaves the searcher's + current setting unchanged. Make sure not to confuse he CalDAV properties with iCalendar properties. @@ -851,36 +977,52 @@ def search( flag on. """ + if compatibility_workarounds is not None: + self._compatibility_workarounds = compatibility_workarounds gen = self._search_impl( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) - result = None - try: - action, data = gen.send(result) - except StopIteration: - return [] - - while True: + ## The driver alternates Phase 1 (execute the yielded action, here) and + ## Phase 2 (feed the result/exception back, in _advance_search_gen). Only + ## Phase 1 differs between sync and async; the generator protocol is shared. + step = _advance_search_gen(gen) # prime the generator + while step is not None: + action, data = step + if action == SearchAction.RETURN: + return data + result = exc = None try: - if action == SearchAction.RECURSIVE_SEARCH: - clone, cal, srv_exp, spl_exp, prp, xm, pf, hk = data - result = clone.search(cal, srv_exp, spl_exp, prp, xm, pf, hk) - elif action == SearchAction.SEARCH_WITH_COMPTYPES: - cal, srv_exp, spl_exp, prp, xm, hk, pf = data - result = self._search_with_comptypes(cal, srv_exp, spl_exp, prp, xm, hk, pf) - elif action == SearchAction.REQUEST_REPORT: - cal, xm, comp_cls, prp = data - result = cal._request_report_build_resultlist(xm, comp_cls, props=prp) - elif action == SearchAction.LOAD_OBJECT: - data.load(only_if_unloaded=True) - result = None - elif action == SearchAction.RETURN: - return data - - action, data = gen.send(result) - except StopIteration: - return [] + result = self._dispatch_search_action(action, data) + except Exception as e: + exc = e + step = _advance_search_gen(gen, result, exc) + return [] + + def _dispatch_search_action(self, action: SearchAction, data: Any) -> Any: + """Phase 1 of the sync search driver: execute one yielded SearchAction. + + Returns the value to feed back into the generator (``None`` for actions + whose effect is a side effect). ``RETURN`` is handled by the driver + loop itself. Sync twin of :meth:`_async_dispatch_search_action`. + """ + if action == SearchAction.RECURSIVE_SEARCH: + clone, cal, srv_exp, spl_exp, prp, xm, pf, hk = data + return clone.search(cal, srv_exp, spl_exp, prp, xm, pf, hk) + if action == SearchAction.SEARCH_WITH_COMPTYPES: + cal, srv_exp, spl_exp, prp, xm, hk, pf = data + return self._search_with_comptypes(cal, srv_exp, spl_exp, prp, xm, hk, pf) + if action == SearchAction.REQUEST_REPORT: + cal, xm, comp_cls, prp = data + return cal._request_report_build_resultlist(xm, comp_cls, props=prp) + if action == SearchAction.LOAD_OBJECT: + data.load(only_if_unloaded=True) + return None + if action == SearchAction.LOAD_OBJECTS_BATCH: + cal, objs = data + cal._batch_load_objects(objs) + return None + raise AssertionError(f"unhandled search action {action!r}") def _search_with_comptypes( self, @@ -894,6 +1036,10 @@ def _search_with_comptypes( ) -> list[CalendarObjectResource]: """ Internal method - does three searches, one for each comp class (event, journal, todo). + + Note that the results come back grouped by component type rather than + in server order; three queries cannot preserve an order that only one + query ever had. Pass a sort key if the order matters. """ if xml and (isinstance(xml, str) or "calendar-query" in xml.tag): # Full XML provided – cannot inject a comp-type filter into it. @@ -904,19 +1050,19 @@ def _search_with_comptypes( return self.sort(objects) objects = [] - assert self.event is None and self.todo is None and self.journal is None + base = self._comptype_split_base() for comp_class in (Event, Todo, Journal): if not calendar.client.features.is_supported( f"save-load.{comp_class.__name__.lower()}" ): continue - clone = replace(self) + clone = replace(base) clone.comp_class = comp_class objects += clone.search( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) - return self.sort(objects) + return self.sort(_dedup_by_url(objects)) async def async_search( self, @@ -927,6 +1073,7 @@ async def async_search( xml: str = None, post_filter=None, _hacks: str = None, + compatibility_workarounds: bool | None = None, ) -> list["AsyncCalendarObjectResource"]: """Async version of search() - does the search on an AsyncCalendar. @@ -935,40 +1082,52 @@ async def async_search( See the sync search() method for full documentation. """ + if compatibility_workarounds is not None: + self._compatibility_workarounds = compatibility_workarounds gen = self._search_impl( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) - result = None - - try: - action, data = gen.send(result) - except StopIteration: - return [] - while True: + ## See the sync search() driver: only Phase 1 (the action execution) is + ## awaited here; the Phase-2 generator protocol is shared via + ## _advance_search_gen. + step = _advance_search_gen(gen) # prime the generator + while step is not None: + action, data = step + if action == SearchAction.RETURN: + return data + result = exc = None try: - if action == SearchAction.RECURSIVE_SEARCH: - clone, cal, srv_exp, spl_exp, prp, xm, pf, hk = data - result = await clone.async_search(cal, srv_exp, spl_exp, prp, xm, pf, hk) - elif action == SearchAction.SEARCH_WITH_COMPTYPES: - cal, srv_exp, spl_exp, prp, xm, hk, pf = data - result = await self._async_search_with_comptypes( - cal, srv_exp, spl_exp, prp, xm, hk, pf - ) - elif action == SearchAction.REQUEST_REPORT: - cal, xm, comp_cls, prp = data - result = await cal._request_report_build_resultlist(xm, comp_cls, props=prp) - elif action == SearchAction.LOAD_OBJECT: - load_result = data.load(only_if_unloaded=True) - if inspect.isawaitable(load_result): - await load_result - result = None - elif action == SearchAction.RETURN: - return data - - action, data = gen.send(result) - except StopIteration: - return [] + result = await self._async_dispatch_search_action(action, data) + except Exception as e: + exc = e + step = _advance_search_gen(gen, result, exc) + return [] + + async def _async_dispatch_search_action(self, action: SearchAction, data: Any) -> Any: + """Phase 1 of the async search driver: execute one yielded SearchAction. + + Async twin of :meth:`_dispatch_search_action`; see it for semantics. + """ + if action == SearchAction.RECURSIVE_SEARCH: + clone, cal, srv_exp, spl_exp, prp, xm, pf, hk = data + return await clone.async_search(cal, srv_exp, spl_exp, prp, xm, pf, hk) + if action == SearchAction.SEARCH_WITH_COMPTYPES: + cal, srv_exp, spl_exp, prp, xm, hk, pf = data + return await self._async_search_with_comptypes(cal, srv_exp, spl_exp, prp, xm, hk, pf) + if action == SearchAction.REQUEST_REPORT: + cal, xm, comp_cls, prp = data + return await cal._request_report_build_resultlist(xm, comp_cls, props=prp) + if action == SearchAction.LOAD_OBJECT: + load_result = data.load(only_if_unloaded=True) + if inspect.isawaitable(load_result): + await load_result + return None + if action == SearchAction.LOAD_OBJECTS_BATCH: + cal, objs = data + await cal._async_batch_load_objects(objs) + return None + raise AssertionError(f"unhandled search action {action!r}") async def _async_search_with_comptypes( self, @@ -990,20 +1149,20 @@ async def _async_search_with_comptypes( return self.sort(objects) objects: list[AsyncCalendarObjectResource] = [] - assert self.event is None and self.todo is None and self.journal is None + base = self._comptype_split_base() for comp_class in (Event, Todo, Journal): if not calendar.client.features.is_supported( f"save-load.{comp_class.__name__.lower()}" ): continue - clone = replace(self) + clone = replace(base) clone.comp_class = comp_class results = await clone.async_search( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) objects.extend(results) - return self.sort(objects) + return self.sort(_dedup_by_url(objects)) def filter( self, @@ -1039,6 +1198,27 @@ def filter( server_expand=server_expand, ) + def _comptype_split_base(self) -> "CalDAVSearcher": + """Return the searcher the per-comp-type clones are built from. + + A truthy ``event``/``todo``/``journal`` flag would have produced a + ``comp_class``, and the split only happens when there is none — so + reaching here with one set means the caller and the driver disagree. + ``event=False`` is *not* such a case: it is a legal argument to the + public ``search()``, and testing it with ``is None`` used to trip a + bare, message-less ``AssertionError``. + + ``include_completed`` defaults to True for the split (otherwise the + VTODO sub-search would quietly drop completed tasks), but that is + resolved on a copy: writing it back to ``self`` would change the + meaning of the caller's searcher for every later call — the + issue-#650 class of bug. + """ + error.assert_(not (self.event or self.todo or self.journal)) + if self.include_completed is None: + return replace(self, include_completed=True) + return self + def build_search_xml_query(self, server_expand=False, props=None, filters=None, _hacks=None): """Build a CalDAV calendar-query XML request. diff --git a/caldav/testing.py b/caldav/testing.py index 1211f30b..f87c2730 100644 --- a/caldav/testing.py +++ b/caldav/testing.py @@ -9,6 +9,7 @@ Docker and external server support lives in tests/test_servers/ (source only). """ +import copy import socket import tempfile import threading @@ -123,7 +124,7 @@ def __init__(self, config: dict[str, Any] | None = None) -> None: if "features" not in config: from caldav import compatibility_hints - features = compatibility_hints.xandikos.copy() + features = copy.deepcopy(compatibility_hints.xandikos) features["auto-connect.url"]["domain"] = f"{config['host']}:{config['port']}" config["features"] = features super().__init__(config) @@ -265,7 +266,7 @@ def __init__(self, config: dict[str, Any] | None = None) -> None: if "features" not in config: from caldav import compatibility_hints - features = compatibility_hints.radicale.copy() + features = copy.deepcopy(compatibility_hints.radicale) features["auto-connect.url"]["domain"] = f"{config['host']}:{config['port']}" config["features"] = features super().__init__(config) diff --git a/docs/design/FULL_CODE_REVIEW_2026-06.md b/docs/design/FULL_CODE_REVIEW_2026-06.md index 988887c3..61fac23f 100644 --- a/docs/design/FULL_CODE_REVIEW_2026-06.md +++ b/docs/design/FULL_CODE_REVIEW_2026-06.md @@ -51,7 +51,7 @@ real bugs, clustering around four themes: ## 1. Crash bugs (realistic trigger → unhandled exception) -### 1.1 `calendarobjectresource.py:1167` + `:1187` — 302 handling iterates headers as tuples `[repro]` +### 1.1 `calendarobjectresource.py:1167` + `:1187` — 302 handling iterates headers as tuples `[repro]` ✅ FIXED (commit 22b9cc66+1) `[x[1] for x in r.headers if x[0] == "location"][0]` — iterating a dict-like `Headers` object (niquests `CaseInsensitiveDict` sync, `httpx.Headers` async) yields key *strings*, so `x[0]` is the first character of each header name. @@ -60,7 +60,7 @@ instead of following the redirect. The same broken pattern appears twice because the whole block is pasted twice (see §6.2; the second copy is partly dead code). Fix: `r.headers.get("location")`. -### 1.2 `davclient.py:302` — URL with username but no password → TypeError `[repro]` +### 1.2 `davclient.py:302` — URL with username but no password → TypeError `[repro]` ✅ FIXED `DAVClient(url='https://user@example.com/dav/', password='secret')`: `self.url.username` is set, so `unquote(self.url.password)` runs with `password=None` → TypeError inside `urllib.parse.unquote`. The async client @@ -68,7 +68,7 @@ dead code). Fix: `r.headers.get("location")`. also gives **explicit kwargs precedence over URL credentials, while sync does the opposite**. Pick one precedence (kwargs should win) and share the code. -### 1.3 `davclient.py:836` / `async_davclient.py:376` — rate-limit retry: `None + float` `[code]` +### 1.3 `davclient.py:836` / `async_davclient.py:376` — rate-limit retry: `None + float` `[code]` ✅ FIXED `sleep_seconds += rate_limit_time_slept / 2` executes *before* the `sleep_seconds is None` check. With `rate_limit_handle=True` and `rate_limit_default_sleep=None`: first 429 has `Retry-After: 5` → retried; @@ -76,7 +76,7 @@ second 429 has no usable Retry-After (`compute_sleep_seconds` returns None, e.g. `Retry-After: 0`) → `None += 2.5` → TypeError instead of the documented `RateLimitError`. Same bug copy-pasted in both clients. -### 1.4 `async_davclient.py:1272` — `aio.get_calendars(calendar_name=...)` can never work `[code]` +### 1.4 `async_davclient.py:1272` — `aio.get_calendars(calendar_name=...)` can never work `[code]` ✅ FIXED The async module-level helper awaits the *synchronous* `Principal.calendar()`, which has no async dispatch (`collection.py:448–475`): `calendar_home_set` → `get_property` returns a coroutine for async clients, and @@ -85,7 +85,7 @@ coroutine → TypeError (swallowed into an empty result when `raise_errors=False`). Name-based calendar lookup via `caldav.aio` is broken end-to-end. -### 1.5 `collection.py:601` — async `freebusy_request` with Principal attendees → AttributeError `[code]` +### 1.5 `collection.py:601` — async `freebusy_request` with Principal attendees → AttributeError `[code]` ✅ FIXED `add_attendee(attendee)` is called *before* the `is_async_client` branch at line 604. For a `Principal` attendee on an async client, `get_vcal_address()` returns a coroutine, and `add_attendee` then does @@ -93,13 +93,13 @@ line 604. For a `Principal` attendee on an async client, `_async_save_with_invites` (`collection.py:983–984`) already does the awaited conversion correctly — the same dance is missing here. -### 1.6 `calendarobjectresource.py:727` — `add_attendee("MAILTO:user@example.com")` → UnboundLocalError `[code]` +### 1.6 `calendarobjectresource.py:727` — `add_attendee("MAILTO:user@example.com")` → UnboundLocalError `[code]` ✅ FIXED The string-branch chain is case-sensitive: uppercase `MAILTO:` (common in real-world iCalendar; RFC 3986 schemes are case-insensitive) fails `startswith("mailto:")` and fails the `":" not in attendee` branch, so `attendee_obj` is never assigned and line 742 raises UnboundLocalError. -### 1.7 `calendarobjectresource.py:1272` — `change_attendee_status` raises bare KeyError; `:1284` literal `%s` `[repro]` +### 1.7 `calendarobjectresource.py:1272` — `change_attendee_status` raises bare KeyError; `:1284` literal `%s` `[repro]` ✅ FIXED When the component has no ATTENDEE property at all, `ical_obj['attendee']` raises `KeyError('ATTENDEE')` — not `error.NotFoundError`, which is the only thing the principal-address loops catch — so the "Principal is not invited" @@ -107,38 +107,38 @@ fallback is unreachable. Additionally the genuine not-found raise is `error.NotFoundError("Participant %s not found in attendee list")` with no `% attendee`: the user literally sees `%s`. -### 1.8 `lib/auth.py:31` — IndexError on malformed WWW-Authenticate `[repro]` +### 1.8 `lib/auth.py:31` — IndexError on malformed WWW-Authenticate `[repro]` ✅ FIXED `extract_auth_types('Basic realm="x",')` (trailing comma — seen in the wild) → the empty segment makes `h.split()[0]` raise IndexError, aborting the auth negotiation with an unrelated traceback. Guard with `for h in header.split(",") if h.strip()`. -### 1.9 `config.py:37` — missing section raises KeyError instead of returning empty `[repro]` +### 1.9 `config.py:37` — missing section raises KeyError instead of returning empty `[repro]` ✅ FIXED `expand_config_section` does `config[section]` for non-glob names. A config file with only named sections (no `default`) makes plain `caldav.get_calendars()` crash with `KeyError: 'default'` instead of falling through to "no configuration found". -### 1.10 `compatibility_hints.py:611` — `copyFeatureSet` crashes merging plain-string features `[repro]` +### 1.10 `compatibility_hints.py:611` — `copyFeatureSet` crashes merging plain-string features `[repro]` ✅ FIXED `FeatureSet({'scheduling': 'unsupported'}).copyFeatureSet({'scheduling': 'fragile'})` → bare AssertionError: the `'support' not in server_node` guard makes string-valued updates of an existing feature fall through to the final `else: raise AssertionError`. Plain strings are the dominant style in the hint dicts, so any two-layer merge expressing the same feature crashes. -### 1.11 `compatibility_hints.py:605` — unknown feature names: warn now, crash later `[repro]` +### 1.11 `compatibility_hints.py:605` — unknown feature names: warn now, crash later `[repro]` ✅ FIXED A typoed feature name in a user's config produces only a UserWarning at set time, but the bad key is still stored — a later `collapse()` / `is_supported()` hits a message-less AssertionError in `find_feature`, far from the config that caused it. Reject (or drop) the key at intake instead. -### 1.12 `lib/vcal.py:93` — bare `assert` on server-supplied data `[repro]` +### 1.12 `lib/vcal.py:93` — bare `assert` on server-supplied data `[repro]` ✅ FIXED Truncated/garbage iCalendar without DTSTAMP and without an `END:` line makes `fix()` raise a bare AssertionError. Under `python -O` the assert (and thus the DTSTAMP fixup logic it guards) is silently skipped. Should be `error.assert_` or a proper parse error. -### 1.13 `jmap/client.py:576` / `jmap/async_client.py:461` — `create_task` missing the guard `create_event` has `[code]` +### 1.13 `jmap/client.py:576` / `jmap/async_client.py:461` — `create_task` missing the guard `create_event` has `[code]` ✅ FIXED `create_event` handles an empty `created` dict with a descriptive `JMAPMethodError` (`client.py:294–298`); `create_task` does `created["new-0"]["id"]` unguarded → bare KeyError, bypassing the JMAP error @@ -148,7 +148,7 @@ hierarchy callers are told to catch. Copy-paste gap in both clients. ## 2. Silent wrong results / data corruption -### 2.1 `lib/vcal.py:80` — COMPLETED fixup merges the next line into the property ⚠ data corruption `[repro]` +### 2.1 `lib/vcal.py:80` — COMPLETED fixup merges the next line into the property ⚠ data corruption `[repro]` ✅ FIXED (commit 22b9cc66) `fix()` normalizes CRLF→LF first, then the COMPLETED date-to-datetime regex `(\d+)\s` *consumes the newline without restoring it*: `COMPLETED:20240101\nSUMMARY:hello` becomes @@ -156,24 +156,24 @@ hierarchy callers are told to catch. Copy-paste gap in both clients. destroyed and the object parses with corrupted data. This runs on every inbound object. -### 2.2 `lib/vcal.py:242` — `create_ical(ical_fragment=...)` injects the fragment inside VALARM `[repro]` +### 2.2 `lib/vcal.py:242` — `create_ical(ical_fragment=...)` injects the fragment inside VALARM `[repro]` ✅ FIXED The fragment is re-inserted before the first `^END:V` line — which is `END:VALARM` when any `alarm_*` props were given. `ical_fragment='RRULE:...'` plus an alarm produces an event *without* recurrence and with an invalid RRULE inside the alarm. Should target `END:VEVENT|VTODO|VJOURNAL`. -### 2.3 `lib/vcal.py:88` — trailing-whitespace fixup is dead code `[repro]` +### 2.3 `lib/vcal.py:88` — trailing-whitespace fixup is dead code `[repro]` ✅ FIXED `re.sub(" *$", "", fixed)` without `re.MULTILINE` only touches the document end, never the per-line trailing spaces (iCloud X-APPLE-STRUCTURED-EVENT) that docstring fix #4 targets. The vobject traceback it was written to prevent still occurs. -### 2.4 `lib/vcal.py:85` — backslash-unescape regex is a no-op `[repro]` +### 2.4 `lib/vcal.py:85` — backslash-unescape regex is a no-op `[repro]` ✅ FIXED `re.sub(r"\\+('\")", r"\1", fixed)` matches only the literal two-character sequence `'"`; the group should be a character class `['\"]`. Harmless for compliant data, but the fix does nothing. -### 2.5 `lib/url.py:143` + `:159` — `canonical()` keeps credentials and mutates self `[repro]` +### 2.5 `lib/url.py:143` + `:159` — `canonical()` keeps credentials and mutates self `[repro]` ✅ FIXED Two related bugs: (a) `canonical()` builds its result from `self.url_parsed` instead of the `unauth()`'ed URL, so `URL('https://user:pass@example.com/cal/').canonical()` **retains the @@ -184,7 +184,7 @@ then overwrites `url_raw`/`url_parsed` **in place** — a mere `==` comparison silently rewrites the URL (port added, path re-quoted; a literal `+` becomes `%2B`), so subsequent requests can go to a different resource. -### 2.6 `search.py:648` — `combined-is-logical-and` workaround silently drops property filters `[code]` +### 2.6 `search.py:648` — `combined-is-logical-and` workaround silently drops property filters `[code]` ✅ FIXED The workaround strips property filters from the server query but passes the *ambient* `post_filter` (still `None` on otherwise-capable servers — e.g. Nextcloud, whose only relevant flag is `search.combined-is-logical-and: @@ -194,58 +194,58 @@ range. The sibling workarounds at 597–604 and 625–632 correctly force `post_filter=True`; this branch also uniquely lacks the `post_filter is not False` guard. -### 2.7 `search.py:193` — `undef` operator misses the category→CATEGORIES alias `[code]` +### 2.7 `search.py:193` — `undef` operator misses the category→CATEGORIES alias `[code]` ✅ FIXED The `undef` branch emits `PropFilter(property.upper())` without the alias mapping the non-undef branch applies, so `add_property_filter('category', '', operator='undef')` queries the nonexistent property `CATEGORY` — `is-not-defined` on it matches *every* object, returning events that do have categories. -### 2.8 `search.py:362`/`:506` — documented `'=='` exact-match is never enforced `[code]` +### 2.8 `search.py:362`/`:506` — documented `'=='` exact-match is never enforced `[code]` ✅ FIXED The docstring promises "`==` — exact match required, enforced client-side", but no code path inspects the `==` operator (only `'contains'` is checked at line 617) and the post-filter default block ignores it. On a fully-capable server, RFC 4791 substring `text-match` semantics leak through: `'=='` `'rain'` matches "Training". -### 2.9 `calendarobjectresource.py:1570` — `_set_data` leaves a stale `DataState` cache `[repro]` +### 2.9 `calendarobjectresource.py:1570` — `_set_data` leaves a stale `DataState` cache `[repro]` ✅ FIXED The raw-string branch clears the legacy instance attributes but never resets `self._state`. Sequence: fetch event → touch `event.id` / `is_loaded()` (caches state v1) → `event.load()` assigns `self.data = r.raw` → afterwards `get_data()` / `get_icalendar_instance()` / `id` still serve the **pre-reload content** while `.data` returns the new content. -### 2.10 `datastate.py:152` (+ `:67`, `:78`) — `BEGIN:FREEBUSY` never matches `VFREEBUSY` `[repro]` +### 2.10 `datastate.py:152` (+ `:67`, `:78`) — `BEGIN:FREEBUSY` never matches `VFREEBUSY` `[repro]` ✅ FIXED The component-type sniffing tests for `BEGIN:FREEBUSY`; real data says `BEGIN:VFREEBUSY`. A `FreeBusy` object holding raw data gets `get_component_type() → None`, so `is_loaded()`/`has_component()` are False, `save()` **silently no-ops** at the early return, and `load(only_if_unloaded=True)` reloads spuriously. -### 2.11 `calendarobjectresource.py:1943` — `_get_duration` isinstance check on the wrapper, not `.dt` `[repro]` +### 2.11 `calendarobjectresource.py:1943` — `_get_duration` isinstance check on the wrapper, not `.dt` `[repro]` ✅ FIXED `isinstance(i["DTSTART"], datetime)` tests the icalendar `vDDDTypes` wrapper (never a datetime), so the date-vs-datetime branch always takes the date path: a VTODO with a timed DTSTART and no DUE/DURATION gets duration **1 day instead of 0**. Completing a recurring task then sets the next DUE a full day late, and `Todo._next` shifts the recurrence. -### 2.12 `calendarobjectresource.py:2140` — sync safe-mode completion ignores `completion_timestamp` `[code]` +### 2.12 `calendarobjectresource.py:2140` — sync safe-mode completion ignores `completion_timestamp` `[code]` ✅ FIXED `_complete_recurring_safe` calls `completed.complete()` (defaults to *now*) while the async twin passes the caller's timestamp through. Sync/async divergence with user-visible effect on the recorded COMPLETED time. -### 2.13 `base_client.py:689` — calendar with displayname `""` dropped from results `[code]` +### 2.13 `base_client.py:689` — calendar with displayname `""` dropped from results `[code]` ✅ FIXED `if _try(calendar.get_display_name, ...)` is a truthiness check, so a calendar explicitly requested by URL whose displayname is the empty string is silently omitted. The async counterpart (`async_davclient.py:1262`) correctly uses `is not None`. -### 2.14 `async_davclient.py:957` — async `get_calendars()` lacks the GMX principal-URL fallback `[code]` +### 2.14 `async_davclient.py:957` — async `get_calendars()` lacks the GMX principal-URL fallback `[code]` ✅ FIXED Sync `get_calendars()` (`davclient.py:486–489`) falls back to the principal URL when `calendar-home-set` is missing; async returns `[]` for the same server. Parity gap. -### 2.15 `async_davclient.py:487` — issue-#158 workaround can return the probe response as the real one `[code]` +### 2.15 `async_davclient.py:487` — issue-#158 workaround can return the probe response as the real one `[code]` ✅ FIXED When the original request dies with a connection abort, the workaround sends a probe GET; if that GET is *not* 401+WWW-Authenticate (e.g. 200 with a login page), the code falls through and returns the **probe GET's response as the @@ -253,25 +253,25 @@ original request's response** — the caller sees status 200 for a PUT that never happened, and the real connection error is lost. Also: the sync client has no #158 workaround at all (parity gap in the other direction). -### 2.16 `async_davclient.py:435` — HTML-on-401 hint checks the wrong headers `[code]` +### 2.16 `async_davclient.py:435` — HTML-on-401 hint checks the wrong headers `[code]` ✅ FIXED The diagnostic checks `self.headers` (the client's own request headers) for `text/html` instead of `r.headers`, so the intended "server returned HTML, maybe set auth_type" hint can never fire. -### 2.17 `config.py:50` — `disable: true` ignored for named sections `[repro]` +### 2.17 `config.py:50` — `disable: true` ignored for named sections `[repro]` ✅ FIXED `expand_config_section` checks `config.get("section", ...)` with the string literal `"section"` instead of the variable. `disable` only works under `section='*'`; sections pulled in via a meta-section's `contains` list (or by name) connect to servers the user explicitly disabled. -### 2.18 `config.py:265` — explicit params without url/features silently discarded `[code]` +### 2.18 `config.py:265` — explicit params without url/features silently discarded `[code]` ✅ FIXED `get_connection_params` honors `explicit_params` only when `url` or `features` is present, and never merges them with the env/file source that wins: `get_davclient(password='secret')` with `CALDAV_URL`/`CALDAV_USERNAME` in env returns a config **without the password**, contradicting the docstring's "explicit parameters take highest priority". -### 2.19 `config.py:180` + `testing.py:127`/`:263` — shared module-level hint dicts get mutated `[repro]` +### 2.19 `config.py:180` + `testing.py:127`/`:263` — shared module-level hint dicts get mutated `[repro]` ✅ FIXED `resolve_features` with a string name returns the module-level `compatibility_hints` dict itself (the `base` branch deepcopies; this branch doesn't). `XandikosServer`/`RadicaleServer` then do a *shallow* `.copy()` and @@ -285,7 +285,7 @@ permanently polluted for the whole process, redirecting any later ## 3. Security -### 3.1 `discovery.py:329` — `require_tls=True` not enforced on well-known redirect target `[code]` +### 3.1 `discovery.py:329` — `require_tls=True` not enforced on well-known redirect target `[code]` ✅ FIXED `_well_known_lookup` never receives `require_tls`; a same-domain `Location: http://...` passes the `_is_subdomain_or_same` check and is returned as `ServiceInfo(tls=False)`, which `discover_service` returns unchecked. A @@ -294,7 +294,7 @@ guarantee to plaintext, and credentials follow. (Otherwise the discovery module's security posture is good: require_tls defaults True, same-domain redirect validation, single manual redirect hop.) -### 3.2 `response.py:277` — XML parser for untrusted server data lacks entity hardening `[code]` +### 3.2 `response.py:277` — XML parser for untrusted server data lacks entity hardening `[code]` ✅ FIXED `etree.XMLParser(remove_blank_text=True, huge_tree=self.huge_tree)` relies on libxml2 defaults for entity resolution. Current libxml2 blocks the classic XXE paths, but the library makes no guarantee across the unpinned dependency @@ -303,12 +303,14 @@ of server data in the package — one line fixes it: add `resolve_entities=False` (and consider `no_network=True`, `dtd_validation=False` explicitly). -### 3.3 `lib/error.py:51` — `PYTHON_CALDAV_COMMDUMP` persists bodies/headers in /tmp (low) `[code]` +**Fixed**: added `resolve_entities=False, no_network=True` to the `etree.XMLParser` call in `response.py:277`. `dtd_validation=False` is lxml's default so was not added explicitly. + +### 3.3 `lib/error.py:51` — `PYTHON_CALDAV_COMMDUMP` persists bodies/headers in /tmp (low) `[code]` ✅ FIXED `NamedTemporaryFile(delete=False)` dumps full request/response headers and bodies (calendar PII, custom auth headers) to files that accumulate indefinitely. Files are 0600, and the niquests-applied Authorization header is added after the dump point, so exposure is limited — but a cleanup policy -or a documented warning would be appropriate. +or a documented warning would be appropriate. **Human notes:** This is in /tmp, so we should expect some kind of cleanup on the OS level. There does exist some security notes in the CHANGELOG for the revision adding the feature, but the CHANGELOG has been pruned, so it's needed to consult git history to find it - it should definitively be lifted up to a more visible place. **Ruled out** (checked, found safe): SSRF via server-returned hrefs (`_normalize_href` reduces absolute URLs to path-only); credential leak on @@ -319,43 +321,43 @@ cross-host redirects (auth applied via auth callable, stripped by ## 4. JMAP backend -### 4.1 `jmap/convert/jscal_to_ical.py:384` — override child VEVENT gets the master's DTSTART `[repro]` +### 4.1 `jmap/convert/jscal_to_ical.py:384` — override child VEVENT gets the master's DTSTART `[repro]` ✅ FIXED `child_start = patch.get("start", start_str)` defaults to the master start. An override that doesn't move the occurrence (e.g. title-only change — the common case) renders a child VEVENT with RECURRENCE-ID at the occurrence but DTSTART at the *master's* start, relocating the occurrence. Default must be the override key (`rid_dt`). -### 4.2 `jmap/convert/jscal_to_ical.py:375` — EXDATE/RECURRENCE-ID value-type mismatch `[repro]` +### 4.2 `jmap/convert/jscal_to_ical.py:375` — EXDATE/RECURRENCE-ID value-type mismatch `[repro]` ✅ FIXED Override keys are rendered as naive floating DATE-TIMEs regardless of the event's `timeZone`/`showWithoutTime`: a TZID-anchored event gets `EXDATE:20260620T100000` (floating — per RFC 5545 it does not match the instance, so the **excluded occurrence reappears**), and an all-day event gets a DATETIME EXDATE against a `VALUE=DATE` DTSTART. -### 4.3 `jmap/convert/ical_to_jscal.py:100` (via `_utils.py:129`) — `Z`-suffix in LocalDateTime slots `[repro]` +### 4.3 `jmap/convert/ical_to_jscal.py:100` (via `_utils.py:129`) — `Z`-suffix in LocalDateTime slots `[repro]` ✅ FIXED UTC inputs produce `...Z` strings for RRULE `until` and recurrenceOverrides keys; RFC 8984 requires LocalDateTime there. Strict servers reject with `invalidArguments`; lenient ones mis-set the boundary, and a `Z`-suffixed override key can never match a LocalDateTime occurrence key. -### 4.4 `jmap/convert/*` — STATUS dropped in both directions `[code]` +### 4.4 `jmap/convert/*` — STATUS dropped in both directions `[code]` ✅ FIXED Neither converter maps `STATUS` ↔ `status` (only participationStatus/freeBusyStatus exist). `STATUS:CANCELLED` round-trips to the JSCalendar default `confirmed`; cancelled meetings come back as active. -### 4.5 `jmap/client.py:346` / `async_client.py:233` — `update_event` patch never clears removed properties `[code]` +### 4.5 `jmap/client.py:346` / `async_client.py:233` — `update_event` patch never clears removed properties `[code]` ✅ FIXED The full converted object is sent as the RFC 8620 PatchObject; the converter only includes keys conditionally, so a property deleted client-side (e.g. LOCATION, VALARM) is simply *absent* from the patch and **persists on the server**. Clearing requires explicit `null` entries. -### 4.6 `jmap/objects/calendar.py:113` — search `after`/`before` are not UTCDate `[code]` +### 4.6 `jmap/objects/calendar.py:113` — search `after`/`before` are not UTCDate `[code]` ✅ FIXED `datetime.isoformat()` is passed straight through (naive → no `Z`, aware → `+02:00` offset, plus microseconds); JMAP requires `...Z` UTCDate. Strict servers reject the query; lenient ones interpret the window inconsistently. -### 4.7 `jmap/client.py:462` / `async_client.py:347` — `newState` from `/changes` discarded `[code]` +### 4.7 `jmap/client.py:462` / `async_client.py:347` — `newState` from `/changes` discarded `[code]` ✅ FIXED `get_objects_by_sync_token` unpacks `new_state` into `_`. Callers' only option for a new baseline is a separate `get_sync_token()` call — changes landing in between are silently skipped on the next sync. @@ -373,50 +375,99 @@ producing drift bugs. modulo `await`. The build-side is already shared via `_JMAPClientBase` / `jmap/_methods`; moving the response-parsing glue into shared pure methods would shrink each sync/async method to ~3 lines. (The §1.13 and - §4.5 bugs are duplicated exactly because of this.) + §4.5 bugs are duplicated exactly because of this.) ✅ FIXED (commit ed3c47b0) 2. **`calendarobjectresource.py:1166–1205` — `_post_put` block pasted twice in sequence**; the second `elif r.status not in (204, 201)` is unreachable. Also factor the Etag/Schedule-Tag header→props snippet repeated in `load`/`_async_load` (the code itself carries a "consider - refactoring - this is repeated many places now" comment). + refactoring - this is repeated many places now" comment). ✅ FIXED — the + dead second copy in `_post_put` was removed and the Etag/Schedule-Tag + capture extracted into a shared `_update_tag_props()` helper now used by + `_post_put`, `load`, and `_async_load`. 3. **`async_davclient.py` re-implements ~200 lines of `DAVClient`** (init tail, get_calendars, rate-limit retry loop — byte-identical except `time.sleep` vs `asyncio.sleep`). The §2.14 GMX gap and §1.3 retry bug are direct drift products. Move into `BaseDAVClient` / `lib/error.py`. + ✅ FIXED — the byte-identical pure logic is now shared via + `BaseDAVClient`: `_init_rate_limit_config()` (the rate-limit init tail), + `_rate_limit_sleep_seconds()` (the retry sleep-decision — where §1.3 + lived), and `_calendar_home_url()` / `_build_calendars_from_propfind()` + (the get_calendars post-processing — where §2.14 lived). Only the + irreducible per-twin parts remain duplicated: the actual `time.sleep` vs + `await asyncio.sleep`, the awaited PROPFIND/principal I/O, and the + library-specific session/header setup in `__init__`. 4. **`search.py:869` — post-processing loads unloaded results one GET at a time**; `Calendar._multiget` can fetch them in a single REPORT. On the issue-#201 workaround path a 200-event search costs ~200 extra - round-trips. + round-trips. ✅ FIXED 5. **JMAP clients open a fresh HTTP connection per request** (async: `async with AsyncSession()` per `_request`; sync: module-level `requests.post`). `__exit__`/`__aexit__` already exist but do nothing — - hold one session in `_JMAPClientBase` and close it there. + hold one session in `_JMAPClientBase` and close it there. ✅ FIXED 6. **`Todo._async_complete_recurring_thisandfuture` copies ~60 lines of its sync twin** (the file says "TERRIBLY much code duplication here"), and the async safe-variant has drifted: it PUTs the completed copy twice. Extract a pure icalendar-mutation helper; keep 5-line sync/async - wrappers. + wrappers. ✅ FIXED — the icalendar mutation now lives once in the pure + (no-I/O) `_prepare_recurring_thisandfuture()` and + `_build_recurring_safe_completed()`; each sync/async twin is reduced to + a thin wrapper that does only the `await`-able save(s). The double-PUT of + the completed copy is gone (the copy is now completed in memory and PUT + once). New offline unit tests in `TestRecurringCompleteHelpers` cover the + mutation and the single-PUT invariant. 7. **`response.py` carries two parallel multistatus-parsing stacks** — legacy `_find_objects_and_props`/`expand_simple_props` (still load-bearing for `_multiget`, report-result building, `search_principals`) vs the newer dataclass parsers. Every parsing quirk (Confluence %2540, purelymail 404) must be maintained twice; the TODO at line 577 already - acknowledges this. + acknowledges this. ✅ FIXED (the duplicated structural parsing) — the + *named* quirks were already shared: Confluence `%2540` and absolute-URL + normalization live in `_normalize_href`, and the purelymail/stalwart 404 + response shapes in `_parse_response`, both of which both stacks call. The + remaining genuinely-duplicated piece — the propstat iteration plus the + "a 404 propstat means the property is absent" skip — is now in the single + `_collect_prop_elements()` helper, used by both `_extract_properties` + (dataclass stack) and `_find_objects_and_props` (legacy stack). As a side + effect the legacy path dropped its over-strict per-propstat asserts + (status-present / `cnt == len(propstat)` / non-404-status validation) that + the dataclass path never had, so the two stacks now treat odd-shaped + propstats identically. `TestParserStackEquivalence` guards the agreement. + What is *not* collapsed (and was not, to avoid touching the slow + integration-tested `_multiget`/report/`search_principals` paths): the two + value-conversion APIs — `_element_to_value` (pre-parsed values for the + dataclass results) vs `_expand_simple_prop` (caller-directed text + expansion). Those are two output formats, not a duplicated quirk; fully + migrating the `expand_simple_props` consumers onto the dataclass results + remains a larger follow-on. 8. **`search.py` sync/async driver loops duplicated** (~80 lines including - the Phase-1/Phase-2 exception-rethrow protocol and + the hase-1/Phase-2 exception-rethrow protocol and `_search_with_comptypes`). A small executor object with sync/async - implementations would leave one driver. + implementations would leave one driver. ✅ FIXED — the drift-prone + Phase-2 generator protocol (`gen.throw`/`gen.send`/`StopIteration`) now + lives once in the shared module-level `_advance_search_gen()`; each + driver loop is reduced to priming + a `while` that defers Phase-1 to a + `_dispatch_search_action` / `_async_dispatch_search_action` method. Only + the irreducible `await` and the per-action one-liners remain duplicated. --- ## 6. Altitude / design notes 1. **`response.py:464` — server fingerprints hardcoded in the generic - parser.** purelymail's `{https://purelymail.com}does-not-exist` tag, - Stalwart's "No resources found" string, and SOGo status notes live in the - core multistatus path instead of going through the compatibility-hints - feature matrix. Adding the next server's 404 shape means editing generic + parser.** ✅ FIXED. purelymail's `{https://purelymail.com}does-not-exist` + tag, Stalwart's "No resources found" string, and SOGo status notes lived in + the core multistatus path instead of going through the compatibility-hints + feature matrix. Adding the next server's 404 shape meant editing generic parsing — the exact inversion the hints mechanism exists to avoid. + + **Resolution:** `` and `` are optional children + of `` per RFC 4918 with server-defined content, so no per-server + config (nor content fingerprint) is warranted at all — `_parse_response` + now accepts either element generically. A genuinely novel tag still hits + `error.weirdness()`. The `check_404` debug guard was made `None`-safe (a + server may legally send these without a response-level status). Tests: + `test_parse_sync_collection_generic_responsedescription` / + `test_parse_sync_collection_generic_error` in `tests/test_protocol.py`. 2. **`vcal.fix()` is a regex-rewriting layer applied to every inbound object.** §2.1–§2.4 show the current fixups are individually broken in four different ways; the module's own TODOs flag the approach. Worth @@ -424,6 +475,8 @@ producing drift bugs. regression-testing each fixup against the exact server output it was written for. + **Human comment:** we've been discussing a bit moving this logic into the icalendar library - but it's hard to make good solutions, so we'll need to keep the stop-gap implementation as for now. + --- ## 7. Recommended priorities diff --git a/docs/design/RELEASE-HOWTO.md b/docs/design/RELEASE-HOWTO.md index f8764cab..e5cb4fa2 100644 --- a/docs/design/RELEASE-HOWTO.md +++ b/docs/design/RELEASE-HOWTO.md @@ -27,16 +27,24 @@ I have no clue on the proper procedures for doing releases, and I keep on doing * Run tests (particularly the style check): `pytest` and `tox -e style`. TODO: is `tox -e style` still relevant? * Push the code to github: `cd ~/caldav ; git push ; git push --tags` * Some people relies on the github release system for finding releases - go to https://github.com/python-caldav/caldav/releases/new, choose the new tag, copy the version number and the release notes in. Remember to check the box to make it the latest release. -* The most important part - push to pypi: +* The most important part - push to pypi. Note that the virtualenv is created + *outside* the release clone: `python -m build` packages the directory it is + pointed at, and a venv sitting inside it goes straight into the tarball. + That is how `caldav-3.2.1.tar.gz` came to contain 1755 files under `venv/`. ``` + python3 -m venv ~/caldav-release-venv + . ~/caldav-release-venv/bin/activate + pip install -U pip build twine tox cd ~/caldav-release - python3 -m venv venv - . venv/bin/activate - pip install -U pip build twine + tox -e package # builds sdist+wheel and fails if anything untracked is in them python -m build python -m twine upload dist/* ``` -* Remove the release dir: `rm -r caldav-release` + `tox -e package` is the safety net for this whole class of mistake: it + compares the sdist file list against `git ls-files` and refuses anything git + does not track. It runs in CI too, but run it here as well - CI checks the + *repository*, this checks the *tree you are about to upload*. +* Remove the release dir and its venv: `rm -r ~/caldav-release ~/caldav-release-venv` ## List of mistakes to be avoided @@ -48,5 +56,5 @@ This is most likely not complete, but should explain some of the "silly" steps a * Forgetting to add new files to the git repo * Having checked out a branch or tag or something, and tagging that as the new release rather than the latest HEAD. * Forgetting to push to pypi, or pushing something else than the tagged revision to pypi -* Pushing out junk files in the pypi-release (i.e. .pyc-files, log files, temp files, `tests/conf_private.py`, `tests/caldav_test_servers.yaml`, etc +* Pushing out junk files in the pypi-release (i.e. .pyc-files, log files, temp files, `tests/conf_private.py`, `tests/caldav_test_servers.yaml`, an entire `venv/`, etc). `tox -e package` now catches this - see the build step above * Not adding the release to the "github releases" (I don't care much about this feature, but apparently some people check there to find the latest release version) diff --git a/docs/source/http-libraries.rst b/docs/source/http-libraries.rst index dcc97b5f..f77ba8a7 100644 --- a/docs/source/http-libraries.rst +++ b/docs/source/http-libraries.rst @@ -11,47 +11,58 @@ There is also information in `GitHub issue #457