Merge branch 'url' of https://github.com/Unrud/Radicale into Unrud-url

config

@@ -51,12 +51,6 @@
 # Reverse DNS to resolve client address in logs
 #dns_lookup = True
 
-# Root URL of Radicale (starting and ending with a slash)
-#base_prefix = /
-
-# Possibility to allow URLs cleaned by a HTTP server, without the base_prefix
-#can_skip_base_prefix = False
-
 # Message displayed in the client when a password is needed
 #realm = Radicale - Password Required
 

radicale/__init__.py

@@ -171,7 +171,7 @@ class RequestHandler(wsgiref.simple_server.WSGIRequestHandler):
     def get_environ(self):
         env = super().get_environ()
         # Parent class only tries latin1 encoding
-        env["PATH_INFO"] = urllib.parse.unquote(self.path.split("?", 1)[0])
+        env["PATH_INFO"] = unquote(self.path.split("?", 1)[0])
         return env
 
     def handle(self):
@@ -308,22 +308,18 @@ class Application:
         headers = pprint.pformat(self.headers_log(environ))
         self.logger.debug("Request headers:\n%s", headers)
 
-        # Strip base_prefix from request URI
-        base_prefix = self.configuration.get("server", "base_prefix")
-        if environ["PATH_INFO"].startswith(base_prefix):
-            environ["PATH_INFO"] = environ["PATH_INFO"][len(base_prefix):]
-        elif self.configuration.get("server", "can_skip_base_prefix"):
-            self.logger.debug(
-                "Prefix already stripped from path: %s", environ["PATH_INFO"])
-        else:
-            # Request path not starting with base_prefix, not allowed
-            self.logger.debug(
-                "Path not starting with prefix: %s", environ["PATH_INFO"])
-            return response(*NOT_ALLOWED)
-
+        # Let reverse proxies overwrite SCRIPT_NAME
+        if "HTTP_X_SCRIPT_NAME" in environ:
+            environ["SCRIPT_NAME"] = environ["HTTP_X_SCRIPT_NAME"]
+            self.logger.debug("Script name overwritten by client: %s",
+                              environ["SCRIPT_NAME"])
+        # Sanitize base prefix
+        environ["SCRIPT_NAME"] = storage.sanitize_path(
+            environ.get("SCRIPT_NAME", "")).rstrip("/")
+        self.logger.debug("Sanitized script name: %s", environ["SCRIPT_NAME"])
+        base_prefix = environ["SCRIPT_NAME"]
         # Sanitize request URI
-        environ["PATH_INFO"] = storage.sanitize_path(
-            unquote(environ["PATH_INFO"]))
+        environ["PATH_INFO"] = storage.sanitize_path(environ["PATH_INFO"])
         self.logger.debug("Sanitized path: %s", environ["PATH_INFO"])
         path = environ["PATH_INFO"]
 
@@ -377,7 +373,8 @@ class Application:
 
         if is_valid_user:
             try:
-                status, headers, answer = function(environ, path, user)
+                status, headers, answer = function(
+                    environ, base_prefix, path, user)
             except socket.timeout:
                 return response(*REQUEST_TIMEOUT)
         else:
@@ -424,7 +421,7 @@ class Application:
             content = None
         return content
 
-    def do_DELETE(self, environ, path, user):
+    def do_DELETE(self, environ, base_prefix, path, user):
         """Manage DELETE request."""
         if not self._access(user, path, "w"):
             return NOT_ALLOWED
@@ -439,12 +436,13 @@ class Application:
                 # ETag precondition not verified, do not delete item
                 return PRECONDITION_FAILED
             if isinstance(item, self.Collection):
-                answer = xmlutils.delete(path, item)
+                answer = xmlutils.delete(base_prefix, path, item)
             else:
-                answer = xmlutils.delete(path, item.collection, item.href)
+                answer = xmlutils.delete(
+                    base_prefix, path, item.collection, item.href)
             return client.OK, {"Content-Type": "text/xml"}, answer
 
-    def do_GET(self, environ, path, user):
+    def do_GET(self, environ, base_prefix, path, user):
         """Manage GET request."""
         # Display a "Radicale works!" message if the root URL is requested
         if not path.strip("/"):
@@ -473,12 +471,13 @@ class Application:
             answer = item.serialize()
             return client.OK, headers, answer
 
-    def do_HEAD(self, environ, path, user):
+    def do_HEAD(self, environ, base_prefix, path, user):
         """Manage HEAD request."""
