From 6281e7a23719a326396fa14b47d398b3721aeb3e Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Fri, 23 Jul 2021 20:40:23 +0200 Subject: [PATCH 1/5] Radicale now passes for this test --- tests/storage/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index c8dbe5a..6a5cadf 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -282,8 +282,6 @@ class StorageTests: async def test_specialchars( self, monkeypatch, requires_collections, get_storage_args, get_item ): - if getattr(self, "dav_server", "") == "radicale": - pytest.skip("Radicale is fundamentally broken.") if getattr(self, "dav_server", "") in ("icloud", "fastmail"): pytest.skip("iCloud and FastMail reject this name.") From 0650cc3bc2490ad096ffee3b3af4f2121fa29853 Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Fri, 23 Jul 2021 20:43:07 +0200 Subject: [PATCH 2/5] Add test for #918 We're doing something wrong with UID/href quoting/unquoting, but I've yet to figure out what. --- tests/storage/__init__.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 6a5cadf..c28da83 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -310,6 +310,26 @@ class StorageTests: if self.storage_class.storage_name.endswith("dav"): assert urlquote(uid, "/@:") in href + @pytest.mark.asyncio + async def test_newline_in_uid( + self, monkeypatch, requires_collections, get_storage_args, get_item + ): + monkeypatch.setattr("vdirsyncer.utils.generate_href", lambda x: x) + + uid = "20210609T084907Z-@synaps-web-54fddfdf7-7kcfm%0A.ics" + + s = self.storage_class(**await get_storage_args()) + item = get_item(uid=uid) + + href, etag = await s.upload(item) + item2, etag2 = await s.get(href) + if etag is not None: + assert etag2 == etag + assert_item_equals(item2, item) + + ((_, etag3),) = await aiostream.stream.list(s.list()) + assert etag2 == etag3 + @pytest.mark.asyncio async def test_empty_metadata(self, requires_metadata, s): if getattr(self, "dav_server", ""): From 889183ec89ba807177ee7a276d6f9113df09e7ac Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Sun, 25 Jul 2021 13:25:16 +0200 Subject: [PATCH 3/5] I think this makes sense --- tests/storage/dav/test_main.py | 11 +++++++++++ vdirsyncer/storage/dav.py | 18 +++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/storage/dav/test_main.py b/tests/storage/dav/test_main.py index 7904ef5..196094b 100644 --- a/tests/storage/dav/test_main.py +++ b/tests/storage/dav/test_main.py @@ -2,6 +2,7 @@ import pytest from vdirsyncer.storage.dav import _BAD_XML_CHARS from vdirsyncer.storage.dav import _merge_xml +from vdirsyncer.storage.dav import _normalize_href from vdirsyncer.storage.dav import _parse_xml @@ -44,3 +45,13 @@ def test_xml_specialchars(char): if char in _BAD_XML_CHARS: assert x.text == "yes\nhello" + + +@pytest.mark.parametrize( + "href", + [ + "/dav/calendars/user/testuser/123/UID%253A20210609T084907Z-@synaps-web-54fddfdf7-7kcfm%250A.ics", # noqa: E501 + ], +) +def test_normalize_href(href): + assert href == _normalize_href("https://example.com", href) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index fda9446..c4d66cb 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -65,8 +65,7 @@ async def _assert_multistatus_success(r): def _normalize_href(base, href): - """Normalize the href to be a path only relative to hostname and - schema.""" + """Normalize the href to be a path only relative to hostname and schema.""" orig_href = href if not href: raise ValueError(href) @@ -74,17 +73,10 @@ def _normalize_href(base, href): x = urlparse.urljoin(base, href) x = urlparse.urlsplit(x).path - # Encoding issues: - # - https://github.com/owncloud/contacts/issues/581 - # - https://github.com/Kozea/Radicale/issues/298 - old_x = None - while old_x is None or x != old_x: - if _contains_quoted_reserved_chars(x): - break - old_x = x - x = urlparse.unquote(x) - - x = urlparse.quote(x, "/@%:") + # We unquote and quote again, but want to make sure we + # keep around the "@" character. + x = urlparse.unquote(x) + x = urlparse.quote(x, "/@") if orig_href == x: dav_logger.debug(f"Already normalized: {x!r}") From 955f434d9d00dc19a409fc81464af220bd8e437b Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Wed, 18 Aug 2021 18:41:00 +0200 Subject: [PATCH 4/5] Also test syncing with an href with a colon Since this was the actual problematic character. --- tests/storage/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index c28da83..699c75e 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -316,7 +316,7 @@ class StorageTests: ): monkeypatch.setattr("vdirsyncer.utils.generate_href", lambda x: x) - uid = "20210609T084907Z-@synaps-web-54fddfdf7-7kcfm%0A.ics" + uid = "UID:20210609T084907Z-@synaps-web-54fddfdf7-7kcfm%0A.ics" s = self.storage_class(**await get_storage_args()) item = get_item(uid=uid) From 48747463ed5b47d6479cf986f6ffc367455289ab Mon Sep 17 00:00:00 2001 From: Hugo Osvaldo Barrera Date: Thu, 19 Aug 2021 20:40:47 +0200 Subject: [PATCH 5/5] Remove unused code --- vdirsyncer/storage/dav.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/vdirsyncer/storage/dav.py b/vdirsyncer/storage/dav.py index c4d66cb..6ea87cd 100644 --- a/vdirsyncer/storage/dav.py +++ b/vdirsyncer/storage/dav.py @@ -29,25 +29,6 @@ dav_logger = logging.getLogger(__name__) CALDAV_DT_FORMAT = "%Y%m%dT%H%M%SZ" -def _generate_path_reserved_chars(): - for x in "/?#[]!$&'()*+,;": - x = urlparse.quote(x, "") - yield x.upper() - yield x.lower() - - -_path_reserved_chars = frozenset(_generate_path_reserved_chars()) -del _generate_path_reserved_chars - - -def _contains_quoted_reserved_chars(x): - for y in _path_reserved_chars: - if y in x: - dav_logger.debug(f"Unsafe character: {y!r}") - return True - return False - - async def _assert_multistatus_success(r): # Xandikos returns a multistatus on PUT. try: