M6.5 [P2]: DAV clients need real namespace-aware XML parsing (stop substring split + silent drops) #257

New Issue

2026-07-31T14:42:53Z

KofTwentyTwo commented

2026-07-31 14:42:53 +00:00

(Migrated from github.com)

CalDavClient/CardDavClient parse_multistatus_response (caldav.rs:69-95, nuncio-contacts/carddav.rs:74-95) split the multistatus XML on a literal tag string rather than real namespace-aware XML parsing: a compliant server using a different namespace prefix silently yields ZERO results instead of an error, and the CalDAV path silently drops a malformed VEVENT with no logging. Goal: parse the DAV multistatus with a real namespace-aware XML parser and surface/log parse failures rather than returning empty silently. Tests with alternate namespace prefixes and a malformed entry.

Source: full-tree audit 2026-07-30 (scratchpad/audit/AUDIT-SUMMARY.md). Follow docs/STORY-WORKFLOW.md: engine+proto(if surface)+CLI+offline E2E, mock-only tests, no fabricated data, comment convention, PR to dev, PO reviews before close.

CalDavClient/CardDavClient parse_multistatus_response (caldav.rs:69-95, nuncio-contacts/carddav.rs:74-95) split the multistatus XML on a literal tag string rather than real namespace-aware XML parsing: a compliant server using a different namespace prefix silently yields ZERO results instead of an error, and the CalDAV path silently drops a malformed VEVENT with no logging. Goal: parse the DAV multistatus with a real namespace-aware XML parser and surface/log parse failures rather than returning empty silently. Tests with alternate namespace prefixes and a malformed entry. Source: full-tree audit 2026-07-30 (scratchpad/audit/AUDIT-SUMMARY.md). Follow docs/STORY-WORKFLOW.md: engine+proto(if surface)+CLI+offline E2E, mock-only tests, no fabricated data, comment convention, PR to dev, PO reviews before close.

KofTwentyTwo commented

2026-08-15 02:49:10 +00:00

(Migrated from github.com)

Fixed in #430 (fb03f2e), merged to dev. Worse than the title suggested.

Both clients split the body on the literal <c:calendar-data> / <card:address-data>. A compliant server using any other prefix, or xmlns="DAV:", returned Ok(vec![]) with nothing logged. A <c:calendar-data-summary> was read as the payload. Propstat status was never read, so a 404 propstat parsed as a value. An HTML error page parsed to "empty calendar, success" — a server outage indistinguishable from an empty calendar.

Replaced with one namespace-aware multistatus walker in nuncio_core::dav, parameterised by {namespace}local-name, on quick-xml (already a workspace dep, declared by both DAV crates and unused). 2xx-only propstats; an unreadable body yields MalformedResponse; every value-less <response> is returned in skipped with a reason, logged with its href, and counted into the existing dropped field — so nothing vanishes silently.

Mutation-verified: wrong DAV_NS → 39 failures, CALDAV_NS → 16 (CardDAV green), CARDDAV_NS → 10. No existing assertion weakened — the old fixtures all used d:/c:/card:, which the new parser reads identically.

Documented judgement: a <propstat> with no <status> is accepted as success. RFC 4918 requires one, but rejecting it would fail fixtures real servers produce.

Fixed in #430 (`fb03f2e`), merged to `dev`. Worse than the title suggested. Both clients split the body on the literal `<c:calendar-data>` / `<card:address-data>`. A compliant server using any other prefix, or `xmlns="DAV:"`, returned `Ok(vec![])` **with nothing logged**. A `<c:calendar-data-summary>` was read as the payload. Propstat status was never read, so a `404` propstat parsed as a value. An HTML error page parsed to "empty calendar, success" — a server outage indistinguishable from an empty calendar. Replaced with one namespace-aware `multistatus` walker in `nuncio_core::dav`, parameterised by `{namespace}local-name`, on `quick-xml` (already a workspace dep, declared by both DAV crates and unused). 2xx-only propstats; an unreadable body yields `MalformedResponse`; every value-less `<response>` is returned in `skipped` with a reason, logged with its href, and counted into the existing `dropped` field — so nothing vanishes silently. Mutation-verified: wrong `DAV_NS` → 39 failures, `CALDAV_NS` → 16 (CardDAV green), `CARDDAV_NS` → 10. No existing assertion weakened — the old fixtures all used `d:`/`c:`/`card:`, which the new parser reads identically. Documented judgement: a `<propstat>` with no `<status>` is accepted as success. RFC 4918 requires one, but rejecting it would fail fixtures real servers produce.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#257