-        status, headers, answer = self.do_GET(environ, path, user)
+        status, headers, answer = self.do_GET(
+            environ, base_prefix, path, user)
         return status, headers, None
 
-    def do_MKCALENDAR(self, environ, path, user):
+    def do_MKCALENDAR(self, environ, base_prefix, path, user):
         """Manage MKCALENDAR request."""
         if not self.authorized(user, path, "w"):
             return NOT_ALLOWED
@@ -494,7 +493,7 @@ class Application:
             self.Collection.create_collection(path, props=props)
             return client.CREATED, {}, None
 
-    def do_MKCOL(self, environ, path, user):
+    def do_MKCOL(self, environ, base_prefix, path, user):
         """Manage MKCOL request."""
         if not self.authorized(user, path, "w"):
             return NOT_ALLOWED
@@ -507,7 +506,7 @@ class Application:
             self.Collection.create_collection(path, props=props)
             return client.CREATED, {}, None
 
-    def do_MOVE(self, environ, path, user):
+    def do_MOVE(self, environ, base_prefix, path, user):
         """Manage MOVE request."""
         to_url = urlparse(environ["HTTP_DESTINATION"])
         if to_url.netloc != environ["HTTP_HOST"]:
@@ -544,7 +543,7 @@ class Application:
             self.Collection.move(item, to_collection, to_href)
             return client.CREATED, {}, None
 
-    def do_OPTIONS(self, environ, path, user):
+    def do_OPTIONS(self, environ, base_prefix, path, user):
         """Manage OPTIONS request."""
         headers = {
             "Allow": ", ".join(
@@ -552,7 +551,7 @@ class Application:
             "DAV": DAV_HEADERS}
         return client.OK, headers, None
 
-    def do_PROPFIND(self, environ, path, user):
+    def do_PROPFIND(self, environ, base_prefix, path, user):
         """Manage PROPFIND request."""
         if not self._access(user, path, "r"):
             return NOT_ALLOWED
@@ -571,13 +570,13 @@ class Application:
             read_items, write_items = self.collect_allowed_items(items, user)
             headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"}
             status, answer = xmlutils.propfind(
-                path, content, read_items, write_items, user)
+                base_prefix, path, content, read_items, write_items, user)
             if status == client.FORBIDDEN:
                 return NOT_ALLOWED
             else:
                 return status, headers, answer
 
-    def do_PROPPATCH(self, environ, path, user):
+    def do_PROPPATCH(self, environ, base_prefix, path, user):
         """Manage PROPPATCH request."""
         if not self.authorized(user, path, "w"):
             return NOT_ALLOWED
@@ -587,10 +586,10 @@ class Application:
             if not isinstance(item, self.Collection):
                 return WEBDAV_PRECONDITION_FAILED
             headers = {"DAV": DAV_HEADERS, "Content-Type": "text/xml"}
-            answer = xmlutils.proppatch(path, content, item)
+            answer = xmlutils.proppatch(base_prefix, path, content, item)
             return client.MULTI_STATUS, headers, answer
 
-    def do_PUT(self, environ, path, user):
+    def do_PUT(self, environ, base_prefix, path, user):
         """Manage PUT request."""
         if not self._access(user, path, "w"):
             return NOT_ALLOWED
@@ -642,7 +641,7 @@ class Application:
             headers = {"ETag": new_item.etag}
             return client.CREATED, headers, None
 
-    def do_REPORT(self, environ, path, user):
+    def do_REPORT(self, environ, base_prefix, path, user):
         """Manage REPORT request."""
         if not self._access(user, path, "w"):
             return NOT_ALLOWED
@@ -658,5 +657,5 @@ class Application:
             else:
                 collection = item.collection
             headers = {"Content-Type": "text/xml"}
-            answer = xmlutils.report(path, content, collection)
+            answer = xmlutils.report(base_prefix, path, content, collection)
             return client.MULTI_STATUS, headers, answer

radicale/__main__.py

@@ -151,9 +151,6 @@ def serve(configuration, logger):
     atexit.register(cleanup)
     logger.info("Starting Radicale")
 
-    logger.debug(
-        "Base URL prefix: %s", configuration.get("server", "base_prefix"))
-
     # Create collection servers
     servers = {}
     if configuration.getboolean("server", "ssl"):

radicale/config.py

