Merge pull request #1898 from pbiering/issue-1897-strict_precondition

Issue 1897 strict preconditions

CHANGELOG.md

@@ -4,6 +4,7 @@
 * Extend [auth]: re-factor & overhaul LDAP authentication, especially for Python's ldap module
 * Fix: out-of-range timestamp on 32-bit systems
 * Feature: extend logging with response size in bytes and flag served as plain or gzip
+* Feature: [storage] strict_preconditions: new config option to enforce strict preconditions check on PUT in case item already exists [RFC6352#9.2]
 
 ## 3.5.7
 * Extend: [auth] dovecot: add support for version >= 2.4

DOCUMENTATION.md

@@ -1523,6 +1523,14 @@ Skip broken item instead of triggering an exception
 
 Default: `True`
 
+##### strict_preconditions
+
+_(>= 3.5.8)_
+
+Strict preconditions check on PUT in case item already exists [RFC6352#9.2](https://www.rfc-editor.org/rfc/rfc6352#section-9.2)
+
+Default: `False`
+
 ##### hook
 
 Command that is run after changes to storage. See the

config

@@ -242,6 +242,9 @@
 # Skip broken item instead of triggering an exception
 #skip_broken_item = True
 
+# Strict preconditions check on PUT
+#strict_preconditions = False
+
 # Command that is run after changes to storage, default is emtpy
 #  Supported placeholders:
 #   %(user)s: logged-in user

radicale/app/__init__.py

@@ -73,8 +73,6 @@ class Application(ApplicationPartDelete, ApplicationPartHead,
     _web_type: str
     _script_name: str
     _extra_headers: Mapping[str, str]
-    _permit_delete_collection: bool
-    _permit_overwrite_collection: bool
 
     def __init__(self, configuration: config.Configuration) -> None:
         """Initialize Application.
@@ -116,6 +114,8 @@ class Application(ApplicationPartDelete, ApplicationPartHead,
         self._extra_headers = dict()
         for key in self.configuration.options("headers"):
             self._extra_headers[key] = configuration.get("headers", key)
+        self._strict_preconditions = configuration.get("storage", "strict_preconditions")
+        logger.info("strict preconditions check: %s", self._strict_preconditions)
 
     def _scrub_headers(self, environ: types.WSGIEnviron) -> types.WSGIEnviron:
         """Mask passwords and cookies."""

radicale/app/base.py

@@ -41,6 +41,7 @@ class ApplicationBase:
     _encoding: str
     _permit_delete_collection: bool
     _permit_overwrite_collection: bool
+    _strict_preconditions: bool
     _hook: hook.BaseHook
 
     def __init__(self, configuration: config.Configuration) -> None:

radicale/app/put.py

@@ -207,6 +207,9 @@ class ApplicationPartPut(ApplicationBase):
                 return httputils.NOT_ALLOWED
 
             etag = environ.get("HTTP_IF_MATCH", "")
+            if item and not etag and self._strict_preconditions:
+                logger.warning("Precondition failed for %r: existing item, no If-Match header, strict mode enabled", path)
+                return httputils.PRECONDITION_FAILED
             if not item and etag:
                 # Etag asked but no item found: item has been removed
                 logger.warning("Precondition failed on PUT request for %r (HTTP_IF_MATCH: %s, item not existing)", path, etag)

radicale/config.py

@@ -430,6 +430,10 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([
             "value": "",
             "help": "command that is run after changes to storage",
             "type": str}),
+        ("strict_preconditions", {
+            "value": "False",
+            "help": "strict preconditions check on PUT",
+            "type": bool}),
         ("_filesystem_fsync", {
             "value": "True",
             "help": "sync all changes to filesystem during requests",

radicale/tests/__init__.py

@@ -75,6 +75,10 @@ class BaseTest:
         if login is not None and not isinstance(login, str):
             raise TypeError("login argument must be %r, not %r" %
                             (str, type(login)))
+        http_if_match = kwargs.pop("http_if_match", None)
+        if http_if_match is not None and not isinstance(http_if_match, str):
+            raise TypeError("http_if_match argument must be %r, not %r" %
+                            (str, type(http_if_match)))
         environ: Dict[str, Any] = {k.upper(): v for k, v in kwargs.items()}
         for k, v in environ.items():
             if not isinstance(v, str):
@@ -84,6 +88,8 @@ class BaseTest:
         if login:
             environ["HTTP_AUTHORIZATION"] = "Basic " + base64.b64encode(
                     login.encode(encoding)).decode()
+        if http_if_match:
+            environ["HTTP_IF_MATCH"] = http_if_match
         environ["REQUEST_METHOD"] = method.upper()
         environ["PATH_INFO"] = path
         if data is not None:

radicale/tests/test_base.py

@@ -229,6 +229,40 @@ permissions: RrWw""")
         _, answer = self.get(path)
         assert "DTSTAMP:20130902T150159Z" in answer
 
+    def test_update_event_no_etag_strict_preconditions_true(self) -> None:
+        """Update an event without serving etag."""
+        self.configure({"storage": {"strict_preconditions": True}})
+        self.mkcalendar("/calendar.ics/")
+        event = get_file_content("event1.ics")
+        event_modified = get_file_content("event1_modified.ics")
+        path = "/calendar.ics/event1.ics"
+        self.put(path, event, check=201)
+        self.put(path, event_modified, check=412)
+
+    def test_update_event_with_etag_strict_preconditions_true(self) -> None:
+        """Update an event with serving etag."""
+        self.configure({"storage": {"strict_preconditions": True}})
+        self.configure({"logging": {"response_content_on_debug": True}})
+        self.mkcalendar("/calendar.ics/")
+        event = get_file_content("event1.ics")
+        event_modified = get_file_content("event1_modified.ics")
+        path = "/calendar.ics/event1.ics"
+        self.put(path, event, check=201)
+        # get etag
+        _, responses = self.report("/calendar.ics/", """\
+<?xml version="1.0" encoding="utf-8" ?>
+<C:calendar-query xmlns:C="urn:ietf:params:xml:ns:caldav">
+    <D:prop xmlns:D="DAV:">
+        <D:getetag/>
+    </D:prop>
+</C:calendar-query>""")
+        assert len(responses) == 1
+        response = responses["/calendar.ics/event1.ics"]
+        assert not isinstance(response, int)
+        status, prop = response["D:getetag"]
+        assert status == 200 and prop.text
+        self.put(path, event_modified, check=204, http_if_match=prop.text)
+
     def test_update_event_uid_event(self) -> None:
         """Update an event with a different UID."""
         self.mkcalendar("/calendar.ics/")