Merge pull request #2122 from pbiering/fix-allprop-bday-regression

Fix allprop bday regression

CHANGELOG.md

@@ -3,6 +3,8 @@
 ## 3.7.3.dev
 * Extension: expose RADICALE:version for authenticated users via PROPFIND
 * Fix: sharing: GET request on single item with bday conversion
+* Fix: sharing: PROPFIND response on single item with bday conversion
+* Fix: sharing: PROPFIND response related to C:supported-calendar-component-set
 
 ## 3.7.2
 * Fix: broken storage/mtime granularity detection on vfat

radicale/app/propfind.py

@@ -158,7 +158,7 @@ def xml_propfind_response(
         # backmap
         if uri.startswith(share['PathMapped']):
             uri = str(share['PathOrToken']) + uri.removeprefix(share['PathMapped'])
-        if share_bday_automap and not uri.endswith("/"):
+        if share_bday_automap and uri.endswith(".vcf"):
             uri = uri.rstrip(".vcf") + ".ics"
 
     href.text = xmlutils.make_href(base_prefix, uri)
@@ -168,7 +168,7 @@ def xml_propfind_response(
         props = []
         # Should list all properties that can be retrieved by the code below
         props.append(xmlutils.make_clark("D:principal-collection-set"))
-        if user:
+        if user and is_collection:
             props.append(xmlutils.make_clark("RADICALE:version"))
         props.append(xmlutils.make_clark("D:current-user-principal"))
         props.append(xmlutils.make_clark("D:current-user-privilege-set"))
@@ -226,7 +226,17 @@ def xml_propfind_response(
         is404 = False
         if tag == xmlutils.make_clark("D:getetag"):
             if not is_collection or is_leaf:
-                element.text = item.etag
+                if isinstance(item, storage.BaseCollection):
+                    element.text = item.etag
+                else:
+                    if share_bday_automap:
+                        item_converted = item.convert_vcf_to_ics()
+                        if item_converted:
+                            element.text = item_converted.etag
+                        else:
+                            is404 = True
+                    else:
+                        element.text = item.etag
             else:
                 is404 = True
         elif tag == xmlutils.make_clark("D:getlastmodified"):
@@ -255,12 +265,14 @@ def xml_propfind_response(
         elif tag == xmlutils.make_clark("C:supported-calendar-component-set"):
             human_tag = xmlutils.make_human_tag(tag)
             if is_collection and is_leaf:
-                components_text = collection.get_meta(human_tag)
+                components = []
-                if components_text:
+                if collection.tag == "VCALENDAR":
-                    components = components_text.split(",")
+                    components_text = collection.get_meta(human_tag)
-                else:
+                    if components_text:
-                    components = ["VTODO", "VEVENT", "VJOURNAL"]
+                        components = components_text.split(",")
-                if share_bday_automap:
+                    else:
+                        components = ["VTODO", "VEVENT", "VJOURNAL"]
+                elif collection.tag == "VADDRESSBOOK" and share_bday_automap:
                     # enforce VEVENT-only
                     components = ["VEVENT"]
                 for component in components:
@@ -360,15 +372,23 @@ def xml_propfind_response(
                 element.append(supported_report)
         elif tag == xmlutils.make_clark("D:getcontentlength"):
             if not is_collection or is_leaf:
-                if collection.tag == "VADDRESSBOOK" and share_bday_automap and isinstance(item, storage.BaseCollection):
+                if collection.tag == "VADDRESSBOOK" and share_bday_automap:
-                    logger.trace("PROPFIND/xml_propfind_response/getcontentlength: start bday automap handling")
+                    if isinstance(item, storage.BaseCollection):
-                    length = 0
+                        logger.trace("PROPFIND/xml_propfind_response/getcontentlength: start bday automap handling for collection")
-                    for entry in item.get_all():
+                        length = 0
-                        item_ics = entry.convert_vcf_to_ics()
+                        for entry in item.get_all():
-                        if item_ics is None:
+                            item_ics = entry.convert_vcf_to_ics()
-                            continue
+                            if item_ics is None:
-                        length += len(item_ics.vobject_item.serialize().encode(encoding))
+                                continue
-                    element.text = str(length)
+                            length += len(item_ics.vobject_item.serialize().encode(encoding))
+                        element.text = str(length)
+                    else:
+                        logger.trace("PROPFIND/xml_propfind_response/getcontentlength: start bday automap handling for single item")
+                        item_converted = item.convert_vcf_to_ics()
+                        if item_converted is not None:
+                            element.text = str(len(item_converted.serialize()))
+                        else:
+                            is404 = True
                 else:
                     element.text = str(len(item.serialize().encode(encoding)))
             else:
@@ -569,6 +589,7 @@ class ApplicationPartPropfind(ApplicationBase):
                 user = share['Owner']
                 permissions_filter = share['Permissions']
                 shares[share['PathOrToken']] = share
+                logger.trace("PROPFIND/shares: add mapping: PathOrToken=%r PathMapped=%r", share['PathOrToken'], share['PathMapped'])
         access = Access(self._rights, user, path, permissions_filter)
         if not access.check("r"):
             return httputils.NOT_ALLOWED
@@ -594,11 +615,17 @@ class ApplicationPartPropfind(ApplicationBase):
                 return httputils.NOT_ALLOWED
             # put item back
             items_iter = itertools.chain([item], items_iter)
-            for item, permission, raw_permissions in list(self._collect_allowed_items(items_iter, user)):
+            item_list = list(self._collect_allowed_items(items_iter, user))
+            len_item_list = len(item_list)
+            for item, permission, raw_permissions in item_list:
                 if self._sharing._enabled and share:
                     if share['Conversion'] == "bday" and not isinstance(item, storage.BaseCollection):
                         if not item.convert_vcf_to_ics():
-                            continue
+                            if len_item_list == 1:
+                                # only dedicated item requested
+                                return httputils.NOT_FOUND
+                            else:
+                                continue
                     allowed_items.append((item, permission, raw_permissions, share['Conversion']))
                 else:
                     allowed_items.append((item, permission, raw_permissions, None))

radicale/sharing/__init__.py

@@ -500,7 +500,9 @@ class BaseSharing:
                 if result['Conversion'] == "bday" and result['PathMapped'].endswith(".ics"):
                     result['PathMapped'] = result['PathMapped'].removesuffix(".ics") + ".vcf"
 
-                logger.info("sharing/%s: resolved path %r->%r, user %r->%r, Permissions=%r Conversion=%r", "map", path, result['PathMapped'], user, result['Owner'], result['Permissions'], result['Conversion'])
+                result['PathOrToken'] = path
+
+                logger.info("sharing/%s: resolved path %r->%r, user %r->%r, Permissions=%r Conversion=%r", "map", result['PathOrToken'], result['PathMapped'], user, result['Owner'], result['Permissions'], result['Conversion'])
                 return result
 
             return None

radicale/tests/test_base.py

@@ -905,7 +905,8 @@ permissions: RrWw""")
         status, prop = response["D:getetag"]
         assert status == 200 and not prop.text
 
-    def test_propfind_allprop(self) -> None:
+    def test_propfind_vcalendar_allprop(self) -> None:
+        self.configure({"logging": {"limit_content": 5000}})
         self.mkcalendar("/calendar.ics/")
         event = get_file_content("event1.ics")
         self.put("/calendar.ics/event.ics", event)
@@ -916,12 +917,57 @@ permissions: RrWw""")
         status, prop = response["D:sync-token"]
         assert status == 200 and prop.text
         assert "C:max-resource-size" not in response
+        assert "C:supported-calendar-component-set" in response
+        assert "CR:supported-address-data" not in response
+        assert "D:resourcetype" in response
+        status, resourcetype = response["D:resourcetype"]
+        resourcetypes = resourcetype.find(xmlutils.make_clark("C:calendar"))
+        assert resourcetypes is not None
+        assert "{urn:ietf:params:xml:ns:caldav}calendar" in resourcetypes.tag
+        assert "{urn:ietf:params:xml:ns:carddav}addressbook" not in resourcetypes.tag
+        status, sup_cal_comp_set = response["C:supported-calendar-component-set"]
+        sup_cal_comp_sets = sup_cal_comp_set.findall(xmlutils.make_clark("C:comp"))
+        comp_attr = []
+        for comp in sup_cal_comp_sets:
+            comp_attr.append(comp.attrib)
+            logging.debug("comp: %r", comp.attrib)
+        assert {'name': 'VEVENT'} in comp_attr
+        assert {'name': 'VTODO'} in comp_attr
+        assert {'name': 'VJOURNAL'} in comp_attr
         _, responses = self.propfind("/calendar.ics/event.ics", propfind)
         response = responses["/calendar.ics/event.ics"]
         assert not isinstance(response, int)
         status, prop = response["D:getetag"]
         assert status == 200 and prop.text
         assert "C:max-resource-size" not in response
+        _, responses = self.propfind("/calendar.ics/event-not-exists.ics", propfind, check=404)
+
+    def test_propfind_vaddressbook_allprop(self) -> None:
+        self.configure({"logging": {"limit_content": 5000}})
+        self.create_addressbook("/addressbook.vcf/")
+        contact = get_file_content("contact1.vcf")
+        self.put("/addressbook.vcf/contact1.vcf", contact)
+        propfind = get_file_content("allprop.xml")
+        _, responses = self.propfind("/addressbook.vcf/", propfind)
+        response = responses["/addressbook.vcf/"]
+        assert not isinstance(response, int)
+        status, prop = response["D:sync-token"]
+        assert status == 200 and prop.text
+        assert "C:max-resource-size" not in response
+        assert "C:supported-calendar-component-set" not in response
+        assert "CR:supported-address-data" in response
+        assert "D:resourcetype" in response
+        status, resourcetype = response["D:resourcetype"]
+        resourcetypes = resourcetype.find(xmlutils.make_clark("CR:addressbook"))
+        assert resourcetypes is not None
+        assert "{urn:ietf:params:xml:ns:carddav}addressbook" in resourcetypes.tag
+        assert "{urn:ietf:params:xml:ns:caldav}calendar" not in resourcetypes.tag
+        _, responses = self.propfind("/addressbook.vcf/contact1.vcf", propfind)
+        response = responses["/addressbook.vcf/contact1.vcf"]
+        assert not isinstance(response, int)
+        status, prop = response["D:getetag"]
+        assert status == 200 and prop.text
+        assert "C:max-resource-size" not in response
 
     def test_propfind_nonexistent(self) -> None:
         """Read a property that does not exist."""

radicale/tests/test_sharing.py

@@ -86,14 +86,16 @@ class TestSharingApiSanity(BaseTest):
         _, headers, answer = self._sharing_api(sharing_type, action, check, login, data, content_type, accept, prefix=prefix)
         return _, headers, answer
 
-    def _propfind_allprop(self, path: str, login: str = "", prefix: Union[str, None] = None) -> dict:
+    def _propfind_allprop(self, path: str, login: str = "", prefix: Union[str, None] = None, check=207) -> dict:
         propfind_allprop = get_file_content("allprop.xml")
         if prefix is not None:
             path = prefix + path
-            _, responses = self.propfind(path=path, data=propfind_allprop, login=login, x_forwarded_for="127.0.0.2")
+            _, responses = self.propfind(path=path, data=propfind_allprop, login=login, x_forwarded_for="127.0.0.2", check=check)
         else:
-            _, responses = self.propfind(path=path, data=propfind_allprop, login=login)
+            _, responses = self.propfind(path=path, data=propfind_allprop, login=login, check=check)
         logging.info("response: %r", responses)
+        if check != 207:
+            return {}
         response = responses[path]
         assert not isinstance(response, int)
         return response
@@ -4772,8 +4774,8 @@ permissions: RrWw""")
             assert row['ShareType'] == "map"
             assert row['Conversion'] == "bday"
 
-            # check PROPFIND item as user
+            # check PROPFIND collection as user
-            logging.info("\n*** PROPFIND item as user -> calendar")
+            logging.info("\n*** PROPFIND collection as user -> calendar")
             response = self._propfind_allprop(path_shared_r, login="user:userpw")
             logging.debug("response: %r", response)
             assert "CR:supported-address-data" not in response
@@ -4781,8 +4783,18 @@ permissions: RrWw""")
             assert "C:supported-calendar-component-set" in response
             assert "D:current-user-privilege-set" in response
 
-            # check PROPFIND/privileges item as user
+            logging.info("\n*** PROPFIND item as user -> calendar")
-            logging.info("\n*** PROPFIND/privileges item as user -> calendar")
+            response = self._propfind_allprop(path_shared_r + "contact2-with-bday.ics", login="user:userpw")
+            logging.debug("response: %r", response)
+            assert "CR:supported-address-data" not in response
+            assert "D:sync-token" not in response
+            assert "D:current-user-privilege-set" in response
+
+            logging.info("\n*** PROPFIND item as user with unsupported bday conversion -> not found")
+            response = self._propfind_allprop(path_shared_r + "contact1.ics", login="user:userpw", check=404)
+
+            # check PROPFIND/privileges collection as user
+            logging.info("\n*** PROPFIND/privileges collection as user -> calendar")
             privileges_list = self._propfind_privileges(path_shared_r, login="user:userpw")
             assert "D:read" in privileges_list
             assert "D:write-content" not in privileges_list
@@ -5168,6 +5180,12 @@ permissions: RrWw""")
             logging.debug("response %r: %r", path_mapped, response)
             assert "C:supported-calendar-component-set" in response
             assert path_shared not in responses
+            assert "D:resourcetype" in response
+            status, resourcetype = response["D:resourcetype"]
+            resourcetypes = resourcetype.find(xmlutils.make_clark("CR:addressbook"))
+            assert resourcetypes is not None
+            assert "{urn:ietf:params:xml:ns:carddav}addressbook" in resourcetypes.tag
+            assert "{urn:ietf:params:xml:ns:caldav}calendar" not in resourcetypes.tag
 
             # enable + unhide
             logging.info("\n*** enable+unhide bday owner to itself -> ok")
@@ -5196,11 +5214,25 @@ permissions: RrWw""")
   </prop>
 </propfind>""", login="owner:ownerpw", HTTP_DEPTH="1")
             # logging.debug("responses: %r", responses)
-            response = responses[path_mapped]
+            response = responses[path_shared]
             assert not isinstance(response, int)
             logging.debug("response %r: %r", path_mapped, response)
             assert "C:supported-calendar-component-set" in response
             assert path_shared in responses
+            status, resourcetype = response["D:resourcetype"]
+            resourcetypes = resourcetype.find(xmlutils.make_clark("C:calendar"))
+            assert resourcetypes is not None
+            assert "{urn:ietf:params:xml:ns:carddav}addressbook" not in resourcetypes.tag
+            assert "{urn:ietf:params:xml:ns:caldav}calendar" in resourcetypes.tag
+            status, sup_cal_comp_set = response["C:supported-calendar-component-set"]
+            sup_cal_comp_sets = sup_cal_comp_set.findall(xmlutils.make_clark("C:comp"))
+            comp_attr = []
+            for comp in sup_cal_comp_sets:
+                comp_attr.append(comp.attrib)
+                logging.debug("comp: %r", comp.attrib)
+            assert {'name': 'VEVENT'} in comp_attr
+            assert {'name': 'VTODO'} not in comp_attr
+            assert {'name': 'VJOURNAL'} not in comp_attr
 
             # check PROPFIND item as owner
             logging.info("\n*** PROPFIND item as owner -> calendar")