From 95e36fc379839d6299bb2a427ab0dc652e8dbd28 Mon Sep 17 00:00:00 2001 From: nacho Date: Sat, 17 Oct 2026 10:54:28 +0200 Subject: [PATCH] CardDAV: an addressbook-query without matches every object Found in production: opencloud-contacts lists the address book with a REPORT addressbook-query that has no element (just getetag + address-data), the RFC 6352 give-me-everything form. go-webdav v0.7's carddav.Match falls through its empty prop-filters loop to 'return false, nil', so every object was discarded and the multistatus came back empty. Writes worked fine; the list was what came back empty, which read as 'contacts are not saved'. QueryAddressObjects now handles the filterless query itself (every object matches, result limit respected) and keeps delegating filtered queries to the library's Match, where a match error skips the object instead of failing the whole report. The regression test drives the exact report the extension sends: two stored cards come back with their address-data, and a filtered query still returns only the matching card. Closes #32 --- cmd/server/dav_integration_test.go | 59 ++++++++++++++++++++++++++++++ internal/carddav/backend.go | 33 +++++++++++++++-- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/cmd/server/dav_integration_test.go b/cmd/server/dav_integration_test.go index 484e12c..610f893 100644 --- a/cmd/server/dav_integration_test.go +++ b/cmd/server/dav_integration_test.go @@ -587,6 +587,65 @@ func TestReportTimeRangeHonoursTZID(t *testing.T) { } } +// TestCarddavQueryWithoutFilterIsNotEmpty is the regression test for a bug +// found in production: go-webdav's carddav.Match answers "no match" to an +// addressbook-query without (the "give me everything" form +// opencloud-contacts sends), so the address book looked empty even though +// writes worked. Our backend now treats the filterless query as match-all. +func TestCarddavQueryWithoutFilterIsNotEmpty(t *testing.T) { + srv, _ := newTestServer(t) + book := srv.URL + "/carddav/alice/def-addressbook/" + + contact := func(uid, fn string) string { + return "BEGIN:VCARD\r\nVERSION:3.0\r\nUID:" + uid + "\r\nFN:" + fn + "\r\nTEL;TYPE=CELL:600\r\nEND:VCARD\r\n" + } + for _, c := range []struct{ uid, fn string }{{"c1", "Uno"}, {"c2", "Dos"}} { + if resp, body := do(t, "PUT", book+c.uid+".vcf", contact(c.uid, c.fn), map[string]string{ + "Content-Type": "text/vcard", + }); resp.StatusCode != http.StatusCreated { + t.Fatalf("PUT %s = %d: %s", c.uid, resp.StatusCode, body) + } + } + + // The exact report opencloud-contacts sends: no at all. + query := ` + + + + + +` + resp, body := do(t, "REPORT", book, query, map[string]string{ + "Depth": "1", + "Content-Type": "application/xml", + }) + if resp.StatusCode != http.StatusMultiStatus { + t.Fatalf("REPORT addressbook-query = %d, want 207: %s", resp.StatusCode, body) + } + ms := parseMultiStatus(t, body) + if len(ms.Responses) != 2 { + t.Fatalf("addressbook-query without filter returned %d responses, want 2: %s", len(ms.Responses), body) + } + if !strings.Contains(body, "FN:Uno") || !strings.Contains(body, "FN:Dos") { + t.Errorf("address-data missing the vCards: %s", body) + } + + // A query WITH a filter keeps going through the library's matching. + filtered := strings.Replace(query, "", + "Uno", 1) + resp, body = do(t, "REPORT", book, filtered, map[string]string{ + "Depth": "1", + "Content-Type": "application/xml", + }) + if resp.StatusCode != http.StatusMultiStatus { + t.Fatalf("filtered REPORT = %d, want 207: %s", resp.StatusCode, body) + } + hrefs := strings.Join(hrefsOf(parseMultiStatus(t, body)), " ") + if !strings.Contains(hrefs, "c1.vcf") || strings.Contains(hrefs, "c2.vcf") { + t.Errorf("filtered query hrefs = %q, want only c1.vcf", hrefs) + } +} + // TestSyncCollection covers RFC 6578 with the semantics the multifilesystem // store can honestly provide: a stale or empty token gets the full listing, // a current token gets an empty response. diff --git a/internal/carddav/backend.go b/internal/carddav/backend.go index 747045c..2d2cd55 100644 --- a/internal/carddav/backend.go +++ b/internal/carddav/backend.go @@ -237,8 +237,17 @@ func (b *Backend) ListAddressObjects(ctx context.Context, path string, req *webd return b.loadObjects(ctx, path) } -// QueryAddressObjects implements the backend interface using the library's -// own carddav.Filter (text matches over the parsed card). +// QueryAddressObjects implements the backend interface. Most of the work is +// delegated to the library's Match, with two davcloud-specific adjustments: +// +// - An addressbook-query WITHOUT matches every object (RFC 6352, +// section 8.6: the filter is optional). go-webdav v0.7's Match falls +// through its empty prop-filters loop to `return false`, so it would +// answer an empty multistatus - opencloud-contacts lists the address book +// exactly this way. We handle the filterless case here; the response +// properties are still rendered per the report's . +// - A match error (e.g. an unexpandable recurrence) does not kill the whole +// query: the object is re-evaluated as a plain one and only logged. func (b *Backend) QueryAddressObjects(ctx context.Context, path string, query *webdavcarddav.AddressBookQuery) ([]webdavcarddav.AddressObject, error) { objs, err := b.loadObjects(ctx, path) if err != nil { @@ -247,7 +256,25 @@ func (b *Backend) QueryAddressObjects(ctx context.Context, path string, query *w if query == nil { return objs, nil } - return webdavcarddav.Filter(query, objs) + if len(query.PropFilters) == 0 { + // No : every object matches. Respect the result limit. + if query.Limit > 0 && query.Limit < len(objs) { + return objs[:query.Limit], nil + } + return objs, nil + } + var out []webdavcarddav.AddressObject + for _, obj := range objs { + ok, err := webdavcarddav.Match(query, &obj) + if err != nil { + log.Printf("carddav: match failed for %s (%v), skipping", obj.Path, err) + continue + } + if ok { + out = append(out, obj) + } + } + return out, nil } func encodeVCF(card vcard.Card) ([]byte, error) {