Allow multiple <D:set> and <D:remove> elements and consider order

radicale/app/mkcalendar.py

@@ -43,6 +43,7 @@ class ApplicationMkcalendarMixin:
             return httputils.REQUEST_TIMEOUT
         # Prepare before locking
         props = xmlutils.props_from_request(xml_content)
+        props = {k: v for k, v in props.items() if v is not None}
         props["tag"] = "VCALENDAR"
         # TODO: use this?
         # timezone = props.get("C:calendar-timezone")

radicale/app/mkcol.py

@@ -44,6 +44,7 @@ class ApplicationMkcolMixin:
             return httputils.REQUEST_TIMEOUT
         # Prepare before locking
         props = xmlutils.props_from_request(xml_content)
+        props = {k: v for k, v in props.items() if v is not None}
         try:
             radicale_item.check_and_sanitize_props(props)
         except ValueError as e:

radicale/app/proppatch.py

@@ -17,6 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with Radicale.  If not, see <http://www.gnu.org/licenses/>.
 
+import contextlib
 import socket
 from http import client
 from xml.etree import ElementTree as ET
@@ -33,18 +34,12 @@ def xml_proppatch(base_prefix, path, xml_request, collection):
     Read rfc4918-9.2 for info.
 
     """
-    props_to_set = xmlutils.props_from_request(xml_request, actions=("set",))
-    props_to_remove = xmlutils.props_from_request(xml_request,
-                                                  actions=("remove",))
-
     multistatus = ET.Element(xmlutils.make_clark("D:multistatus"))
     response = ET.Element(xmlutils.make_clark("D:response"))
     multistatus.append(response)
-
     href = ET.Element(xmlutils.make_clark("D:href"))
     href.text = xmlutils.make_href(base_prefix, path)
     response.append(href)
-
     # Create D:propstat element for props with status 200 OK
     propstat = ET.Element(xmlutils.make_clark("D:propstat"))
     status = ET.Element(xmlutils.make_clark("D:status"))
@@ -55,14 +50,12 @@ def xml_proppatch(base_prefix, path, xml_request, collection):
     response.append(propstat)
 
     new_props = collection.get_meta()
-    for short_name, value in props_to_set.items():
-        new_props[short_name] = value
-        props_ok.append(ET.Element(xmlutils.make_clark(short_name)))
-    for short_name in props_to_remove:
-        try:
-            del new_props[short_name]
-        except KeyError:
-            pass
+    for short_name, value in xmlutils.props_from_request(xml_request).items():
+        if value is None:
+            with contextlib.suppress(KeyError):
+                del new_props[short_name]
+        else:
+            new_props[short_name] = value
         props_ok.append(ET.Element(xmlutils.make_clark(short_name)))
     radicale_item.check_and_sanitize_props(new_props)
     collection.set_meta(new_props)

radicale/xmlutils.py

@@ -146,36 +146,46 @@ def get_content_type(item, encoding):
     return content_type
 
 
-def props_from_request(xml_request, actions=("set", "remove")):
-    """Return a list of properties as a dictionary."""
+def props_from_request(xml_request):
+    """Return a list of properties as a dictionary.
+
+    Properties that should be removed are set to `None`.
+
+    """
     result = OrderedDict()
     if xml_request is None:
         return result
 
-    for action in actions:
-        action_element = xml_request.find(make_clark("D:%s" % action))
-        if action_element is not None:
-            break
-    else:
-        action_element = xml_request
-
-    prop_element = action_element.find(make_clark("D:prop"))
-    if prop_element is not None:
-        for prop in prop_element:
-            if prop.tag == make_clark("D:resourcetype"):
+    # Requests can contain multipe <D:set> and <D:remove> elements.
+    # Each of these elements must contain exactly one <D:prop> element which
+    # can contain multpile properties.
+    # The order of the elements in the document must be respected.
+    props = []
+    for element in xml_request:
+        if element.tag in (make_clark("D:set"), make_clark("D:remove")):
+            for prop in element.findall("./%s/*" % make_clark("D:prop")):
+                props.append((element.tag == make_clark("D:set"), prop))
+    for is_set, prop in props:
+        key = make_human_tag(prop.tag)
+        value = None
+        if prop.tag == make_clark("D:resourcetype"):
+            key = "tag"
+            if is_set:
                 for resource_type in prop:
                     if resource_type.tag == make_clark("C:calendar"):
-                        result["tag"] = "VCALENDAR"
+                        value = "VCALENDAR"
                         break
                     if resource_type.tag == make_clark("CR:addressbook"):
-                        result["tag"] = "VADDRESSBOOK"
+                        value = "VADDRESSBOOK"
                         break
-            elif prop.tag == make_clark("C:supported-calendar-component-set"):
-                result[make_human_tag(prop.tag)] = ",".join(
-                    supported_comp.attrib["name"]
-                    for supported_comp in prop
+        elif prop.tag == make_clark("C:supported-calendar-component-set"):
+            if is_set:
+                value = ",".join(
+                    supported_comp.attrib["name"] for supported_comp in prop
                     if supported_comp.tag == make_clark("C:comp"))
-            else:
-                result[make_human_tag(prop.tag)] = prop.text
+        elif is_set:
+            value = prop.text or ""
+        result[key] = value
+        result.move_to_end(key)
 
     return result