Merge remote-tracking branch 'upstream/master'

radicale/app/__init__.py

@@ -167,8 +167,8 @@ class Application(
         elif environ.get("REMOTE_ADDR"):
             remote_host = environ["REMOTE_ADDR"]
         if environ.get("HTTP_X_FORWARDED_FOR"):
-            remote_host = "%r (forwarded by %s)" % (
+            remote_host = "%s (forwarded for %r)" % (
-                environ["HTTP_X_FORWARDED_FOR"], remote_host)
+                remote_host, environ["HTTP_X_FORWARDED_FOR"])
         remote_useragent = ""
         if environ.get("HTTP_USER_AGENT"):
             remote_useragent = " using %r" % environ["HTTP_USER_AGENT"]
@@ -230,7 +230,8 @@ class Application(
         elif user:
             logger.info("Successful login: %r -> %r", login, user)
         elif login:
-            logger.info("Failed login attempt: %r", login)
+            logger.warning("Failed login attempt from %s: %r",
+                           remote_host, login)
             # Random delay to avoid timing oracles and bruteforce attacks
             delay = self.configuration.get("auth", "delay")
             if delay > 0:

radicale/app/mkcalendar.py

@@ -39,10 +39,11 @@ class ApplicationMkcalendarMixin:
                 "Bad MKCALENDAR request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             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

@@ -40,10 +40,11 @@ class ApplicationMkcolMixin:
                 "Bad MKCOL request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             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/propfind.py

@@ -40,18 +40,18 @@ def xml_propfind(base_prefix, path, xml_request, allowed_items, user,
     """
     # A client may choose not to submit a request body.  An empty PROPFIND
     # request body MUST be treated as if it were an 'allprop' request.
-    top_tag = (xml_request[0] if xml_request is not None else
+    top_element = (xml_request[0] if xml_request is not None else
-               ET.Element(xmlutils.make_clark("D:allprop")))
+                   ET.Element(xmlutils.make_clark("D:allprop")))
 
     props = ()
     allprop = False
     propname = False
-    if top_tag.tag == xmlutils.make_clark("D:allprop"):
+    if top_element.tag == xmlutils.make_clark("D:allprop"):
         allprop = True
-    elif top_tag.tag == xmlutils.make_clark("D:propname"):
+    elif top_element.tag == xmlutils.make_clark("D:propname"):
         propname = True
-    elif top_tag.tag == xmlutils.make_clark("D:prop"):
+    elif top_element.tag == xmlutils.make_clark("D:prop"):
-        props = [prop.tag for prop in top_tag]
+        props = [prop.tag for prop in top_element]
 
     if xmlutils.make_clark("D:current-user-principal") in props and not user:
         # Ask for authentication
@@ -152,17 +152,17 @@ def xml_propfind_response(base_prefix, path, item, props, user, encoding,
             else:
                 is404 = True
         elif tag == xmlutils.make_clark("D:principal-collection-set"):
-            tag = ET.Element(xmlutils.make_clark("D:href"))
+            child_element = ET.Element(xmlutils.make_clark("D:href"))
-            tag.text = xmlutils.make_href(base_prefix, "/")
+            child_element.text = xmlutils.make_href(base_prefix, "/")
-            element.append(tag)
+            element.append(child_element)
         elif (tag in (xmlutils.make_clark("C:calendar-user-address-set"),
                       xmlutils.make_clark("D:principal-URL"),
                       xmlutils.make_clark("CR:addressbook-home-set"),
                       xmlutils.make_clark("C:calendar-home-set")) and
               collection.is_principal and is_collection):
-            tag = ET.Element(xmlutils.make_clark("D:href"))
+            child_element = ET.Element(xmlutils.make_clark("D:href"))
-            tag.text = xmlutils.make_href(base_prefix, path)
+            child_element.text = xmlutils.make_href(base_prefix, path)
-            element.append(tag)
+            element.append(child_element)
         elif tag == xmlutils.make_clark("C:supported-calendar-component-set"):
             human_tag = xmlutils.make_human_tag(tag)
             if is_collection and is_leaf:
@@ -179,9 +179,10 @@ def xml_propfind_response(base_prefix, path, item, props, user, encoding,
                 is404 = True
         elif tag == xmlutils.make_clark("D:current-user-principal"):
             if user:
-                tag = ET.Element(xmlutils.make_clark("D:href"))
+                child_element = ET.Element(xmlutils.make_clark("D:href"))
-                tag.text = xmlutils.make_href(base_prefix, "/%s/" % user)
+                child_element.text = xmlutils.make_href(
-                element.append(tag)
+                    base_prefix, "/%s/" % user)
+                element.append(child_element)
             else:
                 element.append(ET.Element(
                     xmlutils.make_clark("D:unauthenticated")))
@@ -213,9 +214,10 @@ def xml_propfind_response(base_prefix, path, item, props, user, encoding,
             for human_tag in reports:
                 supported_report = ET.Element(
                     xmlutils.make_clark("D:supported-report"))
-                report_tag = ET.Element(xmlutils.make_clark("D:report"))
+                report_element = ET.Element(xmlutils.make_clark("D:report"))
-                report_tag.append(ET.Element(xmlutils.make_clark(human_tag)))
+                report_element.append(
-                supported_report.append(report_tag)
+                    ET.Element(xmlutils.make_clark(human_tag)))
+                supported_report.append(report_element)
                 element.append(supported_report)
         elif tag == xmlutils.make_clark("D:getcontentlength"):
             if not is_collection or is_leaf:
@@ -225,10 +227,10 @@ def xml_propfind_response(base_prefix, path, item, props, user, encoding,
         elif tag == xmlutils.make_clark("D:owner"):
             # return empty elment, if no owner available (rfc3744-5.1)
             if collection.owner:
-                tag = ET.Element(xmlutils.make_clark("D:href"))
+                child_element = ET.Element(xmlutils.make_clark("D:href"))
-                tag.text = xmlutils.make_href(
+                child_element.text = xmlutils.make_href(
                     base_prefix, "/%s/" % collection.owner)
-                element.append(tag)
+                element.append(child_element)
         elif is_collection:
             if tag == xmlutils.make_clark("D:getcontenttype"):
                 if is_leaf:
@@ -237,18 +239,20 @@ def xml_propfind_response(base_prefix, path, item, props, user, encoding,
                     is404 = True
             elif tag == xmlutils.make_clark("D:resourcetype"):
                 if item.is_principal:
-                    tag = ET.Element(xmlutils.make_clark("D:principal"))
+                    child_element = ET.Element(
-                    element.append(tag)
+                        xmlutils.make_clark("D:principal"))
+                    element.append(child_element)
                 if is_leaf:
                     if item.get_meta("tag") == "VADDRESSBOOK":
-                        tag = ET.Element(
+                        child_element = ET.Element(
                             xmlutils.make_clark("CR:addressbook"))
-                        element.append(tag)
+                        element.append(child_element)
                     elif item.get_meta("tag") == "VCALENDAR":
-                        tag = ET.Element(xmlutils.make_clark("C:calendar"))
+                        child_element = ET.Element(
-                        element.append(tag)
+                            xmlutils.make_clark("C:calendar"))
-                tag = ET.Element(xmlutils.make_clark("D:collection"))
+                        element.append(child_element)
-                element.append(tag)
+                child_element = ET.Element(xmlutils.make_clark("D:collection"))
+                element.append(child_element)
             elif tag == xmlutils.make_clark("RADICALE:displayname"):
                 # Only for internal use by the web interface
                 displayname = item.get_meta("D:displayname")
@@ -353,7 +357,7 @@ class ApplicationPropfindMixin:
                 "Bad PROPFIND request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             return httputils.REQUEST_TIMEOUT
         with self._storage.acquire_lock("r", user):
             items = self._storage.discover(

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
@@ -27,57 +28,35 @@ from radicale import storage, xmlutils
 from radicale.log import logger
 
 
-def xml_add_propstat_to(element, tag, status_number):
-    """Add a PROPSTAT response structure to an element.
-
-    The PROPSTAT answer structure is defined in rfc4918-9.1. It is added to the
-    given ``element``, for the following ``tag`` with the given
-    ``status_number``.
-
-    """
-    propstat = ET.Element(xmlutils.make_clark("D:propstat"))
-    element.append(propstat)
-
-    prop = ET.Element(xmlutils.make_clark("D:prop"))
-    propstat.append(prop)
-
-    clark_tag = xmlutils.make_clark(tag)
-    prop_tag = ET.Element(clark_tag)
-    prop.append(prop_tag)
-
-    status = ET.Element(xmlutils.make_clark("D:status"))
-    status.text = xmlutils.make_response(status_number)
-    propstat.append(status)
-
-
 def xml_proppatch(base_prefix, path, xml_request, collection):
     """Read and answer PROPPATCH requests.
 
     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"))
+    status.text = xmlutils.make_response(200)
+    props_ok = ET.Element(xmlutils.make_clark("D:prop"))
+    propstat.append(props_ok)
+    propstat.append(status)
+    response.append(propstat)
 
     new_props = collection.get_meta()
-    for short_name, value in props_to_set.items():
+    for short_name, value in xmlutils.props_from_request(xml_request).items():
-        new_props[short_name] = value
+        if value is None:
-        xml_add_propstat_to(response, short_name, 200)
+            with contextlib.suppress(KeyError):
-    for short_name in props_to_remove:
+                del new_props[short_name]
-        try:
+        else:
-            del new_props[short_name]
+            new_props[short_name] = value
-        except KeyError:
+        props_ok.append(ET.Element(xmlutils.make_clark(short_name)))
-            pass
-        xml_add_propstat_to(response, short_name, 200)
     radicale_item.check_and_sanitize_props(new_props)
     collection.set_meta(new_props)
 
@@ -97,7 +76,7 @@ class ApplicationProppatchMixin:
                 "Bad PROPPATCH request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             return httputils.REQUEST_TIMEOUT
         with self._storage.acquire_lock("w", user):
             item = next(self._storage.discover(path), None)

radicale/app/put.py

@@ -123,7 +123,7 @@ class ApplicationPutMixin:
             logger.warning("Bad PUT request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             return httputils.REQUEST_TIMEOUT
         # Prepare before locking
         content_type = environ.get("CONTENT_TYPE", "").split(";")[0]

radicale/app/report.py

@@ -104,8 +104,8 @@ def xml_report(base_prefix, path, xml_request, collection, encoding,
     else:
         hreferences = (path,)
     filters = (
-        root.findall("./%s" % xmlutils.make_clark("C:filter")) +
+        root.findall(xmlutils.make_clark("C:filter")) +
-        root.findall("./%s" % xmlutils.make_clark("CR:filter")))
+        root.findall(xmlutils.make_clark("CR:filter")))
 
     def retrieve_items(collection, hreferences, multistatus):
         """Retrieves all items that are referenced in ``hreferences`` from
@@ -181,7 +181,7 @@ def xml_report(base_prefix, path, xml_request, collection, encoding,
                     radicale_filter.prop_match(item.vobject_item, f, "CR")
                     for f in filter_)
             raise ValueError("Unsupported filter test: %r" % test)
-        raise ValueError("unsupported filter %r for %r" % (filter_.tag, tag))
+        raise ValueError("Unsupported filter %r for %r" % (filter_.tag, tag))
 
     while retrieved_items:
         # ``item.vobject_item`` might be accessed during filtering.
@@ -231,9 +231,9 @@ def xml_item_response(base_prefix, href, found_props=(), not_found_props=(),
                       found_item=True):
     response = ET.Element(xmlutils.make_clark("D:response"))
 
-    href_tag = ET.Element(xmlutils.make_clark("D:href"))
+    href_element = ET.Element(xmlutils.make_clark("D:href"))
-    href_tag.text = xmlutils.make_href(base_prefix, href)
+    href_element.text = xmlutils.make_href(base_prefix, href)
-    response.append(href_tag)
+    response.append(href_element)
 
     if found_item:
         for code, props in ((200, found_props), (404, not_found_props)):
@@ -241,10 +241,10 @@ def xml_item_response(base_prefix, href, found_props=(), not_found_props=(),
                 propstat = ET.Element(xmlutils.make_clark("D:propstat"))
                 status = ET.Element(xmlutils.make_clark("D:status"))
                 status.text = xmlutils.make_response(code)
-                prop_tag = ET.Element(xmlutils.make_clark("D:prop"))
+                prop_element = ET.Element(xmlutils.make_clark("D:prop"))
                 for prop in props:
-                    prop_tag.append(prop)
+                    prop_element.append(prop)
-                propstat.append(prop_tag)
+                propstat.append(prop_element)
                 propstat.append(status)
                 response.append(propstat)
     else:
@@ -268,7 +268,7 @@ class ApplicationReportMixin:
                 "Bad REPORT request on %r: %s", path, e, exc_info=True)
             return httputils.BAD_REQUEST
         except socket.timeout:
-            logger.debug("client timed out", exc_info=True)
+            logger.debug("Client timed out", exc_info=True)
             return httputils.REQUEST_TIMEOUT
         with contextlib.ExitStack() as lock_stack:
             lock_stack.enter_context(self._storage.acquire_lock("r", user))

radicale/config.py

@@ -94,7 +94,7 @@ def unspecified_type(value):
 
 def _convert_to_bool(value):
     if value.lower() not in RawConfigParser.BOOLEAN_STATES:
-        raise ValueError("Not a boolean: %r" % value)
+        raise ValueError("not a boolean: %r" % value)
     return RawConfigParser.BOOLEAN_STATES[value.lower()]
 
 

radicale/item/__init__.py

@@ -134,8 +134,8 @@ def check_and_sanitize_items(vobject_items, is_collection=False, tag=None):
             try:
                 component.rruleset
             except Exception as e:
-                raise ValueError("invalid recurrence rules in %s" %
+                raise ValueError("Invalid recurrence rules in %s in object %r"
-                                 component.name) from e
+                                 % (component.name, component_uid)) from e
     elif tag == "VADDRESSBOOK":
         # https://tools.ietf.org/html/rfc6352#section-5.1
         object_uids = set()
@@ -311,10 +311,10 @@ class Item:
         """
         if text is None and vobject_item is None:
             raise ValueError(
-                "at least one of 'text' or 'vobject_item' must be set")
+                "At least one of 'text' or 'vobject_item' must be set")
         if collection_path is None:
             if collection is None:
-                raise ValueError("at least one of 'collection_path' or "
+                raise ValueError("At least one of 'collection_path' or "
                                  "'collection' must be set")
             collection_path = collection.path
         assert collection_path == pathutils.strip_path(

radicale/tests/__init__.py

@@ -76,11 +76,11 @@ class BaseTest:
                 status = propstat.find(xmlutils.make_clark("D:status"))
                 assert status.text.startswith("HTTP/1.1 ")
                 status_code = int(status.text.split(" ")[1])
-                for prop in propstat.findall(xmlutils.make_clark("D:prop")):
+                for element in propstat.findall(
-                    for element in prop:
+                        "./%s/*" % xmlutils.make_clark("D:prop")):
-                        human_tag = xmlutils.make_human_tag(element.tag)
+                    human_tag = xmlutils.make_human_tag(element.tag)
-                        assert human_tag not in prop_respones
+                    assert human_tag not in prop_respones
-                        prop_respones[human_tag] = (status_code, element)
+                    prop_respones[human_tag] = (status_code, element)
             status = response.find(xmlutils.make_clark("D:status"))
             if status is not None:
                 assert not prop_respones

radicale/tests/static/mkcol_make_calendar.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:mkcol xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
+  <D:set>
+    <D:prop>
+      <D:resourcetype><collection /><C:calendar /></D:resourcetype>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/">#BADA55</I:calendar-color>
+    </D:prop>
+  </D:set>
+</D:mkcol>

radicale/tests/static/propfind_multiple.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propfind xmlns:D="DAV:">
+  <D:prop>
+    <I:calendar-color xmlns:I="http://apple.com/ns/ical/" />
+    <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav" />
+  </D:prop>
+</D:propfind>

radicale/tests/static/proppatch_remove_calendar_color.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:remove>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/" />
+    </D:prop>
+  </D:remove>
+</D:propertyupdate>

radicale/tests/static/proppatch_remove_multiple1.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:remove>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/" />
+      <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav" />
+    </D:prop>
+  </D:remove>
+</D:propertyupdate>

radicale/tests/static/proppatch_remove_multiple2.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:remove>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/" />
+    </D:prop>
+  </D:remove>
+  <D:remove>
+    <D:prop>
+      <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav" />
+    </D:prop>
+  </D:remove>
+</D:propertyupdate>

radicale/tests/static/proppatch_set_and_remove.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:remove>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/" />
+    </D:prop>
+  </D:remove>
+  <D:set>
+    <D:prop>
+      <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav">test2</C:calendar-description>
+    </D:prop>
+  </D:set>
+</D:propertyupdate>

radicale/tests/static/proppatch_set_multiple1.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:set>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/">#BADA55</I:calendar-color>
+      <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav">test</C:calendar-description>
+    </D:prop>
+  </D:set>
+</D:propertyupdate>

radicale/tests/static/proppatch_set_multiple2.xml

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+  <D:set>
+    <D:prop>
+      <I:calendar-color xmlns:I="http://apple.com/ns/ical/">#BADA55</I:calendar-color>
+    </D:prop>
+  </D:set>
+  <D:set>
+    <D:prop>
+      <C:calendar-description xmlns:C="urn:ietf:params:xml:ns:caldav">test</C:calendar-description>
+    </D:prop>
+  </D:set>
+</D:propertyupdate>

radicale/tests/test_base.py

@@ -235,7 +235,7 @@ class BaseRequestsMixIn:
         assert "END:VCALENDAR" in answer
 
     def test_mkcalendar_overwrite(self):
-        """Make a calendar."""
+        """Try to overwrite an existing calendar."""
         self.mkcalendar("/calendar.ics/")
         status, answer = self.mkcalendar("/calendar.ics/", check=False)
         assert status in (403, 409)
@@ -244,6 +244,40 @@ class BaseRequestsMixIn:
         assert xml.find(xmlutils.make_clark(
             "D:resource-must-be-null")) is not None
 
+    def test_mkcalendar_intermediate(self):
+        """Try make a calendar in a unmapped collection."""
+        status, _ = self.mkcalendar("/unmapped/calendar.ics/", check=False)
+        assert status == 409
+
+    def test_mkcol(self):
+        """Make a collection."""
+        self.mkcol("/user/")
+
+    def test_mkcol_overwrite(self):
+        """Try to overwrite an existing collection."""
+        self.mkcol("/user/")
+        status = self.mkcol("/user/", check=False)
+        assert status == 405
+
+    def test_mkcol_intermediate(self):
+        """Try make a collection in a unmapped collection."""
+        status = self.mkcol("/unmapped/user/", check=False)
+        assert status == 409
+
+    def test_mkcol_make_calendar(self):
+        """Make a calendar with additional props."""
+        mkcol_make_calendar = get_file_content("mkcol_make_calendar.xml")
+        self.mkcol("/calendar.ics/", mkcol_make_calendar)
+        _, answer = self.get("/calendar.ics/")
+        assert "BEGIN:VCALENDAR" in answer
+        assert "END:VCALENDAR" in answer
+        # Read additional properties
+        propfind = get_file_content("propfind_calendar_color.xml")
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 1
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and prop.text == "#BADA55"
+
     def test_move(self):
         """Move a item."""
         self.mkcalendar("/calendar.ics/")
@@ -390,22 +424,22 @@ class BaseRequestsMixIn:
     def test_propfind_nonexistent(self):
         """Read a property that does not exist."""
         self.mkcalendar("/calendar.ics/")
-        propfind = get_file_content("propfind1.xml")
+        propfind = get_file_content("propfind_calendar_color.xml")
         _, responses = self.propfind("/calendar.ics/", propfind)
         assert len(responses["/calendar.ics/"]) == 1
         status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
         assert status == 404 and not prop.text
 
     def test_proppatch(self):
-        """Write a property and read it back."""
+        """Set/Remove a property and read it back."""
         self.mkcalendar("/calendar.ics/")
-        proppatch = get_file_content("proppatch1.xml")
+        proppatch = get_file_content("proppatch_set_calendar_color.xml")
         _, responses = self.proppatch("/calendar.ics/", proppatch)
         assert len(responses["/calendar.ics/"]) == 1
         status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
         assert status == 200 and not prop.text
         # Read property back
-        propfind = get_file_content("propfind1.xml")
+        propfind = get_file_content("propfind_calendar_color.xml")
         _, responses = self.propfind("/calendar.ics/", propfind)
         assert len(responses["/calendar.ics/"]) == 1
         status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
@@ -414,6 +448,109 @@ class BaseRequestsMixIn:
         _, responses = self.propfind("/calendar.ics/", propfind)
         status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
         assert status == 200 and prop.text == "#BADA55"
+        # Remove property
+        proppatch = get_file_content("proppatch_remove_calendar_color.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 1
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        # Read property back
+        propfind = get_file_content("propfind_calendar_color.xml")
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 1
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 404
+
+    def test_proppatch_multiple1(self):
+        """Set/Remove a multiple properties and read them back."""
+        self.mkcalendar("/calendar.ics/")
+        propfind = get_file_content("propfind_multiple.xml")
+        proppatch = get_file_content("proppatch_set_multiple1.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and not prop.text
+        # Read properties back
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and prop.text == "#BADA55"
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and prop.text == "test"
+        # Remove properties
+        proppatch = get_file_content("proppatch_remove_multiple1.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and not prop.text
+        # Read properties back
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 404
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 404
+
+    def test_proppatch_multiple2(self):
+        """Set/Remove a multiple properties and read them back."""
+        self.mkcalendar("/calendar.ics/")
+        propfind = get_file_content("propfind_multiple.xml")
+        proppatch = get_file_content("proppatch_set_multiple2.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and not prop.text
+        # Read properties back
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and prop.text == "#BADA55"
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and prop.text == "test"
+        # Remove properties
+        proppatch = get_file_content("proppatch_remove_multiple2.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and not prop.text
+        # Read properties back
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 404
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 404
+
+    def test_proppatch_set_and_remove(self):
+        """Set and remove multiple properties in single request."""
+        self.mkcalendar("/calendar.ics/")
+        propfind = get_file_content("propfind_multiple.xml")
+        # Prepare
+        proppatch = get_file_content("proppatch_set_multiple1.xml")
+        self.proppatch("/calendar.ics/", proppatch)
+        # Remove and set properties in single request
+        proppatch = get_file_content("proppatch_set_and_remove.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 200 and not prop.text
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and not prop.text
+        # Read properties back
+        _, responses = self.propfind("/calendar.ics/", propfind)
+        assert len(responses["/calendar.ics/"]) == 2
+        status, prop = responses["/calendar.ics/"]["ICAL:calendar-color"]
+        assert status == 404
+        status, prop = responses["/calendar.ics/"]["C:calendar-description"]
+        assert status == 200 and prop.text == "test2"
 
     def test_put_whole_calendar_multiple_events_with_same_uid(self):
         """Add two events with the same UID."""

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")):
+def props_from_request(xml_request):
-    """Return a list of properties as a dictionary."""
+    """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:
+    # Requests can contain multipe <D:set> and <D:remove> elements.
-        action_element = xml_request.find(make_clark("D:%s" % action))
+    # Each of these elements must contain exactly one <D:prop> element which
-        if action_element is not None:
+    # can contain multpile properties.
-            break
+    # The order of the elements in the document must be respected.
-    else:
+    props = []
-        action_element = xml_request
+    for element in xml_request:
-
+        if element.tag in (make_clark("D:set"), make_clark("D:remove")):
-    prop_element = action_element.find(make_clark("D:prop"))
+            for prop in element.findall("./%s/*" % make_clark("D:prop")):
-    if prop_element is not None:
+                props.append((element.tag == make_clark("D:set"), prop))
-        for prop in prop_element:
+    for is_set, prop in props:
-            if prop.tag == make_clark("D:resourcetype"):
+        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"):
+        elif prop.tag == make_clark("C:supported-calendar-component-set"):
-                result[make_human_tag(prop.tag)] = ",".join(
+            if is_set:
-                    supported_comp.attrib["name"]
+                value = ",".join(
-                    for supported_comp in prop
+                    supported_comp.attrib["name"] for supported_comp in prop
                     if supported_comp.tag == make_clark("C:comp"))
-            else:
+        elif is_set:
-                result[make_human_tag(prop.tag)] = prop.text
+            value = prop.text or ""
+        result[key] = value
+        result.move_to_end(key)
 
     return result