Merge pull request #1844 from pbiering/fix-1842-storagehooks

Fix 1842 storagehooks

CHANGELOG.md

@@ -13,6 +13,8 @@
 * Fix: add support for query without comp-type
 * Fix: expanded event with dates are missing VALUE=DATE
 * Add: [hook] dryrun: option to disable real hook action for testing, add tests for email+rabbitmq
+* Fix: storage hook path now added to DELETE, MKCOL, MKCALENDAR, MOVE, and PROPPATCH
+* Add: storage hook placeholder now supports "request" and "to_path" (MOVE only)
 
 ## 3.5.4
 * Improve: item filter enhanced for 3rd level supporting VALARM and honoring TRIGGER (offset or absolute)

DOCUMENTATION.md

@@ -1373,6 +1373,8 @@ Supported placeholders:
  - `%(user)s`: logged-in user
  - `%(cwd)s`: current working directory _(>= 3.5.1)_
  - `%(path)s`: full path of item _(>= 3.5.1)_
+ - `%(to_path)s`: full path of destination item (only set on MOVE request) _(>= 3.5.5)_
+ - `%(request)s`: request method _(>= 3.5.5)_
 
 Command will be executed with base directory defined in `filesystem_folder` (see above)
 

config

@@ -234,9 +234,12 @@
 #   %(user)s: logged-in user
 #   %(cwd)s : current working directory
 #   %(path)s: full path of item
+#   %(to_path)s: full path of destination item (only set on MOVE request)
+#   %(request)s: request method
 #  Command will be executed with base directory defined in filesystem_folder
 #  For "git" check DOCUMENTATION.md for bootstrap instructions
 # Example(test): echo \"user=%(user)s path=%(path)s cwd=%(cwd)s\"
+# Example(test/json): echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\"
 # Example(git): git add -A && (git diff --cached --quiet || git commit -m "Changes by \"%(user)s\"")
 #hook =
 

radicale/app/delete.py

@@ -2,8 +2,8 @@
 # Copyright © 2008 Nicolas Kandel
 # Copyright © 2008 Pascal Halter
 # Copyright © 2008-2017 Guillaume Ayoub
-# Copyright © 2017-2018 Unrud <unrud@outlook.com>
+# Copyright © 2017-2020 Unrud <unrud@outlook.com>
-# Copyright © 2024-2024 Peter Bieringer <pb@bieringer.de>
+# Copyright © 2024-2025 Peter Bieringer <pb@bieringer.de>
 #
 # 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
@@ -60,7 +60,7 @@ class ApplicationPartDelete(ApplicationBase):
         access = Access(self._rights, user, path)
         if not access.check("w"):
             return httputils.NOT_ALLOWED
-        with self._storage.acquire_lock("w", user):
+        with self._storage.acquire_lock("w", user, path=path, request="DELETE"):
             item = next(iter(self._storage.discover(path)), None)
             if not item:
                 return httputils.NOT_FOUND

radicale/app/mkcalendar.py

@@ -57,7 +57,7 @@ class ApplicationPartMkcalendar(ApplicationBase):
             return httputils.BAD_REQUEST
         # TODO: use this?
         # timezone = props.get("C:calendar-timezone")
-        with self._storage.acquire_lock("w", user):
+        with self._storage.acquire_lock("w", user, path=path, request="MKCALENDAR"):
             item = next(iter(self._storage.discover(path)), None)
             if item:
                 return self._webdav_error_response(

radicale/app/mkcol.py

@@ -62,7 +62,7 @@ class ApplicationPartMkcol(ApplicationBase):
         if not props.get("tag") and "W" not in permissions:
             logger.warning("MKCOL request %r (type:%s): %s", path, collection_type, "rejected because of missing rights 'W'")
             return httputils.NOT_ALLOWED
-        with self._storage.acquire_lock("w", user):
+        with self._storage.acquire_lock("w", user, path=path, request="MKCOL"):
             item = next(iter(self._storage.discover(path)), None)
             if item:
                 return httputils.METHOD_NOT_ALLOWED

radicale/app/move.py

@@ -73,7 +73,7 @@ class ApplicationPartMove(ApplicationBase):
         if not to_access.check("w"):
             return httputils.NOT_ALLOWED
 
-        with self._storage.acquire_lock("w", user):
+        with self._storage.acquire_lock("w", user, path=path, request="MOVE", to_path=to_path):
             item = next(iter(self._storage.discover(path)), None)
             if not item:
                 return httputils.NOT_FOUND

radicale/app/proppatch.py

@@ -87,7 +87,7 @@ class ApplicationPartProppatch(ApplicationBase):
         except socket.timeout:
             logger.debug("Client timed out", exc_info=True)
             return httputils.REQUEST_TIMEOUT
-        with self._storage.acquire_lock("w", user):
+        with self._storage.acquire_lock("w", user, path=path, request="PROPPATCH"):
             item = next(iter(self._storage.discover(path)), None)
             if not item:
                 return httputils.NOT_FOUND

radicale/app/put.py

@@ -174,7 +174,7 @@ class ApplicationPartPut(ApplicationBase):
              bool(rights.intersect(access.permissions, "Ww")),
              bool(rights.intersect(access.parent_permissions, "w")))
 
-        with self._storage.acquire_lock("w", user, path=path):
+        with self._storage.acquire_lock("w", user, path=path, request="PUT"):
             item = next(iter(self._storage.discover(path)), None)
             parent_item = next(iter(
                 self._storage.discover(access.parent_path)), None)

radicale/storage/multifilesystem/lock.py

@@ -78,10 +78,16 @@ class StoragePartLock(StorageBase):
                     preexec_fn = os.setpgrp
                 # optional argument
                 path = kwargs.get('path', "")
