From 7873edfa5520be65cdbb299b9b94f7be8c810669 Mon Sep 17 00:00:00 2001 From: Justin Kondratenko Date: Thu, 6 Aug 2026 15:10:15 +0000 Subject: [PATCH] fix: encode spaces in caldav URLs --- nextcloud_mcp_server/client/calendar.py | 46 ++++++++++++++- tests/unit/client/test_calendar.py | 59 ++++++++++++++++++- .../client/test_dav_principal_discovery.py | 57 ++++++++++++++++++ 3 files changed, 158 insertions(+), 4 deletions(-) diff --git a/nextcloud_mcp_server/client/calendar.py b/nextcloud_mcp_server/client/calendar.py index 6744b44a1..5eff2bee9 100644 --- a/nextcloud_mcp_server/client/calendar.py +++ b/nextcloud_mcp_server/client/calendar.py @@ -6,7 +6,7 @@ import re import uuid from typing import Any -from urllib.parse import unquote, urlsplit, urlunsplit +from urllib.parse import quote, unquote, urlsplit, urlunsplit from zoneinfo import ZoneInfo, ZoneInfoNotFoundError import anyio @@ -15,6 +15,7 @@ from caldav.aio import AsyncCalendar, AsyncDAVClient, AsyncEvent from caldav.elements import cdav, dav from caldav.lib import error as caldav_error +from caldav.lib import url as caldav_url from icalendar import Alarm, Calendar, Timezone, vDDDTypes, vRecur from icalendar import Event as ICalEvent from icalendar import Todo as ICalTodo @@ -22,6 +23,11 @@ from ..config import get_nextcloud_ssl_verify +# Characters allowed unencoded in a CalDAV URL path (RFC 3986 pchar plus +# "/" and "%"). "Nextcloud User"-style UIDs contain spaces, which must be +# percent-encoded in hrefs. +_DAV_SAFE = "/%:@&=+$,;~*()!'-._" + logger = logging.getLogger(__name__) @@ -240,6 +246,37 @@ def _as_utc_datetime(value: dt.date) -> dt.datetime: return dt.datetime(value.year, value.month, value.day, tzinfo=dt.UTC) +def _patch_caldav_url_join() -> None: + """Encode spaces in caldav URL joins. + + Nextcloud hrefs embed the raw username. With a space in the UID, + caldav's ``URL.join()`` keeps it and ``DAVObject`` rejects the URL. + Patch ``join()`` to percent-encode the path. ``_DAV_SAFE`` includes + ``%`` so ``quote()`` is idempotent on already-encoded paths. + """ + if getattr(caldav_url.URL, "_patched_url_join", False): + return + + _orig_join = caldav_url.URL.join + + def _join(self, path): + joined = _orig_join(self, path) + parsed = joined.url_parsed + if parsed is not None and " " in (parsed.path or ""): + cls = type(parsed) + parts = list(parsed) + parts[2] = quote(parsed.path, safe=_DAV_SAFE) + joined.url_parsed = cls(*parts) + joined.url_raw = None + return joined + + caldav_url.URL.join = _join + caldav_url.URL._patched_url_join = True + + +_patch_caldav_url_join() + + class CalendarClient: """Client for Nextcloud CalDAV calendar and task operations.""" @@ -303,7 +340,7 @@ def __init__( headers={"X-NC-CalDAV-Webcal-Caching": "On"}, **auth_kwargs, ) - self._calendar_home_url = f"{base_url}/remote.php/dav/calendars/{username}/" + self._calendar_home_url = f"{base_url}/remote.php/dav/calendars/{quote(username, safe=_DAV_SAFE)}/" self._principal_resolved = False def _calendar_home_url_from_home_set(self, home_set: Any) -> str | None: @@ -328,6 +365,9 @@ def _calendar_home_url_from_home_set(self, home_set: Any) -> str | None: # (issue #1007). origin = urlsplit(self.base_url) home_url = urlunsplit((origin.scheme, origin.netloc, home_url, "", "")) + if " " in home_url: + # Nextcloud hrefs embed the raw username; encode spaces. + home_url = quote(home_url, safe=_DAV_SAFE) if not home_url.endswith("/"): home_url = f"{home_url}/" return home_url @@ -384,7 +424,7 @@ async def _ensure_calendar_home(self) -> None: principal_id = unquote(str(principal_url).rstrip("/").split("/")[-1]) if principal_id: self._calendar_home_url = ( - f"{self.base_url}/remote.php/dav/calendars/{principal_id}/" + f"{self.base_url}/remote.php/dav/calendars/{quote(principal_id, safe=_DAV_SAFE)}/" ) self._principal_resolved = True except (caldav_error.DAVError, httpx.HTTPError, ValueError) as e: diff --git a/tests/unit/client/test_calendar.py b/tests/unit/client/test_calendar.py index 461ec739a..abfa23c70 100644 --- a/tests/unit/client/test_calendar.py +++ b/tests/unit/client/test_calendar.py @@ -128,7 +128,7 @@ def test_auth_username_used_for_credential_uid_for_fallback_path(mocker): assert client.username == "Ada Lovelace" assert ( client._calendar_home_url - == "https://cloud.example.org/remote.php/dav/calendars/Ada Lovelace/" + == "https://cloud.example.org/remote.php/dav/calendars/Ada%20Lovelace/" ) @@ -1147,3 +1147,60 @@ def test_reminder_model_rejects_incoherent_triggers(payload): with pytest.raises(ValidationError): Reminder(**payload) + + +def test_calendar_home_url_encodes_username_with_space(mocker): + """Constructor percent-encodes a username containing a space.""" + mocker.patch("nextcloud_mcp_server.client.calendar.AsyncDAVClient") + + from nextcloud_mcp_server.client.calendar import CalendarClient + + client = CalendarClient( + "https://cloud.example.org", "Nextcloud User", password="app-pw-1234" + ) + + assert client._calendar_home_url == ( + "https://cloud.example.org/remote.php/dav/calendars/Nextcloud%20User/" + ) + + +def test_caldav_url_join_patch_encodes_spaced_paths(mocker): + """URL.join() patch percent-encodes spaces in joined paths. + + Nextcloud hrefs embed the raw username; a spaced UID therefore + produces hrefs like ``.../calendars/Nextcloud User/``. caldav's + ``DAVObject`` rejects literal spaces, so the patch must encode them. + """ + mocker.patch("nextcloud_mcp_server.client.calendar.AsyncDAVClient") + + from caldav.lib.url import URL + + from nextcloud_mcp_server.client.calendar import _patch_caldav_url_join + + _patch_caldav_url_join() + + base = URL.objectify("https://cloud.example.org/remote.php/dav/") + joined = base.join("/remote.php/dav/calendars/Nextcloud User/personal/") + + assert " " not in str(joined) + assert str(joined).endswith( + "/remote.php/dav/calendars/Nextcloud%20User/personal/" + ) + + +def test_caldav_url_join_patch_is_idempotent_for_encoded_paths(mocker): + """Already-encoded paths are not double-encoded.""" + mocker.patch("nextcloud_mcp_server.client.calendar.AsyncDAVClient") + + from caldav.lib.url import URL + + from nextcloud_mcp_server.client.calendar import _patch_caldav_url_join + + _patch_caldav_url_join() + + base = URL.objectify("https://cloud.example.org/remote.php/dav/") + joined = base.join("/remote.php/dav/calendars/Nextcloud%20User/personal/") + + assert str(joined).endswith( + "/remote.php/dav/calendars/Nextcloud%20User/personal/" + ) diff --git a/tests/unit/client/test_dav_principal_discovery.py b/tests/unit/client/test_dav_principal_discovery.py index 7b1c59136..d85e37944 100644 --- a/tests/unit/client/test_dav_principal_discovery.py +++ b/tests/unit/client/test_dav_principal_discovery.py @@ -473,3 +473,60 @@ async def test_caldav_event_operations_use_discovered_home_url(mocker): mock_calendar.call_args.kwargs["url"] == "https://cloud.example.org/remote.php/dav/calendars/alice_1234/team/" ) + +async def test_caldav_discovery_failure_falls_back_to_encoded_username(mocker): + """Spaced username keeps an encoded fallback URL when discovery fails.""" + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + dav_client = mock_dav_client.return_value + dav_client.get_principal = mocker.AsyncMock( + side_effect=caldav_error.DAVError("temporary failure") + ) + dav_client.propfind = mocker.AsyncMock( + return_value=mocker.Mock(raw=_calendar_multistatus("Nextcloud User")) + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + client = CalendarClient( + "https://cloud.example.org", "Nextcloud User", password=_APP_PW + ) + + calendars = await client.list_calendars() + + assert [calendar["name"] for calendar in calendars] == ["personal"] + assert ( + dav_client.propfind.await_args.args[0] + == "https://cloud.example.org/remote.php/dav/calendars/Nextcloud%20User/" + ) + + +async def test_caldav_principal_fallback_encodes_principal_id(mocker): + """Principal-id fallback percent-encodes a spaced principal URL.""" + mock_dav_client = mocker.patch( + "nextcloud_mcp_server.client.calendar.AsyncDAVClient" + ) + dav_client = mock_dav_client.return_value + dav_client.get_principal = mocker.AsyncMock( + return_value=SimpleNamespace( + url="https://cloud.example.org/remote.php/dav/principals/users/Nextcloud User/" + ) + ) + dav_client.propfind = mocker.AsyncMock( + return_value=mocker.Mock(raw=_calendar_multistatus("Nextcloud User")) + ) + + from nextcloud_mcp_server.client.calendar import CalendarClient + + client = CalendarClient( + "https://cloud.example.org", "Nextcloud User", password=_APP_PW + ) + + calendars = await client.list_calendars() + + assert [calendar["name"] for calendar in calendars] == ["personal"] + assert ( + dav_client.propfind.await_args.args[0] + == "https://cloud.example.org/remote.php/dav/calendars/Nextcloud%20User/" + )