Interviews: on a server that percent-encodes its hrefs (Radicale), every event is flagged removed on the second run #19

New issue

Closed

opened 2026-10-01 10:47:51 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:51 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, reading the sync and running it against the test suite's fake server with its listing hrefs percent-encoded.

An interview's event is written to an address built from the interview's UID, and that literal string is stored on the link. The UID always contains an @. A server that percent-encodes the hrefs it lists, as Radicale does, lists the same event with %40, and the plugin compares the two strings as they are. On the second run every interview looks deleted from the calendar: it is flagged removed, a false entry goes on the application's timeline, and the interview is never pushed or pulled again. Radicale is one of the servers the README names.

Where: src/postulo_dav/sync.py:576 (_sync_interviews, href = urljoin(calendar.href, f"{interview.uid}.ics"), stored as remote_href at :581); src/postulo_dav/sync.py:589-601 (if link.remote_href not in remote: flags remote_gone and calls record_event); src/postulo_dav/sync.py:667-673 (the same resource counted as foreign); src/postulo_dav/dav.py:204-221 (Server.etags, which keys its result by self.absolute(href_node.text.strip()) and uses unquote only to skip the collection itself).
What happens:
1. The core mints every interview UID as <uuid4>@postulo (applications/models.py:694 in the core). Run 1 PUTs …/postulo-interviews/<uuid>@postulo.ics and stores that string.
2. Radicale lists the resource through urllib.parse.quote, whose default safe set is /, so the PROPFIND answer says …/<uuid>%40postulo.ics.
3. Run 2: the stored href is not a key of remote, so the link gets remote_gone = True, the report says "1 removed, 1 skipped · 1 events on the calendar are not from Postulo and were left alone", and the timeline gains "Panel removed from the calendar on another device; kept here."
4. From then on if link.remote_gone: continue (:569-570) skips the interview: a time moved here never reaches the phone, and one moved on the phone never reaches Postulo.
  Reproduced with the FakeDav from tests/test_dav.py, changed only so that _multistatus passes each href through quote(unquote(href)). Contacts escape only because their resource names are bare uuid4 values (:441).
Why it matters: the calendar half of the plugin stops working for every interview of every Radicale user after one run, silently, and writes false entries into the application's timeline, which Postulo treats as the record of what happened.
Fix: compare hrefs in one canonical form. Normalise in one place in dav.py (decode the path with unquote and re-encode it with one fixed safe set, or keep decoded paths throughout) and apply it to the keys etags() returns, to every href stored on a link and to every lookup against them. Independently, build new resource names from a filename-safe form of the UID (for example the UUID part alone) so the address carries no reserved characters to begin with; existing links keep working once comparison is canonical.

Related to #7, which asks that a twin missing from the listing be looked up by its UID before it is called gone; that would hide this case but not fix the comparison.

Done when

A FakeDav variant in tests/test_dav.py that percent-encodes its listing hrefs, and a test there that schedules an interview, syncs twice and asserts the link is not remote_gone, the second run reports nothing to do, and no "removed from the calendar" entry is on the timeline.

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, reading the sync and running it against the test suite's fake server with its listing hrefs percent-encoded. An interview's event is written to an address built from the interview's UID, and that literal string is stored on the link. The UID always contains an `@`. A server that percent-encodes the hrefs it lists, as Radicale does, lists the same event with `%40`, and the plugin compares the two strings as they are. On the second run every interview looks deleted from the calendar: it is flagged removed, a false entry goes on the application's timeline, and the interview is never pushed or pulled again. Radicale is one of the servers the README names. - **Where:** `src/postulo_dav/sync.py:576` (`_sync_interviews`, `href = urljoin(calendar.href, f"{interview.uid}.ics")`, stored as `remote_href` at `:581`); `src/postulo_dav/sync.py:589-601` (`if link.remote_href not in remote:` flags `remote_gone` and calls `record_event`); `src/postulo_dav/sync.py:667-673` (the same resource counted as foreign); `src/postulo_dav/dav.py:204-221` (`Server.etags`, which keys its result by `self.absolute(href_node.text.strip())` and uses `unquote` only to skip the collection itself). - **What happens:** 1. The core mints every interview UID as `<uuid4>@postulo` (`applications/models.py:694` in the core). Run 1 PUTs `…/postulo-interviews/<uuid>@postulo.ics` and stores that string. 2. Radicale lists the resource through `urllib.parse.quote`, whose default safe set is `/`, so the PROPFIND answer says `…/<uuid>%40postulo.ics`. 3. Run 2: the stored href is not a key of `remote`, so the link gets `remote_gone = True`, the report says "1 removed, 1 skipped · 1 events on the calendar are not from Postulo and were left alone", and the timeline gains "Panel removed from the calendar on another device; kept here." 4. From then on `if link.remote_gone: continue` (`:569-570`) skips the interview: a time moved here never reaches the phone, and one moved on the phone never reaches Postulo. Reproduced with the `FakeDav` from `tests/test_dav.py`, changed only so that `_multistatus` passes each href through `quote(unquote(href))`. Contacts escape only because their resource names are bare `uuid4` values (`:441`). - **Why it matters:** the calendar half of the plugin stops working for every interview of every Radicale user after one run, silently, and writes false entries into the application's timeline, which Postulo treats as the record of what happened. - **Fix:** compare hrefs in one canonical form. Normalise in one place in `dav.py` (decode the path with `unquote` and re-encode it with one fixed safe set, or keep decoded paths throughout) and apply it to the keys `etags()` returns, to every href stored on a link and to every lookup against them. Independently, build new resource names from a filename-safe form of the UID (for example the UUID part alone) so the address carries no reserved characters to begin with; existing links keep working once comparison is canonical. Related to #7, which asks that a twin missing from the listing be looked up by its UID before it is called gone; that would hide this case but not fix the comparison. ## Done when - A `FakeDav` variant in `tests/test_dav.py` that percent-encodes its listing hrefs, and a test there that schedules an interview, syncs twice and asserts the link is not `remote_gone`, the second run reports nothing to do, and no "removed from the calendar" entry is on the timeline.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:47:51 +00:00

tiagoagueda added the

bug

label

2026-10-01 10:47:51 +00:00

tiagoagueda referenced this issue from a commit

2026-10-19 07:13:10 +00:00

Review fixes: pairing by UID, bounded reads, per-resource tolerance, notes per reader, and a calendar that carries no attendee

tiagoagueda referenced this issue

2026-10-19 07:13:15 +00:00

Review fixes: pairing by UID, bounded reads, notes per reader, per-resource tolerance #35