diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f62112d..12e5c97e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 42aa64b6..2d92c7af 100644 --- a/DOCUMENTATION.md +++ b/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 diff --git a/config b/config index 70c2343e..77df29b9 100644 --- a/config +++ b/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 diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index ce948e82..bb418431 100644 --- a/radicale/app/__init__.py +++ b/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.""" diff --git a/radicale/app/base.py b/radicale/app/base.py index 28b6f262..6e3a7cd3 100644 --- a/radicale/app/base.py +++ b/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: diff --git a/radicale/app/put.py b/radicale/app/put.py index d7818eaa..6cfed1eb 100644 --- a/radicale/app/put.py +++ b/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) diff --git a/radicale/config.py b/radicale/config.py index 7693e9e6..a4ba6610 100644 --- a/radicale/config.py +++ b/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", diff --git a/radicale/tests/__init__.py b/radicale/tests/__init__.py index e5ecb1f9..c1a2aab2 100644 --- a/radicale/tests/__init__.py +++ b/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: diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index a9d0acc7..cf9b87d8 100644 --- a/radicale/tests/test_base.py +++ b/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/", """\ + + + + + +""") + 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/")