fix(dav): parse multistatus with namespaces, not substrings #430

2026-08-15T02:36:10Z

KofTwentyTwo commented

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

(Migrated from github.com)

What was silently wrong

CalDavClient::parse_multistatus_response and
CardDavClient::parse_multistatus_response located their payloads by splitting the
response body on the literal strings <c:calendar-data> and <card:address-data>.
XML namespace prefixes are arbitrary, so this only worked for servers that happened
to choose the same prefixes Nuncio's own request bodies use.

Every failure mode was silent — the client returned Ok with a short or empty list,
and nothing was logged:

A different prefix returned nothing. <D:multistatus xmlns:CAL="…caldav"> —
perfectly compliant — yielded zero events and reported success. The user saw an
empty calendar and had no way to tell it apart from a calendar that really is empty.
A default namespace returned nothing. Same for <multistatus xmlns="DAV:">.
A lookalike element was read as the payload. <c:calendar-data-summary> contains
the searched substring, so its content was fed to the iCalendar parser.
A 404 propstat was treated as a value. RFC 4918 propstat status was never read,
so a property the server explicitly said it could not return was parsed as if it had.
A body that was not a multistatus at all — a captive portal's HTML, a truncated
response — parsed to zero results and returned success.

What changed

New nuncio_core::dav: a namespace-aware multistatus walker over quick-xml
(already a workspace dependency; it was declared by both DAV crates and never used).
CalDAV and CardDAV return the same multistatus/response/propstat/prop shape and
differ only in which property carries the payload, so the traversal is written once and
parameterized by {namespace}local-name.

Elements match on their resolved namespace and local name — never a prefix, never a
substring. DAV:, urn:ietf:params:xml:ns:caldav, urn:ietf:params:xml:ns:carddav.
<propstat> status is honoured: only a 2xx propstat's content is a value. A status
line with no readable code counts as a failure rather than being optimistically accepted.
(A propstat with no <status> at all is accepted — RFC 4918 requires one, but only an
explicitly non-2xx status makes the content a non-value.)
Nothing is dropped in silence. A body that cannot be traversed is now
CalendarError::MalformedResponse / CardDavError::MalformedResponse (new variants),
not an empty list. Every <response> that yields no readable value is returned in
MultistatusReport::skipped with a reason (Status(…) / EmptyValue / Absent), and
both clients log each one at warn! with the href and reason, counted into the existing
dropped field of the sync completion event.
Markup nested inside the payload property (which is character data only) is an error
rather than being quietly discarded by a text-only capture.
quick-xml moved from nuncio-cal/nuncio-contacts to nuncio-core; no new crates.

Both clients are done — CardDAV keeps its existing fail-fast behaviour on an unparseable
vCard, which is already an honest error rather than a silent drop.

Tests

No existing assertion was weakened or deleted, and no existing test encoded the substring
behaviour — the pre-existing fixtures all used d:/c:/card: prefixes, which the new
parser reads identically. They pass unchanged.

Added (all offline, wiremock-mocked transport):

18 unit tests on the shared walker (nuncio-core/src/dav.rs): prefix variants, default
namespace, substring-lookalike element, same local name in a foreign namespace, non-2xx
propstat, response-level status, empty property, non-multistatus root, mismatched end tag,
truncated document, undeclared prefix, nested markup, entity refs and CDATA, unresolvable
entity, property outside a propstat/prop.
Client unit tests in both crates: three prefix bindings of the same document; warn!
assertions (via the existing tracing recorder) that a lookalike element and a 404
propstat each produce a visible warning with href and reason; malformed and non-multistatus
bodies produce MalformedResponse.
wiremock integration tests in both crates driving a real REPORT round trip:
uppercase-prefix and default-namespace documents, lookalike element, 404 propstat,
truncated body, HTML body.

Mutation-verified. Changing CALDAV_NS to a wrong value fails 16 tests, all on the
CalDAV path, leaving CardDAV green; changing CARDDAV_NS fails 10, all on the CardDAV path,
leaving CalDAV green; changing DAV_NS fails 39. The namespace matching is genuinely load-bearing.

Gate

cargo fmt --all -- --check, cargo check-all, cargo test-all all green, plus
RUST_TEST_THREADS=2 cargo test-all three times with zero FAILED lines each.

Closes #257