@@ -74,12 +74,6 @@ INITIAL_CONFIG = OrderedDict([
         ("dns_lookup", {
             "value": "True",
             "help": "use reverse DNS to resolve client address in logs"}),
-        ("base_prefix", {
-            "value": "/",
-            "help": "root URL of Radicale, starting and ending with a slash"}),
-        ("can_skip_base_prefix", {
-            "value": "False",
-            "help": "allow URLs cleaned by a HTTP server"}),
         ("realm", {
             "value": "Radicale - Password Required",
             "help": "message displayed when a password is needed"})])),

radicale/xmlutils.py

@@ -31,7 +31,7 @@ import xml.etree.ElementTree as ET
 from collections import OrderedDict
 from datetime import datetime, timedelta, timezone
 from http import client
-from urllib.parse import unquote, urlparse
+from urllib.parse import quote, unquote, urlparse
 
 from . import storage
 
@@ -102,11 +102,9 @@ def _response(code):
     return "HTTP/1.1 %i %s" % (code, client.responses[code])
 
 
-def _href(collection, href):
+def _href(base_prefix, href):
     """Return prefixed href."""
-    return "%s%s" % (
-        collection.configuration.get("server", "base_prefix"),
-        href.lstrip("/"))
+    return quote("%s%s" % (base_prefix, href))
 
 
 def _date_to_datetime(date_):
@@ -425,7 +423,11 @@ def name_from_path(path, collection):
     start = collection.path + "/"
     if not path.startswith(start):
         raise ValueError("'%s' doesn't start with '%s'" % (path, start))
-    return path[len(start):].rstrip("/")
+    name = path[len(start):][:-1]
+    if name and not storage.is_safe_path_component(name):
+        raise ValueError("'%s' is not a component in collection '%s'" %
+                         (path, collection.path))
+    return name
 
 
 def props_from_request(root, actions=("set", "remove")):
@@ -466,7 +468,7 @@ def props_from_request(root, actions=("set", "remove")):
     return result
 
 
-def delete(path, collection, href=None):
+def delete(base_prefix, path, collection, href=None):
     """Read and answer DELETE requests.
 
     Read rfc4918-9.6 for info.
@@ -479,7 +481,7 @@ def delete(path, collection, href=None):
     multistatus.append(response)
 
     href = ET.Element(_tag("D", "href"))
-    href.text = _href(collection, path)
+    href.text = _href(base_prefix, path)
     response.append(href)
 
     status = ET.Element(_tag("D", "status"))
@@ -489,7 +491,8 @@ def delete(path, collection, href=None):
     return _pretty_xml(multistatus)
 
 
-def propfind(path, xml_request, read_collections, write_collections, user):
+def propfind(base_prefix, path, xml_request, read_collections,
+             write_collections, user):
     """Read and answer PROPFIND requests.
 
     Read rfc4918-9.1 for info.
@@ -522,19 +525,19 @@ def propfind(path, xml_request, read_collections, write_collections, user):
     for collection in write_collections:
         collections.append(collection)
         response = _propfind_response(
-            path, collection, props, user, write=True)
+            base_prefix, path, collection, props, user, write=True)
         multistatus.append(response)
     for collection in read_collections:
         if collection in collections:
             continue
         response = _propfind_response(
-            path, collection, props, user, write=False)
+            base_prefix, path, collection, props, user, write=False)
         multistatus.append(response)
 
     return client.MULTI_STATUS, _pretty_xml(multistatus)
 
 
-def _propfind_response(path, item, props, user, write=False):
+def _propfind_response(base_prefix, path, item, props, user, write=False):
     """Build and return a PROPFIND response."""
     is_collection = isinstance(item, storage.BaseCollection)
     if is_collection:
@@ -552,7 +555,7 @@ def _propfind_response(path, item, props, user, write=False):
     else:
         uri = "/" + posixpath.join(collection.path, item.href)
 
-    href.text = _href(collection, uri)
+    href.text = _href(base_prefix, uri)
     response.append(href)
 
     propstat404 = ET.Element(_tag("D", "propstat"))
@@ -574,7 +577,7 @@ def _propfind_response(path, item, props, user, write=False):
             element.text = item.last_modified
         elif tag == _tag("D", "principal-collection-set"):
             tag = ET.Element(_tag("D", "href"))
