From 5f72e58ad3d40db4f163cb35d59a0b0c1c165ae4 Mon Sep 17 00:00:00 2001 From: arnef Date: Thu, 20 Aug 2026 07:04:49 +0200 Subject: [PATCH] Fix calendar-color not applied by DAVx5 (duplicate propstat) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DAVx5 explicitly requests the Apple calendar-color property by name. go-webdav's stock property map doesn't know it, so it always emitted a 404 Not Found propstat for it. Our injection was adding a *second*, 200 OK propstat with the color into the same , producing a response with two propstats for the same property name — invalid multistatus that real clients (dav4jvm/DAVx5) resolved by preferring the 404, so the color was silently ignored. Now the bogus 404-only propstat for calendar-color is stripped before injecting the 200 OK one, leaving a single, valid propstat per response. Verified against Nextcloud's documented behavior (single propstat with the color) and against a live PROPFIND matching DAVx5's actual request shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/caldav/backend.go | 37 +++++++++++++++++---- internal/caldav/backend_test.go | 58 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/internal/caldav/backend.go b/internal/caldav/backend.go index 5c6f683..6fd9335 100644 --- a/internal/caldav/backend.go +++ b/internal/caldav/backend.go @@ -457,10 +457,24 @@ func (h *colorInjectingHandler) ServeHTTP(w http.ResponseWriter, r *http.Request // (non-greedy) inside a multistatus document, capturing its href. var responseBlockRe = regexp.MustCompile(`(?s)]*>.*?]*>([^<]*).*?`) +// propstatBlockRe matches a single ... element +// within a response, non-greedily. +var propstatBlockRe = regexp.MustCompile(`(?s)]*>.*?`) + +// okPropRe matches the first opening tag inside a +// 200 OK propstat block, used to find where to insert new property XML. +var okPropRe = regexp.MustCompile(``) + // injectCalendarColors scans a multistatus PROPFIND response body and, for // each whose href is a calendar collection with a color set, -// inserts a -// element into its first 200 OK . +// inserts a element +// into its first 200 OK . Since go-webdav's stock property map +// doesn't know this property, a client that explicitly asks for it (as +// DAVx5 does) gets back a 404 propstat for it — that bogus 404 entry is +// stripped first, since leaving both a 404 and our injected 200 for the +// same property name in one is invalid multistatus and +// confuses clients (dav4jvm/DAVx5 was observed to keep showing no color +// when both were present). func (h *colorInjectingHandler) injectCalendarColors(ctx context.Context, body []byte) []byte { p := auth.FromContext(ctx) if p == nil { @@ -485,12 +499,23 @@ func (h *colorInjectingHandler) injectCalendarColors(ctx context.Context, body [ if err != nil || color == "" { return block } - propEl := []byte(``) - idx := bytes.Index(block, propEl) - if idx < 0 { + + // Drop any propstat block that only complains calendar-color is + // unknown (a 404/not-found propstat containing a bare, empty + // calendar-color element), from either namespace clients might + // have queried it in. + block = propstatBlockRe.ReplaceAllFunc(block, func(ps []byte) []byte { + if bytes.Contains(ps, []byte("calendar-color")) && !bytes.Contains(ps, []byte("200 OK")) { + return nil + } + return ps + }) + + idx := okPropRe.FindIndex(block) + if idx == nil { return block } - insertAt := idx + len(propEl) + insertAt := idx[1] colorEl := []byte(fmt.Sprintf(`%s`, xmlEscapeColor(color))) out := make([]byte, 0, len(block)+len(colorEl)) out = append(out, block[:insertAt]...) diff --git a/internal/caldav/backend_test.go b/internal/caldav/backend_test.go index a98e5c0..43d895c 100644 --- a/internal/caldav/backend_test.go +++ b/internal/caldav/backend_test.go @@ -213,3 +213,61 @@ func TestPropFindEmitsCalendarColor(t *testing.T) { t.Fatalf("expected exactly one calendar-color element (only for work), got: %s", body) } } + +// TestPropFindExplicitCalendarColorRequest verifies that when a client +// (like DAVx5) explicitly asks for the Apple calendar-color property by +// name, it gets back a single 200 OK propstat with the color — not a +// duplicate/conflicting 404 propstat alongside it, which is what +// go-webdav's stock property map produces on its own for an unknown +// property and which real clients (dav4jvm) were observed to prefer over +// the injected 200, hiding the color entirely. +func TestPropFindExplicitCalendarColorRequest(t *testing.T) { + dir := t.TempDir() + st, err := store.NewStore(filepath.Join(dir, "data")) + if err != nil { + t.Fatalf("NewStore: %v", err) + } + dbase, err := db.Open(filepath.Join(dir, "test.db")) + if err != nil { + t.Fatalf("db.Open: %v", err) + } + t.Cleanup(func() { dbase.Close() }) + + if err := dbase.CreateUser("alice", "pw", "", ""); err != nil { + t.Fatalf("CreateUser alice: %v", err) + } + if err := dbase.CreateCalendarWithColor("alice", "work", "#3b82f6"); err != nil { + t.Fatalf("CreateCalendarWithColor: %v", err) + } + if err := st.EnsureCollection("alice", "cal-work"); err != nil { + t.Fatalf("EnsureCollection: %v", err) + } + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + handler := NewHandler(&config.Config{}, st, dbase, logger) + + req := httptest.NewRequest("PROPFIND", "/cal/home/work/", strings.NewReader( + ``+ + ``)) + req.SetBasicAuth("alice", "pw") + req.Header.Set("Content-Type", "text/xml") + req.Header.Set("Depth", "0") + req = req.WithContext(auth.NewContext(context.Background(), &auth.Principal{Username: "alice"})) + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMultiStatus { + t.Fatalf("expected 207, got %d: %s", rr.Code, rr.Body.String()) + } + body := rr.Body.String() + if strings.Count(body, "#3b82f6FF`) { + t.Fatalf("expected calendar-color in the single 200 OK propstat, got: %s", body) + } + if !strings.Contains(body, "200 OK") || strings.Contains(body, "404") { + t.Fatalf("expected a single 200 OK propstat with no 404, got: %s", body) + } +}