## What was silently wrong `CalDavClient::parse_multistatus_response` and `CardDavClient::parse_multistatus_response` located their payloads by splitting the response body on the literal strings `<c:calendar-data>` and `<card:address-data>`. XML namespace prefixes are arbitrary, so this only worked for servers that happened to choose the same prefixes Nuncio's own request bodies use. Every failure mode was silent — the client returned `Ok` with a short or empty list, and nothing was logged: - **A different prefix returned nothing.** `<D:multistatus xmlns:CAL="…caldav">` — perfectly compliant — yielded zero events and reported success. The user saw an empty calendar and had no way to tell it apart from a calendar that really is empty. - **A default namespace returned nothing.** Same for `<multistatus xmlns="DAV:">`. - **A lookalike element was read as the payload.** `<c:calendar-data-summary>` contains the searched substring, so its content was fed to the iCalendar parser. - **A `404` propstat was treated as a value.** RFC 4918 propstat status was never read, so a property the server explicitly said it could not return was parsed as if it had. - **A body that was not a multistatus at all** — a captive portal's HTML, a truncated response — parsed to zero results and returned success. ## What changed New `nuncio_core::dav`: a namespace-aware `multistatus` walker over `quick-xml` (already a workspace dependency; it was declared by both DAV crates and never used). CalDAV and CardDAV return the same `multistatus`/`response`/`propstat`/`prop` shape and differ only in which property carries the payload, so the traversal is written once and parameterized by `{namespace}local-name`. - Elements match on their **resolved namespace and local name** — never a prefix, never a substring. `DAV:`, `urn:ietf:params:xml:ns:caldav`, `urn:ietf:params:xml:ns:carddav`. - **`<propstat>` status is honoured**: only a 2xx propstat's content is a value. A status line with no readable code counts as a failure rather than being optimistically accepted. (A propstat with no `<status>` at all is accepted — RFC 4918 requires one, but only an explicitly non-2xx status makes the content a non-value.) - **Nothing is dropped in silence.** A body that cannot be traversed is now `CalendarError::MalformedResponse` / `CardDavError::MalformedResponse` (new variants), not an empty list. Every `<response>` that yields no readable value is returned in `MultistatusReport::skipped` with a reason (`Status(…)` / `EmptyValue` / `Absent`), and both clients log each one at `warn!` with the href and reason, counted into the existing `dropped` field of the sync completion event. - Markup nested inside the payload property (which is character data only) is an error rather than being quietly discarded by a text-only capture. - `quick-xml` moved from `nuncio-cal`/`nuncio-contacts` to `nuncio-core`; no new crates. Both clients are done — CardDAV keeps its existing fail-fast behaviour on an unparseable vCard, which is already an honest error rather than a silent drop. ## Tests No existing assertion was weakened or deleted, and no existing test encoded the substring behaviour — the pre-existing fixtures all used `d:`/`c:`/`card:` prefixes, which the new parser reads identically. They pass unchanged. Added (all offline, `wiremock`-mocked transport): - 18 unit tests on the shared walker (`nuncio-core/src/dav.rs`): prefix variants, default namespace, substring-lookalike element, same local name in a foreign namespace, non-2xx propstat, response-level status, empty property, non-multistatus root, mismatched end tag, truncated document, undeclared prefix, nested markup, entity refs and CDATA, unresolvable entity, property outside a `propstat`/`prop`. - Client unit tests in both crates: three prefix bindings of the same document; `warn!` assertions (via the existing tracing recorder) that a lookalike element and a `404` propstat each produce a visible warning with href and reason; malformed and non-multistatus bodies produce `MalformedResponse`. - `wiremock` integration tests in both crates driving a real `REPORT` round trip: uppercase-prefix and default-namespace documents, lookalike element, `404` propstat, truncated body, HTML body. **Mutation-verified.** Changing `CALDAV_NS` to a wrong value fails 16 tests, all on the CalDAV path, leaving CardDAV green; changing `CARDDAV_NS` fails 10, all on the CardDAV path, leaving CalDAV green; changing `DAV_NS` fails 39. The namespace matching is genuinely load-bearing. ## Gate `cargo fmt --all -- --check`, `cargo check-all`, `cargo test-all` all green, plus `RUST_TEST_THREADS=2 cargo test-all` three times with zero `FAILED` lines each. Closes #257

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#430