-            tag.text = _href(collection, "/")
+            tag.text = _href(base_prefix, "/")
             element.append(tag)
         elif (tag in (_tag("C", "calendar-user-address-set"),
                       _tag("D", "principal-URL"),
@@ -582,7 +585,7 @@ def _propfind_response(path, item, props, user, write=False):
                       _tag("C", "calendar-home-set")) and
                 collection.is_principal and is_collection):
             tag = ET.Element(_tag("D", "href"))
-            tag.text = _href(collection, path)
+            tag.text = _href(base_prefix, path)
             element.append(tag)
         elif tag == _tag("C", "supported-calendar-component-set"):
             human_tag = _tag_from_clark(tag)
@@ -600,7 +603,7 @@ def _propfind_response(path, item, props, user, write=False):
                 is404 = True
         elif tag == _tag("D", "current-user-principal"):
             tag = ET.Element(_tag("D", "href"))
-            tag.text = _href(collection, ("/%s/" % user) if user else "/")
+            tag.text = _href(base_prefix, ("/%s/" % user) if user else "/")
             element.append(tag)
         elif tag == _tag("D", "current-user-privilege-set"):
             privilege = ET.Element(_tag("D", "privilege"))
@@ -721,7 +724,7 @@ def _add_propstat_to(element, tag, status_number):
     propstat.append(status)
 
 
-def proppatch(path, xml_request, collection):
+def proppatch(base_prefix, path, xml_request, collection):
     """Read and answer PROPPATCH requests.
 
     Read rfc4918-9.2 for info.
@@ -736,7 +739,7 @@ def proppatch(path, xml_request, collection):
     multistatus.append(response)
 
     href = ET.Element(_tag("D", "href"))
-    href.text = _href(collection, path)
+    href.text = _href(base_prefix, path)
     response.append(href)
 
     for short_name in props_to_remove:
@@ -749,7 +752,7 @@ def proppatch(path, xml_request, collection):
     return _pretty_xml(multistatus)
 
 
-def report(path, xml_request, collection):
+def report(base_prefix, path, xml_request, collection):
     """Read and answer REPORT requests.
 
     Read rfc3253-3.6 for info.
@@ -766,12 +769,15 @@ def report(path, xml_request, collection):
                 _tag("C", "calendar-multiget"),
                 _tag("CR", "addressbook-multiget")):
             # Read rfc4791-7.9 for info
-            base_prefix = collection.configuration.get("server", "base_prefix")
             hreferences = set()
             for href_element in root.findall(_tag("D", "href")):
-                href_path = unquote(urlparse(href_element.text).path)
-                if href_path.startswith(base_prefix):
-                    hreferences.add(href_path[len(base_prefix) - 1:])
+                href_path = storage.sanitize_path(
+                    unquote(urlparse(href_element.text).path))
+                if (href_path + "/").startswith(base_prefix + "/"):
+                    hreferences.add(href_path[len(base_prefix):])
+                else:
+                    collection.logger.info(
+                        "Skipping invalid path: %s", href_path)
         else:
             hreferences = (path,)
         filters = (
@@ -783,12 +789,20 @@ def report(path, xml_request, collection):
     multistatus = ET.Element(_tag("D", "multistatus"))
 
     for hreference in hreferences:
-        name = name_from_path(hreference, collection)
+        try:
+            name = name_from_path(hreference, collection)
+        except ValueError:
+            collection.logger.info("Skipping invalid path: %s", hreference)
+            response = _item_response(base_prefix, hreference,
+                                      found_item=False)
+            multistatus.append(response)
+            continue
         if name:
             # Reference is an item
             item = collection.get(name)
             if not item:
-                response = _item_response(hreference, found_item=False)
+                response = _item_response(base_prefix, hreference,
+                                          found_item=False)
                 multistatus.append(response)
                 continue
             items = [item]
@@ -829,17 +843,18 @@ def report(path, xml_request, collection):
 
             uri = "/" + posixpath.join(collection.path, item.href)
             multistatus.append(_item_response(
-                uri, found_props=found_props,
+                base_prefix, uri, found_props=found_props,
                 not_found_props=not_found_props, found_item=True))
 
     return _pretty_xml(multistatus)
 
 
-def _item_response(href, found_props=(), not_found_props=(), found_item=True):
+def _item_response(base_prefix, href, found_props=(), not_found_props=(),
+                   found_item=True):
     response = ET.Element(_tag("D", "response"))
 
     href_tag = ET.Element(_tag("D", "href"))
-    href_tag.text = href
+    href_tag.text = _href(base_prefix, href)
     response.append(href_tag)
 
     if found_item: