Merge pull request #2043 from pbiering/check-max-resource-size-support-2039

Check max_resource_size on storage/discover+get

CHANGELOG.md

@@ -16,6 +16,7 @@
 * Add: [server] delay_on_error option
 * Add: [logging] limit_content option
 * Add: [headers] Content-Security-Policy is now set to be strict on new configs
+* Improve: [server] max_resource_size is now also checked on storage discover+get to avoid issues with items stored outside using PUT
 
 ## 3.6.1
 

radicale/app/put.py

@@ -105,11 +105,11 @@ def prepare(vobject_items: List[vobject.base.Component], path: str,
                         raise
                     size = len(item.serialize())
                     if (size > max_resource_size):
-                        logger.warning("PUT request contains item with UID %r size %d > limit %d: %r", item.uid, size, max_resource_size, path)
+                        logger.warning("PUT request contains item with UID %r size %s > limit %s: %r", item.uid, utils.format_unit(size, binary=True), utils.format_unit(max_resource_size, binary=True), path)
                         # Use OverflowError as flag for max_resource_size
                         raise OverflowError
                     else:
-                        logger.debug("PUT request contains item with UID %r size %d <= limit %d: %r", item.uid, size, max_resource_size, path)
+                        logger.debug("PUT request contains item with UID %r size %s <= limit %s: %r", item.uid, utils.format_unit(size, binary=True), utils.format_unit(max_resource_size, binary=True), path)
                     items.append(item)
             elif write_whole_collection and tag == "VADDRESSBOOK":
                 for vobject_item in vobject_items:

radicale/pathutils.py

@@ -32,6 +32,7 @@ from tempfile import TemporaryDirectory
 from typing import Iterator, Type, Union
 
 from radicale import storage, types, utils
+from radicale.log import logger
 
 if sys.platform == "win32":
     import ctypes
@@ -355,3 +356,12 @@ def path_permissions_as_string(path):
     pp = path_permissions(path)
     s = "path=%r owner=%s(%s) group=%s(%s) mode=%s" % (path, pp[0], pp[1], pp[2], pp[3], pp[4])
     return s
+
+
+def file_check_size(path: str, limit: int):
+    if os.path.isfile(path):
+        size = os.stat(path).st_size
+        if size > limit:
+            logger.warning("file skipped because size exceeds limit %s > %s: %r", utils.format_unit(size, binary=True), utils.format_unit(limit, binary=True), path)
+            return False
+    return True

radicale/storage/multifilesystem/base.py

@@ -78,6 +78,7 @@ class StorageBase(storage.BaseStorage):
     _debug_cache_actions: bool
     _folder_umask: str
     _config_umask: int
+    _max_resource_size: int
 
     def __init__(self, configuration: config.Configuration) -> None:
         super().__init__(configuration)
@@ -99,6 +100,8 @@ class StorageBase(storage.BaseStorage):
             "storage", "folder_umask")
         self._debug_cache_actions = configuration.get(
             "logging", "storage_cache_actions_on_debug")
+        self._max_resource_size = configuration.get(
+            "server", "max_resource_size")
 
     def _get_collection_root_folder(self) -> str:
         return os.path.join(self._filesystem_folder, "collection-root")

radicale/storage/multifilesystem/discover.py

@@ -77,7 +77,8 @@ class StoragePartDiscover(StorageBase):
         if href:
             item = collection._get(href)
             if item is not None:
-                yield item
+                if pathutils.file_check_size(filesystem_path, self._max_resource_size):
+                    yield item
             return
 
         yield collection

radicale/storage/multifilesystem/get.py

@@ -175,7 +175,8 @@ class CollectionPartGet(CollectionPartCache, CollectionPartLock,
                              href)
                 yield (href, None)
             else:
-                yield (href, self._get(href, verify_href=False))
+                if pathutils.file_check_size(path, self._storage._max_resource_size):
+                    yield (href, self._get(href, verify_href=False))
 
     def get_all(self) -> Iterator[radicale_item.Item]:
         for href in self._list():
@@ -183,4 +184,5 @@ class CollectionPartGet(CollectionPartCache, CollectionPartLock,
             # are from os.listdir.
             item = self._get(href, verify_href=False)
             if item is not None:
-                yield item
+                if pathutils.file_check_size(os.path.join(self._filesystem_path, href), self._storage._max_resource_size):
+                    yield item

radicale/tests/test_base.py

@@ -193,6 +193,38 @@ permissions: RrWw""")
         path = "/calendar.ics/"
         self.put(path, event, check=412)
 
+    def test_get_vcard_exceed_size(self) -> None:
+        """Add vcards partially exceeding max-resource-size (adjusted after upload)."""
+        path_base = "/contacts.vcf/"
+        file1 = "contact1.vcf"
+        file2 = "contact_photo_with_data_uri.vcf"
+        self.create_addressbook(path_base)
+        contact1 = get_file_content(file1)
+        contact2 = get_file_content(file2)
+        path1 = path_base + file1
+        path2 = path_base + file2
+        self.put(path1, contact1)
+        self.put(path2, contact2)
+        self.configure({"server": {"max_resource_size": 100}})
+        # test "get"
+        self.get(path1, check=200)
+        self.get(path2, check=404)
+        # test "report"
+        _, responses = self.report(path_base, """\
+<?xml version="1.0"?>
+<CR:addressbook-multiget xmlns="DAV:" xmlns:CR="urn:ietf:params:xml:ns:carddav">
+   <prop>
+     <getetag />
+   </prop>
+   <href>""" + path_base + """</href>
+</CR:addressbook-multiget>""")
+        logging.info("response: %r", responses)
+        assert len(responses) == 1
+        response = responses[path1]
+        assert isinstance(response, dict)
+        status, prop = response["D:getetag"]
+        assert status == 200 and prop.text
+
     def test_add_event_broken(self) -> None:
         """Add a broken event."""
         self.mkcalendar("/calendar.ics/")

radicale/tests/test_server.py

@@ -1,5 +1,5 @@
 # This file is part of Radicale - CalDAV and CardDAV server
-# Copyright © 2018-2019 Unrud <unrud@outlook.com>
+# Copyright © 2018-2022 Unrud <unrud@outlook.com>
 #
 # This library is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

radicale/utils.py

@@ -378,7 +378,10 @@ def format_unit(value: float, binary: bool = False) -> str:
             unit = "k"
         else:
             unit = ""
-    return ("%.1f %s" % (value, unit))
+    if unit == "":
+        return ("%d" % value)
+    else:
+        return ("%.1f %s" % (value, unit))
 
 
 def limit_str(content: str, limit: int) -> str: