From 11c5dfdb53441c41c74691d75d0dc2a3106babfc Mon Sep 17 00:00:00 2001 From: Unrud Date: Sun, 7 May 2017 08:17:39 +0200 Subject: [PATCH] Improve handling of XML requests and responses * Move parsing/serialization of XML requests/responses from ``xmlutils.py`` to ``__init__.py``. * Log XML requests/responses in pretty-printed form. * Previously only the responses were logged in readable form. This is useful for debugging. * The XML documents are only converted for pretty-printing if debugging is enabled (it's expensive) * Send XML responses in minimized form to clients. * Add **encoding** attribute to XML declaration in XML response. * Only decode XML requests once. (Previously they were decoded, encoded and decoded again.) --- radicale/__init__.py | 90 +++++++++++++++++++++++++++++++------------- radicale/xmlutils.py | 46 +++++++++++----------- 2 files changed, 85 insertions(+), 51 deletions(-) diff --git a/radicale/__init__.py b/radicale/__init__.py index 4a37238..3c99baa 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -31,6 +31,7 @@ import contextlib import datetime import io import itertools +import logging import os import posixpath import pprint @@ -46,6 +47,7 @@ import wsgiref.simple_server import zlib from http import client from urllib.parse import unquote, urlparse +from xml.etree import ElementTree as ET import vobject @@ -463,16 +465,43 @@ class Application: allowed |= self.authorized(user, parent_path, permission) return allowed - def _read_content(self, environ): + def _read_raw_content(self, environ): content_length = int(environ.get("CONTENT_LENGTH") or 0) - if content_length > 0: - content = self.decode( - environ["wsgi.input"].read(content_length), environ) - self.logger.debug("Request content:\n%s", content.strip()) - else: - content = None + if not content_length: + return b"" + content = environ["wsgi.input"].read(content_length) + if len(content) < content_length: + raise ValueError("Request body too short: %d" % len(content)) return content + def _read_content(self, environ): + content = self.decode(self._read_raw_content(environ), environ) + self.logger.debug("Request content:\n%s", content) + return content + + def _read_xml_content(self, environ): + content = self.decode(self._read_raw_content(environ), environ) + if not content: + return None + try: + xml_content = ET.fromstring(content) + except ET.ParseError: + self.logger.debug("Request content (Invalid XML):\n%s", content) + raise + if self.logger.isEnabledFor(logging.DEBUG): + self.logger.debug("Request content:\n%s", + xmlutils.pretty_xml(xml_content)) + return xml_content + + def _write_xml_content(self, xml_content): + if self.logger.isEnabledFor(logging.DEBUG): + self.logger.debug("Response content:\n%s", + xmlutils.pretty_xml(xml_content)) + f = io.BytesIO() + ET.ElementTree(xml_content).write(f, encoding=self.encoding, + xml_declaration=True) + return f.getvalue() + def do_DELETE(self, environ, base_prefix, path, user): """Manage DELETE request.""" if not self._access(user, path, "w"): @@ -488,11 +517,12 @@ class Application: # ETag precondition not verified, do not delete item return PRECONDITION_FAILED if isinstance(item, self.Collection): - answer = xmlutils.delete(base_prefix, path, item) + xml_answer = xmlutils.delete(base_prefix, path, item) else: - answer = xmlutils.delete( + xml_answer = xmlutils.delete( base_prefix, path, item.collection, item.href) - return client.OK, {"Content-Type": "text/xml"}, answer + headers = {"Content-Type": "text/xml; charset=%s" % self.encoding} + return client.OK, headers, self._write_xml_content(xml_answer) def do_GET(self, environ, base_prefix, path, user): """Manage GET request.""" @@ -533,12 +563,12 @@ class Application: """Manage MKCALENDAR request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED - content = self._read_content(environ) + xml_content = self._read_xml_content(environ) with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if item: return WEBDAV_PRECONDITION_FAILED - props = xmlutils.props_from_request(content) + props = xmlutils.props_from_request(xml_content) props["tag"] = "VCALENDAR" # TODO: use this? # timezone = props.get("C:calendar-timezone") @@ -549,12 +579,12 @@ class Application: """Manage MKCOL request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED - content = self._read_content(environ) + xml_content = self._read_xml_content(environ) with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if item: return WEBDAV_PRECONDITION_FAILED - props = xmlutils.props_from_request(content) + props = xmlutils.props_from_request(xml_content) self.Collection.create_collection(path, props=props) return client.CREATED, {}, None @@ -607,7 +637,7 @@ class Application: """Manage PROPFIND request.""" if not self._access(user, path, "r"): return NOT_ALLOWED - content = self._read_content(environ) + xml_content = self._read_xml_content(environ) with self.Collection.acquire_lock("r", user): items = self.Collection.discover( path, environ.get("HTTP_DEPTH", "0")) @@ -620,26 +650,30 @@ class Application: # put item back items = itertools.chain([item], items) read_items, write_items = self.collect_allowed_items(items, user) - headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"} - status, answer = xmlutils.propfind( - base_prefix, path, content, read_items, write_items, user) + headers = {"DAV": DAV_HEADERS, + "Content-Type": "text/xml; charset=%s" % self.encoding} + status, xml_answer = xmlutils.propfind( + base_prefix, path, xml_content, read_items, write_items, user) if status == client.FORBIDDEN: return NOT_ALLOWED else: - return status, headers, answer + return status, headers, self._write_xml_content(xml_answer) def do_PROPPATCH(self, environ, base_prefix, path, user): """Manage PROPPATCH request.""" if not self.authorized(user, path, "w"): return NOT_ALLOWED - content = self._read_content(environ) + xml_content = self._read_xml_content(environ) with self.Collection.acquire_lock("w", user): item = next(self.Collection.discover(path), None) if not isinstance(item, self.Collection): return WEBDAV_PRECONDITION_FAILED - headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"} - answer = xmlutils.proppatch(base_prefix, path, content, item) - return client.MULTI_STATUS, headers, answer + headers = {"DAV": DAV_HEADERS, + "Content-Type": "text/xml; charset=%s" % self.encoding} + xml_answer = xmlutils.proppatch(base_prefix, path, xml_content, + item) + return (client.MULTI_STATUS, headers, + self._write_xml_content(xml_answer)) def do_PUT(self, environ, base_prefix, path, user): """Manage PUT request.""" @@ -697,7 +731,7 @@ class Application: """Manage REPORT request.""" if not self._access(user, path, "r"): return NOT_ALLOWED - content = self._read_content(environ) + xml_content = self._read_xml_content(environ) with self.Collection.acquire_lock("r", user): item = next(self.Collection.discover(path), None) if not self._access(user, path, "r", item): @@ -708,6 +742,8 @@ class Application: collection = item else: collection = item.collection - headers = {"Content-Type": "text/xml"} - answer = xmlutils.report(base_prefix, path, content, collection) - return client.MULTI_STATUS, headers, answer + headers = {"Content-Type": "text/xml; charset=%s" % self.encoding} + xml_answer = xmlutils.report( + base_prefix, path, xml_content, collection) + return (client.MULTI_STATUS, headers, + self._write_xml_content(xml_answer)) diff --git a/radicale/xmlutils.py b/radicale/xmlutils.py index 9f2da2c..3d7b189 100644 --- a/radicale/xmlutils.py +++ b/radicale/xmlutils.py @@ -25,6 +25,7 @@ in them for XML requests (all but PUT). """ +import copy import posixpath import re import xml.etree.ElementTree as ET @@ -56,8 +57,10 @@ CLARK_TAG_REGEX = re.compile(r"{(?P[^}]*)}(?P.*)", re.VERBOSE) HUMAN_REGEX = re.compile(r"(?P[^:{}]*)(?P.*)", re.VERBOSE) -def _pretty_xml(element, level=0): +def pretty_xml(element, level=0): """Indent an ElementTree ``element`` and its children.""" + if not level: + element = copy.deepcopy(element) i = "\n" + level * " " if len(element): if not element.text or not element.text.strip(): @@ -65,7 +68,7 @@ def _pretty_xml(element, level=0): if not element.tail or not element.tail.strip(): element.tail = i for sub_element in element: - _pretty_xml(sub_element, level + 1) + pretty_xml(sub_element, level + 1) if not sub_element.tail or not sub_element.tail.strip(): sub_element.tail = i else: @@ -439,21 +442,18 @@ def name_from_path(path, collection): return name -def props_from_request(root, actions=("set", "remove")): +def props_from_request(xml_request, actions=("set", "remove")): """Return a list of properties as a dictionary.""" result = OrderedDict() - if root: - if not hasattr(root, "tag"): - root = ET.fromstring(root.encode("utf8")) - else: + if xml_request is None: return result for action in actions: - action_element = root.find(_tag("D", action)) + action_element = xml_request.find(_tag("D", action)) if action_element is not None: break else: - action_element = root + action_element = xml_request prop_element = action_element.find(_tag("D", "prop")) if prop_element is not None: @@ -497,7 +497,7 @@ def delete(base_prefix, path, collection, href=None): status.text = _response(200) response.append(status) - return _pretty_xml(multistatus) + return multistatus def propfind(base_prefix, path, xml_request, read_collections, @@ -510,12 +510,10 @@ def propfind(base_prefix, path, xml_request, read_collections, in the output. """ - # Reading request - root = ET.fromstring(xml_request.encode("utf8")) if xml_request else None - # 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 = root[0] if root is not None else ET.Element(_tag("D", "allprop")) + top_tag = (xml_request[0] if xml_request is not None else + ET.Element(_tag("D", "allprop"))) props = () if top_tag.tag == _tag("D", "allprop"): @@ -567,7 +565,7 @@ def propfind(base_prefix, path, xml_request, read_collections, if response: multistatus.append(response) - return client.MULTI_STATUS, _pretty_xml(multistatus) + return client.MULTI_STATUS, multistatus def _propfind_response(base_prefix, path, item, props, user, write=False, @@ -802,9 +800,8 @@ def proppatch(base_prefix, path, xml_request, collection): Read rfc4918-9.2 for info. """ - root = ET.fromstring(xml_request.encode("utf8")) - props_to_set = props_from_request(root, actions=("set",)) - props_to_remove = props_from_request(root, actions=("remove",)) + props_to_set = props_from_request(xml_request, actions=("set",)) + props_to_remove = props_from_request(xml_request, actions=("remove",)) multistatus = ET.Element(_tag("D", "multistatus")) response = ET.Element(_tag("D", "response")) @@ -821,7 +818,7 @@ def proppatch(base_prefix, path, xml_request, collection): for short_name in props_to_set: _add_propstat_to(response, short_name, 200) - return _pretty_xml(multistatus) + return multistatus def report(base_prefix, path, xml_request, collection): @@ -830,7 +827,10 @@ def report(base_prefix, path, xml_request, collection): Read rfc3253-3.6 for info. """ - root = ET.fromstring(xml_request.encode("utf8")) + multistatus = ET.Element(_tag("D", "multistatus")) + if xml_request is None: + return multistatus + root = xml_request if root.tag in ( _tag("D", "principal-search-property-set"), _tag("D", "principal-property-search"), @@ -840,7 +840,7 @@ def report(base_prefix, path, xml_request, collection): # InfCloud asks for expand-property reports (even if we don't announce # support for them) and stops working if an error code is returned. collection.logger.warning("Unsupported report method: %s", root.tag) - return _pretty_xml(ET.Element(_tag("D", "multistatus"))) + return multistatus prop_element = root.find(_tag("D", "prop")) props = ( [prop.tag for prop in prop_element] @@ -865,8 +865,6 @@ def report(base_prefix, path, xml_request, collection): root.findall("./%s" % _tag("C", "filter")) + root.findall("./%s" % _tag("CR", "filter"))) - multistatus = ET.Element(_tag("D", "multistatus")) - for hreference in hreferences: try: name = name_from_path(hreference, collection) @@ -927,7 +925,7 @@ def report(base_prefix, path, xml_request, collection): base_prefix, uri, found_props=found_props, not_found_props=not_found_props, found_item=True)) - return _pretty_xml(multistatus) + return multistatus def _item_response(base_prefix, href, found_props=(), not_found_props=(),