+                request = kwargs.get('request', "NONE")
+                to_path = kwargs.get('to_path', "")
+                if to_path != "":
+                    to_path = shlex.quote(self._get_collection_root_folder() + to_path)
                 try:
                     command = self._hook % {
                         "path": shlex.quote(self._get_collection_root_folder() + path),
+                        "to_path": to_path,
                         "cwd": shlex.quote(self._filesystem_folder),
+                        "request": shlex.quote(request),
                         "user": shlex.quote(user or "Anonymous")}
                 except KeyError as e:
                     logger.error("Storage hook contains not supported placeholder %s (skip execution of: %r)" % (e, self._hook))

radicale/tests/test_storage.py

@@ -1,7 +1,7 @@
 # This file is part of Radicale - CalDAV and CardDAV server
 # Copyright © 2012-2017 Guillaume Ayoub
 # Copyright © 2017-2022 Unrud <unrud@outlook.com>
-# Copyright © 2024-2024 Peter Bieringer <pb@bieringer.de>
+# Copyright © 2024-2025 Peter Bieringer <pb@bieringer.de>
 #
 # 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
@@ -21,7 +21,10 @@ Tests for storage backends.
 
 """
 
+import json
+import logging
 import os
+import re
 import shutil
 from typing import ClassVar, cast
 
@@ -71,8 +74,7 @@ class TestMultiFileSystem(BaseTest):
         self.propfind("/")
         self.propfind("/created_by_hook/", check=404)
 
-    @pytest.mark.skipif(not shutil.which("flock"),
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
-                        reason="flock command not found")
     def test_hook_storage_locked(self) -> None:
         """Verify that the storage is locked when the hook runs."""
         self.configure({"storage": {"hook": (
@@ -186,6 +188,247 @@ class TestMultiFileSystem(BaseTest):
             assert answer is not None
             assert "\r\nUID:%s\r\n" % uid in answer
 
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_PUT(self, caplog) -> None:
+        """Run hook and check placeholders: PUT"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcalendar("/calendar.ics/")
+        event = get_file_content("event1.ics")
+        path = "/calendar.ics/event1.ics"
+        self.put(path, event)
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/calendar.ics/event1.ics":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "PUT":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == "":
+                        found = found | 256
+                else:
+                    found = found | 256 | 32
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, d)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_DELETE(self, caplog) -> None:
+        """Run hook and check placeholders: DELETE"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcalendar("/calendar.ics/")
+        event = get_file_content("event1.ics")
+        path = "/calendar.ics/event1.ics"
+        self.put(path, event)
+        self.delete(path)
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/calendar.ics/event1.ics":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "DELETE":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == "":
+                        found = found | 256
+                else:
+                    found = found | 256 | 32
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, s)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_MKCALENDAR(self, caplog) -> None:
+        """Run hook and check placeholders: MKCALENDAR"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcalendar("/calendar.ics/")
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/calendar.ics/":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "MKCALENDAR":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == "":
+                        found = found | 256
+                else:
+                    found = found | 256 | 32
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, d)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_MKCOL(self, caplog) -> None:
+        """Run hook and check placeholders: MKCOL"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcol("/user1/")
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/user1/":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "MKCOL":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == "":
+                        found = found | 256
+                else:
+                    found = found | 256 | 32
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, d)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_PROPPATCH(self, caplog) -> None:
+        """Run hook and check placeholders: PROPPATCH"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcalendar("/calendar.ics/")
+        proppatch = get_file_content("proppatch_set_calendar_color.xml")
+        _, responses = self.proppatch("/calendar.ics/", proppatch)
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/calendar.ics/":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "PROPPATCH":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == "":
+                        found = found | 256
+                else:
+                    found = found | 256 | 32
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, d)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
+    @pytest.mark.skipif(not shutil.which("flock"), reason="flock command not found")
+    def test_hook_placeholders_MOVE(self, caplog) -> None:
+        """Run hook and check placeholders: MOVE"""
+        self.configure({"storage": {"hook": "echo \"hook-json {'user':'%(user)s', 'cwd':'%(cwd)s', 'path':'%(path)s', 'request':'%(request)s', 'to_path':'%(to_path)s'}\""}})
+        found = 0
+        self.mkcalendar("/calendar.ics/")
+        event = get_file_content("event1.ics")
+        path1 = "/calendar.ics/event1.ics"
+        path2 = "/calendar.ics/event2.ics"
+        self.put(path1, event)
+        self.request("MOVE", path1, check=201,
+                     HTTP_DESTINATION="http://127.0.0.1/"+path2)
+        for line in caplog.messages:
+            if line.find("\"hook-json ") != -1:
+                found = 1
+                r = re.search('.*\"hook-json ({.*})".*', line)
+                if r:
+                    s = r.group(1).replace("'", "\"")
+                else:
+                    break
+                d = json.loads(s)
+                if d["user"] == "Anonymous":
+                    found = found | 2
+                if d["cwd"]:
+                    found = found | 4
+                if d["path"]:
+                    found = found | 8
+                    if d["path"] == d["cwd"] + "/collection-root/calendar.ics/event1.ics":
+                        found = found | 16
+                if d["request"]:
+                    found = found | 64
+                    if d["request"] == "MOVE":
+                        found = found | 128
+                if d["to_path"]:
+                    found = found | 32
+                    if d["to_path"] == d["cwd"] + "/collection-root/calendar.ics/event2.ics":
+                        found = found | 256
+        if (found != 511):
+            raise ValueError("Logging misses expected hook log line, found=%d data=%r", found, d)
+        else:
+            logging.info("Logging contains expected hook line, found=%d data=%r", found, d)
+
 
 class TestMultiFileSystemNoLock(BaseTest):
     """Tests for multifilesystem_nolock."""