Small fixes: propfind values, password type, DAVError.status, UID conflicts

caldav/base_client.py

@@ -446,7 +446,8 @@ def _raise_authorization_error(self, url_str: str, reason_source: Any) -> NoRetu
         precondition elements parsed out (issue #738) - a 403 with
         ``DAV:valid-sync-token`` means "resync", not "forbidden".
         ``reason_source`` is the raw HTTP response on both clients; httpx
-        names the reason ``reason_phrase``.
+        names the reason ``reason_phrase``.  Its status is kept too, so a
+        caller can tell 401 (credentials) from 403 (permission).
         """
         self._unwind_unprompted_basic()
         reason = getattr(reason_source, "reason", None) or getattr(
@@ -462,6 +463,7 @@ def _raise_authorization_error(self, url_str: str, reason_source: Any) -> NoRetu
             reason=reason,
             preconditions=error.parse_preconditions(body),
             body=body,
+            status=error._status_of(reason_source),
         )
 
     # ── Rate-limit handling ─────────────────────────────────────────────────

caldav/calendarobjectresource.py

@@ -1369,7 +1369,9 @@ def _raise_uid_conflict(e: error.DAVError, consistency: bool = False) -> None:
                 )
             else:
                 cls = error.UID_CONFLICT_ERRORS.get(type(e), error.UidConflictError)
-            raise cls(e.url, e.reason, preconditions=e.preconditions, body=e.body) from e
+            raise cls(
+                e.url, e.reason, preconditions=e.preconditions, body=e.body, status=e.status
+            ) from e
 
     def _post_put(
         self,
@@ -1896,6 +1898,13 @@ def _validate_save_constraints(self, existing, uid, no_overwrite, no_create):
         if not uid and no_create:
             raise error.ConsistencyError("no_create flag was set, but no ID given")
         if no_overwrite and existing:
+            ## The same UID under another URL is what the server's
+            ## CALDAV:no-uid-conflict precondition would refuse (issue #806)
+            target = self.url or (self._generate_url() if uid else None)
+            if existing.url and target and existing.url != target:
+                raise error.UidConflictConsistencyError(
+                    "no_overwrite flag was set, but another object holds the UID"
+                )
             raise error.ConsistencyError("no_overwrite flag was set, but object already exists")
         if no_create and not existing:
             raise error.ConsistencyError("no_create flag was set, but object does not exist")

caldav/collection.py

@@ -1557,7 +1557,9 @@ def _post_multiget(self, results: Iterable[tuple[str, str, dict[str, str]]]) ->
             for url, data, tags in results
         ]
 
-    def multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) -> Iterable[_CC]:
+    def multiget(
+        self, event_urls: Iterable[URL], raise_notfound: bool = False
+    ) -> "list[_CC] | Coroutine[Any, Any, list[_CC]]":
         """
         get multiple events' data
         TODO: Does it overlap the _request_report_build_resultlist method?

caldav/davclient.py

@@ -256,9 +256,6 @@ def __init__(
         self.auth = auth
         self.auth_type = auth_type
 
-        ## I had problems with passwords with non-ascii letters in it ...
-        if isinstance(self.password, str):
-            self.password = self.password.encode("utf-8")
         if auth and self.auth_type:
             logging.error(
                 "both auth object and auth_type sent to DAVClient.  The latter will be ignored."
@@ -716,7 +713,7 @@ def build_auth_object(self, auth_types: list[str] | None = None) -> None:
         # Use shared selection logic
         auth_type = self._select_auth_type(auth_types)
 
-        # Decode password if it's bytes (HTTPDigestAuth needs string)
+        # A caller may still pass the password as bytes (HTTPDigestAuth needs a string)
         password = self.password
         if isinstance(password, bytes):
             password = password.decode("utf-8")

caldav/elements/dav.py

@@ -97,6 +97,10 @@ class SupportedReportSet(BaseElement):
     tag = ns("D", "supported-report-set")
 
 
+class CurrentUserPrivilegeSet(BaseElement):
+    tag: ClassVar[str] = ns("D", "current-user-privilege-set")
+
+
 class Response(BaseElement):
     tag: ClassVar[str] = ns("D", "response")
 

caldav/lib/error.py

@@ -108,7 +108,8 @@ class DAVError(Exception):
 
     ``preconditions`` holds the Clark-notation tags of any precondition
     elements in a ``DAV:error`` response body (RFC 4918 section 16), and
-    ``body`` the raw response body (or None).  A server may explain any
+    ``body`` the raw response body (or None), and ``status`` the HTTP
+    status code (or None) - e.g. 401 or 403 for an AuthorizationError.  A server may explain any
     failure that way, whatever the status: e.g. ``["{DAV:}valid-sync-token"]``
     for an expired sync token (RFC 6578 section 3.2), or
     ``["{urn:ietf:params:xml:ns:caldav}no-uid-conflict"]`` for a create whose
@@ -124,13 +125,15 @@ def __init__(
         reason: str | None = None,
         preconditions: list[str] | None = None,
         body: bytes | str | None = None,
+        status: int | None = None,
     ) -> None:
         if url:
             self.url = url
         if reason:
             self.reason = reason
         self.preconditions = list(preconditions) if preconditions else []
         self.body = body
+        self.status = status
 
     def __str__(self) -> str:
         return "%s at '%s', reason %s" % (
@@ -191,9 +194,20 @@ def error_from_response(
         errmsg(response) if message is None else message,
         preconditions=parse_preconditions(body),
         body=body,
+        status=_status_of(response),
     )
 
 
+def _status_of(response: object) -> int | None:
+    """The HTTP status code of ``response``, which may be a ``DAVResponse``
+    (``status``) or a raw requests/niquests/httpx response (``status_code``)."""
+    for attr in ("status", "status_code"):
+        status = getattr(response, attr, None)
+        if isinstance(status, int):
+            return status
+    return None
+
+
 class PropsetError(DAVError):
     pass
 
@@ -312,8 +326,9 @@ def __init__(
         reason: str | None = None,
         retry_after: str | None = None,
         retry_after_seconds: float | None = None,
+        status: int | None = None,
     ) -> None:
-        super().__init__(url=url, reason=reason)
+        super().__init__(url=url, reason=reason, status=status)
         self.retry_after = retry_after
         self.retry_after_seconds = retry_after_seconds
 
@@ -394,6 +409,7 @@ def raise_if_rate_limited(
         reason=f"Rate limited or service unavailable. Retry after: {retry_after_header}",
         retry_after=retry_after_header,
         retry_after_seconds=retry_seconds,
+        status=status_code,
     )
 
 

caldav/response.py

@@ -182,12 +182,24 @@ def _extract_properties(propstats: "list[_Element]") -> "dict[str, Any]":
 
 
 def _element_to_value(elem: _Element) -> Any:
-    """Convert a complex XML element to a Python value."""
-    if len(elem) == 0:
-        return elem.text
+    """Convert a complex XML element to a Python value.
 
+    A property this does not know, with structure it cannot flatten into
+    a string or a list of strings (e.g. DAV:acl), is returned as the lxml
+    element itself.
+    """
     tag = elem.tag
 
+    if tag in (dav.SupportedReportSet.tag, dav.CurrentUserPrivilegeSet.tag):
+        ## RFC 3253 section 3.1.5: supported-report/report/<name>, and
+        ## RFC 3744 section 5.4: privilege/<name> - the names are wanted
+        return [
+            leaf.tag for leaf in elem.iter(etree.Element) if len(leaf) == 0 and leaf is not elem
+        ]
+
+    if len(elem) == 0:
+        return elem.text
+
     if tag == cdav.SupportedCalendarComponentSet.tag:
         return [child.get("name") for child in elem if child.get("name")]
 

changes/+no-overwrite-if-none-match.changed.md

@@ -1 +1 @@
-`save(no_overwrite=True)` and `save(no_create=True)` (and `add_event(..., no_overwrite=True)` and friends) have improved handling on RFC-compliant servers: the server enforces the flag at the same URL in the same request as the write, so another client can no longer slip in between the check and the PUT there.  A refusal still raises `ConsistencyError`.
+`save(no_overwrite=True)` and `save(no_create=True)` (and `add_event(..., no_overwrite=True)` and friends) have improved handling on RFC-compliant servers: the server enforces the flag at the same URL in the same request as the write, so another client can no longer slip in between the check and the PUT there.  A refusal still raises `ConsistencyError`.  When the refusal is because another object (at another URL) holds the UID, it is the subclass `UidConflictConsistencyError`, whether the server refused the UID or caldav's own UID search found it (https://github.com/python-caldav/caldav/issues/806).

changes/795.fixed.md

@@ -0,0 +1 @@
+The `supported-report-set` and `current-user-privilege-set` properties from `propfind()` were the raw lxml element.  They are now lists of the report and privilege names in Clark notation, e.g. `["{urn:ietf:params:xml:ns:caldav}calendar-query", "{DAV:}sync-collection"]`.  Code that walked the returned element has to read the list instead.

changes/799.changed.md

@@ -0,0 +1 @@
+`DAVClient.password` is now the `str` that was passed in, as on `AsyncDAVClient`; it used to be encoded to UTF-8 `bytes`.  The return type of `Calendar.multiget()` is annotated as a list, or a coroutine of one for an async client, like `search()`.

changes/805.added.md

@@ -0,0 +1 @@
+`DAVError` and its subclasses have a new `status` attribute with the HTTP status code of the failed response (or `None`), so an `AuthorizationError` tells a 401 (bad or missing credentials) from a 403 (authenticated, but not allowed).

docs/design/TODO.md

@@ -39,3 +39,11 @@ A response whose `Content-Type` says XML but whose body is not XML (an HTML erro
 ## A waiting session-lock claim blocks other threads
 
 `TestServer.claim_server()` in `tests/test_servers/base.py` polls for the flock (up to `PYTHON_CALDAV_TEST_SERVER_LOCK_TIMEOUT`, 30 minutes by default) while holding the module-wide `_session_locks_mutex`.  A thread waiting for one busy server therefore blocks every other thread claiming a different server, which contradicts "runs against different servers proceed in parallel".  Harmless for single-threaded pytest.  Fix: hold the mutex only around the `_session_locks` checks and inserts, with a per-name lock against double claims.  Found in the review of https://github.com/python-caldav/caldav/pull/776.
+
+## Config errors from extract_conn_params_from_section name no section
+
+`extract_conn_params_from_section()` in `caldav/config.py` passes the fixed name `"section"` to calendaring-config's `section_connection()`, so a failing `password_command` reads "password_command of section section exited with status 3" for plann users.  Fix: an optional `section_name` parameter, passed on, and plann giving its section names.  Found in the review of plann's switch to caldav 3.6, see https://github.com/python-caldav/caldav/issues/786.
+
+## calendar_multiget() fails on an async client
+
+`Calendar.calendar_multiget()` in `caldav/collection.py` does `list(self.multiget(...))`, but `multiget()` returns a coroutine for an async client, so the call raises `TypeError`.  `calendar_multiget()` has no return annotation, so a type checker does not catch it.  Fix: return `self.multiget(...)` for an async client (or annotate it like `multiget()`), with an async unit test.  Found in the review of the fix for https://github.com/python-caldav/caldav/issues/799.

tests/test_caldav_unit.py

@@ -3616,6 +3616,7 @@ def test_raise_if_rate_limited_429_with_header(self):
             error.raise_if_rate_limited(429, "http://x/", "30")
         assert exc_info.value.retry_after == "30"
         assert exc_info.value.retry_after_seconds == 30.0
+        assert exc_info.value.status == 429
 
     def test_raise_if_rate_limited_503_with_header(self):
         with pytest.raises(error.RateLimitError):
@@ -3996,6 +3997,34 @@ def test_authorization_error_defaults(self):
         e = error.AuthorizationError(url="/", reason="Forbidden")
         assert e.preconditions == []
         assert e.body is None
+        assert e.status is None
+
+    @pytest.mark.parametrize("status", [401, 403])
+    @mock.patch("caldav.davclient.requests.Session.request")
+    def test_status_is_kept(self, mocked, status):
+        """Issue https://github.com/python-caldav/caldav/issues/805: 401 (bad
+        or missing credentials) and 403 (not allowed) are both an
+        AuthorizationError, told apart by ``status``."""
+        mocked.return_value = self._make_response(status)
+        client = DAVClient(url="https://cal.example.com/", auth=mock.MagicMock())
+        with pytest.raises(error.AuthorizationError) as exc_info:
+            client.request("/")
+        assert exc_info.value.status == status
+
+    def test_status_from_httpx_response(self):
+        """An httpx response names the reason ``reason_phrase``; the status
+        is ``status_code`` in all the HTTP libraries."""
+        client = DAVClient(url="https://cal.example.com/")
+        r = mock.Mock(spec=["status_code", "reason_phrase", "content"])
+        r.status_code, r.reason_phrase, r.content = 401, "Unauthorized", b""
+        with pytest.raises(error.AuthorizationError) as exc_info:
+            client._raise_authorization_error("https://cal.example.com/", r)
+        assert exc_info.value.status == 401
+        assert exc_info.value.reason == "Unauthorized"
+
+    def test_error_from_response_keeps_status(self):
+        r = mock.Mock(status=404, reason="Not Found", raw="", _raw=b"")
+        assert error.error_from_response(error.NotFoundError, r).status == 404
 
     def _calendar(self, report_error, sync_token_support=None):
         client = DAVClient(url="https://cal.example.com/")
@@ -5592,7 +5621,7 @@ def test_url_with_user_but_no_password_does_not_crash(self):
         """DAVClient(url='https://user@host/', password='p') must not raise TypeError."""
         client = DAVClient(url="https://user@cal.example.com/dav/", password="secret")
         assert client.username == "user"
-        assert client.password == b"secret"
+        assert client.password == "secret"
 
     def test_explicit_kwargs_take_precedence_over_url_credentials(self):
         """Explicit username/password kwargs must override credentials embedded in the URL."""
@@ -5602,7 +5631,7 @@ def test_explicit_kwargs_take_precedence_over_url_credentials(self):
             password="kwargpass",
         )
         assert client.username == "kwarguser"
-        assert client.password == b"kwargpass"
+        assert client.password == "kwargpass"
 
     def test_explicit_username_does_not_borrow_url_password(self):
         """Gate finding F6: credentials are a pair, not two independent fields.
@@ -5624,7 +5653,7 @@ def test_explicit_password_still_pairs_with_the_url_username(self):
             password="s3cret",
         )
         assert client.username == "bob"
-        assert client.password == b"s3cret"
+        assert client.password == "s3cret"
 
 
 class TestRepeatedHrefCalendarData:
@@ -7898,6 +7927,7 @@ def test_uid_conflict_is_also_consistency_error(self, base, status) -> None:
             self._run(self._event(client).save(no_overwrite=True))
         assert isinstance(exc_info.value, error.ConsistencyError)
         assert exc_info.value.preconditions == ["{urn:ietf:params:xml:ns:caldav}no-uid-conflict"]
+        assert exc_info.value.status == status
         assert [m for m, _ in sent] == ["PUT"]
 
     def test_403_uid_conflict_is_also_consistency_error(self, base) -> None:
@@ -7907,13 +7937,34 @@ def test_403_uid_conflict_is_also_consistency_error(self, base) -> None:
             reason="Forbidden",
             preconditions=[tag],
             body=UNIQUE_SCHEDULING_BODY,
+            status=403,
         )
         client, _ = self._client(base, features=self.NO_UID_CONFLICT, exc=exc)
         with pytest.raises(error.UidConflictError) as exc_info:
             self._run(self._event(client).save(no_overwrite=True))
         assert isinstance(exc_info.value, error.ConsistencyError)
         ## ... and still what earlier versions raised (issue #756)
         assert isinstance(exc_info.value, error.AuthorizationError)
+        assert exc_info.value.status == 403
+
+    def test_uid_found_elsewhere_by_search_is_uid_conflict(self, base) -> None:
+        """Issue https://github.com/python-caldav/caldav/issues/806: where
+        caldav searches for the UID itself, a hit under another URL is the
+        same UidConflictConsistencyError the server's precondition gives."""
+        client, sent = self._client(base, report=EV1_MULTISTATUS)
+        with pytest.raises(error.UidConflictConsistencyError):
+            self._run(self._event(client).save(no_overwrite=True))
+        assert [m for m, _ in sent] == ["REPORT"]
+
+    def test_uid_found_at_own_url_by_search_is_plain_consistency_error(self, base) -> None:
+        """The object found is the one at the URL to save to: the name is
+        taken, as with a 412 to If-None-Match: *, not a UID conflict."""
+        client, sent = self._client(base, report=EV1_MULTISTATUS)
+        event = self._event(client)
+        event.url = URL(self.URL + "ev1.ics")
+        with pytest.raises(error.ConsistencyError) as exc_info:
+            self._run(event.save(no_overwrite=True))
+        assert not isinstance(exc_info.value, error.UidConflictError)
 
     def test_server_ignoring_if_none_match_keeps_search(self, base) -> None:
         """Where If-None-Match: * is known to overwrite (Zimbra, SOGo) the

tests/test_protocol.py

@@ -329,6 +329,61 @@ def test_parse_sync_collection_generic_error(self):
         assert result.deleted == ["/cal/gone.ics"]
         assert result.changed == []
 
+    def test_parse_nested_properties(self):
+        """Issue https://github.com/python-caldav/caldav/issues/795: properties
+        nested deeper than one level gave the raw lxml element."""
+        xml = b"""<?xml version="1.0"?>
+        <D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
+            <D:response>
+                <D:href>/calendars/user/calendar/</D:href>
+                <D:propstat>
+                    <D:prop>
+                        <D:supported-report-set>
+                            <D:supported-report><D:report><C:calendar-query/></D:report></D:supported-report>
+                            <D:supported-report><D:report><D:sync-collection/></D:report></D:supported-report>
+                        </D:supported-report-set>
+                        <D:current-user-privilege-set>
+                            <D:privilege><D:read/></D:privilege>
+                            <D:privilege><D:write-content/></D:privilege>
+                        </D:current-user-privilege-set>
+                    </D:prop>
+                    <D:status>HTTP/1.1 200 OK</D:status>
+                </D:propstat>
+            </D:response>
+        </D:multistatus>"""
+
+        props = DAVResponse.from_bytes(xml).parse_propfind()[0].properties
+
+        assert props["{DAV:}supported-report-set"] == [
+            "{urn:ietf:params:xml:ns:caldav}calendar-query",
+            "{DAV:}sync-collection",
+        ]
+        assert props["{DAV:}current-user-privilege-set"] == [
+            "{DAV:}read",
+            "{DAV:}write-content",
+        ]
+
+    def test_parse_empty_nested_properties(self):
+        """No reports, no privileges: an empty list, not None."""
+        xml = b"""<?xml version="1.0"?>
+        <D:multistatus xmlns:D="DAV:">
+            <D:response>
+                <D:href>/calendars/user/calendar/</D:href>
+                <D:propstat>
+                    <D:prop>
+                        <D:supported-report-set/>
+                        <D:current-user-privilege-set/>
+                    </D:prop>
+                    <D:status>HTTP/1.1 200 OK</D:status>
+                </D:propstat>
+            </D:response>
+        </D:multistatus>"""
+
+        props = DAVResponse.from_bytes(xml).parse_propfind()[0].properties
+
+        assert props["{DAV:}supported-report-set"] == []
+        assert props["{DAV:}current-user-privilege-set"] == []
+
     def test_parse_complex_properties(self):
         """Parse complex properties like supported-calendar-component-set."""
         xml = b"""<?xml version="1.0"?>