diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index b0c53a98..564a45b3 100644 --- a/radicale/app/__init__.py +++ b/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)" % ( - environ["HTTP_X_FORWARDED_FOR"], remote_host) + remote_host = "%s (forwarded for %r)" % ( + 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: diff --git a/radicale/app/mkcalendar.py b/radicale/app/mkcalendar.py index d5c6c4db..ae141819 100644 --- a/radicale/app/mkcalendar.py +++ b/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") diff --git a/radicale/app/mkcol.py b/radicale/app/mkcol.py index f7195410..4118141d 100644 --- a/radicale/app/mkcol.py +++ b/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: diff --git a/radicale/app/propfind.py b/radicale/app/propfind.py index 106e2e84..cacbed93 100644 --- a/radicale/app/propfind.py +++ b/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 - ET.Element(xmlutils.make_clark("D:allprop"))) + top_element = (xml_request[0] if xml_request is not None else + 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"): - props = [prop.tag for prop in top_tag] + elif top_element.tag == xmlutils.make_clark("D:prop"): + 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")) - tag.text = xmlutils.make_href(base_prefix, "/") - element.append(tag) + child_element = ET.Element(xmlutils.make_clark("D:href")) + child_element.text = xmlutils.make_href(base_prefix, "/") + 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")) - tag.text = xmlutils.make_href(base_prefix, path) - element.append(tag) + child_element = ET.Element(xmlutils.make_clark("D:href")) + child_element.text = xmlutils.make_href(base_prefix, path) + 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")) - tag.text = xmlutils.make_href(base_prefix, "/%s/" % user) - element.append(tag) + child_element = ET.Element(xmlutils.make_clark("D:href")) + child_element.text = xmlutils.make_href( + 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_tag.append(ET.Element(xmlutils.make_clark(human_tag))) - supported_report.append(report_tag) + report_element = ET.Element(xmlutils.make_clark("D:report")) + report_element.append( + 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")) - tag.text = xmlutils.make_href( + child_element = ET.Element(xmlutils.make_clark("D: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")) - element.append(tag) + child_element = ET.Element( + 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")) - element.append(tag) - tag = ET.Element(xmlutils.make_clark("D:collection")) - element.append(tag) + child_element = ET.Element( + xmlutils.make_clark("C:calendar")) + element.append(child_element) + 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( diff --git a/radicale/app/proppatch.py b/radicale/app/proppatch.py index 3ec10c30..a77228f6 100644 --- a/radicale/app/proppatch.py +++ b/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 . +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(): - new_props[short_name] = value - xml_add_propstat_to(response, short_name, 200) - for short_name in props_to_remove: - try: - del new_props[short_name] - except KeyError: - pass - xml_add_propstat_to(response, short_name, 200) + 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) @@ -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) diff --git a/radicale/app/put.py b/radicale/app/put.py index efaf9053..29519745 100644 --- a/radicale/app/put.py +++ b/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] diff --git a/radicale/app/report.py b/radicale/app/report.py index 18ea4e79..99cd01a2 100644 --- a/radicale/app/report.py +++ b/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("./%s" % xmlutils.make_clark("CR:filter"))) + root.findall(xmlutils.make_clark("C: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_tag.text = xmlutils.make_href(base_prefix, href) - response.append(href_tag) + href_element = ET.Element(xmlutils.make_clark("D:href")) + href_element.text = xmlutils.make_href(base_prefix, href) + 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) - propstat.append(prop_tag) + prop_element.append(prop) + 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)) diff --git a/radicale/config.py b/radicale/config.py index d30e677c..10dd59f5 100644 --- a/radicale/config.py +++ b/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()] diff --git a/radicale/item/__init__.py b/radicale/item/__init__.py index 53a21137..3bbc55e9 100644 --- a/radicale/item/__init__.py +++ b/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" % - component.name) from e + raise ValueError("Invalid recurrence rules in %s in object %r" + % (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( diff --git a/radicale/tests/__init__.py b/radicale/tests/__init__.py index e2e283e2..a3de3007 100644 --- a/radicale/tests/__init__.py +++ b/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 prop: - human_tag = xmlutils.make_human_tag(element.tag) - assert human_tag not in prop_respones - prop_respones[human_tag] = (status_code, element) + for element in propstat.findall( + "./%s/*" % xmlutils.make_clark("D:prop")): + human_tag = xmlutils.make_human_tag(element.tag) + assert human_tag not in prop_respones + prop_respones[human_tag] = (status_code, element) status = response.find(xmlutils.make_clark("D:status")) if status is not None: assert not prop_respones diff --git a/radicale/tests/static/mkcol_make_calendar.xml b/radicale/tests/static/mkcol_make_calendar.xml new file mode 100644 index 00000000..cfffd5f9 --- /dev/null +++ b/radicale/tests/static/mkcol_make_calendar.xml @@ -0,0 +1,9 @@ + + + + + + #BADA55 + + + \ No newline at end of file diff --git a/radicale/tests/static/propfind1.xml b/radicale/tests/static/propfind_calendar_color.xml similarity index 100% rename from radicale/tests/static/propfind1.xml rename to radicale/tests/static/propfind_calendar_color.xml diff --git a/radicale/tests/static/propfind_multiple.xml b/radicale/tests/static/propfind_multiple.xml new file mode 100644 index 00000000..9b5d03b5 --- /dev/null +++ b/radicale/tests/static/propfind_multiple.xml @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch_remove_calendar_color.xml b/radicale/tests/static/proppatch_remove_calendar_color.xml new file mode 100644 index 00000000..d92edd70 --- /dev/null +++ b/radicale/tests/static/proppatch_remove_calendar_color.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch_remove_multiple1.xml b/radicale/tests/static/proppatch_remove_multiple1.xml new file mode 100644 index 00000000..0ade9b0f --- /dev/null +++ b/radicale/tests/static/proppatch_remove_multiple1.xml @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch_remove_multiple2.xml b/radicale/tests/static/proppatch_remove_multiple2.xml new file mode 100644 index 00000000..c0cfaf6f --- /dev/null +++ b/radicale/tests/static/proppatch_remove_multiple2.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch_set_and_remove.xml b/radicale/tests/static/proppatch_set_and_remove.xml new file mode 100644 index 00000000..d4d56492 --- /dev/null +++ b/radicale/tests/static/proppatch_set_and_remove.xml @@ -0,0 +1,13 @@ + + + + + + + + + + test2 + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch1.xml b/radicale/tests/static/proppatch_set_calendar_color.xml similarity index 100% rename from radicale/tests/static/proppatch1.xml rename to radicale/tests/static/proppatch_set_calendar_color.xml diff --git a/radicale/tests/static/proppatch_set_multiple1.xml b/radicale/tests/static/proppatch_set_multiple1.xml new file mode 100644 index 00000000..f5cd26f4 --- /dev/null +++ b/radicale/tests/static/proppatch_set_multiple1.xml @@ -0,0 +1,9 @@ + + + + + #BADA55 + test + + + \ No newline at end of file diff --git a/radicale/tests/static/proppatch_set_multiple2.xml b/radicale/tests/static/proppatch_set_multiple2.xml new file mode 100644 index 00000000..8977eb37 --- /dev/null +++ b/radicale/tests/static/proppatch_set_multiple2.xml @@ -0,0 +1,13 @@ + + + + + #BADA55 + + + + + test + + + \ No newline at end of file diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index 97acbec0..5aef8494 100644 --- a/radicale/tests/test_base.py +++ b/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.""" diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index af60ce6b..fa6f6bf8 100644 --- a/radicale/xmlutils.py +++ b/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 and elements. + # Each of these elements must contain exactly one 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