Fix calendar-color not applied by DAVx5 (duplicate propstat)

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 <response>, 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

@@ -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)<response[^>]*>.*?<href[^>]*>([^<]*)</href>.*?</response>`)
 
+// propstatBlockRe matches a single <propstat>...</propstat> element
+// within a response, non-greedily.
+var propstatBlockRe = regexp.MustCompile(`(?s)<propstat[^>]*>.*?</propstat>`)
+
+// okPropRe matches the first <prop xmlns="DAV:"> opening tag inside a
+// 200 OK propstat block, used to find where to insert new property XML.
+var okPropRe = regexp.MustCompile(`<prop xmlns="DAV:">`)
+
 // injectCalendarColors scans a multistatus PROPFIND response body and, for
 // each <response> whose href is a calendar collection with a color set,
-// inserts a <apple:calendar-color xmlns:apple="http://apple.com/ns/ical/">
+// inserts a <calendar-color xmlns="http://apple.com/ns/ical/"> element
-// element into its first 200 OK <prop>.
+// into its first 200 OK <prop>. 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 <response> 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(`<prop xmlns="DAV:">`)
+
-		idx := bytes.Index(block, propEl)
+		// Drop any propstat block that only complains calendar-color is
-		if idx < 0 {
+		// 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(`<calendar-color xmlns="http://apple.com/ns/ical/">%s</calendar-color>`, xmlEscapeColor(color)))
 		out := make([]byte, 0, len(block)+len(colorEl))
 		out = append(out, block[:insertAt]...)

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(
+		`<?xml version="1.0"?><D:propfind xmlns:D="DAV:" xmlns:A="http://apple.com/ns/ical/">`+
+			`<D:prop><D:displayname/><A:calendar-color/></D:prop></D:propfind>`))
+	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, "<propstat") != 1 {
+		t.Fatalf("expected exactly one propstat (no leftover 404 for calendar-color), got: %s", body)
+	}
+	if !strings.Contains(body, `<calendar-color xmlns="http://apple.com/ns/ical/">#3b82f6FF</calendar-color>`) {
+		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)
+	}
+}