Merge pull request #2098 from pbiering/regression-fixes

Regression fixes

Author: pbiering

.github/workflows/test.yml

@@ -85,6 +85,24 @@ jobs:
       - name: Test with newest Python on latest Ubuntu using VFAT
         run: tox -c pyproject.toml -e py_filesystem_vfat
 
+  test-ubuntu-python-newest-with-vfat-utf8:
+    name: Test VFAT UTF-8 Python:newest Ubuntu:latest
+    needs: [lint, test-ubuntu-python-newest, test-ubuntu-python-newest-with-vfat]
+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        python-version: ['3.14']
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v5
+      - uses: actions/setup-python@v6
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install Test dependencies
+        run: pip install tox
+      - name: Test with newest Python on latest Ubuntu using VFAT UTF-8
+        run: tox -c pyproject.toml -e py_filesystem_vfat_utf8
+
   test-ubuntu-python-newest-with-ntfs:
     name: Test NTFS Python:newest Ubuntu:latest
     needs: [lint, test-ubuntu-python-newest]
@@ -163,7 +181,7 @@ jobs:
 
   test-otheros-python-newest:
     name: Test MacOS/Windows:latest Python:newest
-    needs: [lint, test-ubuntu-python-newest, test-ubuntu-python-newest-with-ntfs, test-ubuntu-python-newest-with-vfat]
+    needs: [lint, test-ubuntu-python-newest, test-ubuntu-python-newest-with-ntfs, test-ubuntu-python-newest-with-vfat, test-ubuntu-python-newest-with-vfat-utf8]
     strategy:
       matrix:
         os: [macos-latest, windows-latest]
@@ -310,7 +328,7 @@ jobs:
       - uses: actions/checkout@v5
       - uses: actions/setup-python@v6
         with:
-          python-version: '3.12'
+          python-version: '3.14'
       - name: Install uv
         run: pip install uv
       - name: Install Playwright Browsers

CHANGELOG.md

@@ -8,6 +8,11 @@
 * Improve: application will stop on startup if TEMP is provided but not existing or not writable
 * Extension: tox with new optional test cases to test with LinuxOS vfat, hfsplus, ntfs filesystems
 * Adjust: respond with 500 in case principal collection cannot be created (e.g. filesystem issues)
+* Improve: sharing supports now unicode
+* Add: [server] new options validate_user_value/validate_path_value for ability to block unwanted values (autoenable "strict" on non-unicode filesystem)
+* Adjust: several log levels incl. final result depending on status code
+* Fix: sharing/csv: quote handling and honor stock encoding
+* Extension: sharing: add ICAL:calendar-order to property overlay whitelist
 
 ## 3.7.1
 * Fix: share address book collection as birthday calendar not working on non-DEBUG level

DOCUMENTATION.md

@@ -1617,6 +1617,35 @@ Strict preconditions check on PUT in case item already exists [RFC6352#9.2](http
 
 Default: `False`
 
+##### validate_user_value
+
+_(>= 3.7.2)_
+
+Validate user value content
+
+Available types are:
+* `none`
+* `minimal` (control and some special chars)
+* `unicode-letter` (unicode letters)
+* `no-unicode` (no unicode)
+* `strict` (reduced ASCII set)
+
+Default: `minimum`
+
+##### validate_path_type
+
+_(>= 3.7.2)_
+
+Validate path value content
+
+* `none`
+* `minimal` (control and some special chars)
+* `unicode-letter` (unicode letters)
+* `no-unicode` (no unicode)
+* `strict` (reduced ASCII set)
+
+Default: `minimum`
+
 ##### hook
 
 Command that is run after changes to storage. See the

config

@@ -58,6 +58,14 @@
 # script name to strip from URI if called by reverse proxy
 #script_name = (default taken from HTTP_X_SCRIPT_NAME or SCRIPT_NAME)
 
+# validate user type
+# Value: none|minimal|unicode-letter|no-unicode|strict
+#validate_user_value = minimal
+
+# validate path value
+# Value: none|minimal|unicode-letter|no-unicode|strict
+#validate_path_value = minimal
+
 
 [encoding]
 
@@ -424,6 +432,7 @@
 # This may become the default in future versions, override if you need a different CSP.
 Content-Security-Policy = default-src 'self'; object-src 'none'
 
+
 [hook]
 
 # Hook types

pyproject.toml

@@ -101,7 +101,7 @@ commands_pre = [
     # unconditionally create mount point
     ["mkdir", "-p", "/tmp/vfat"],
     # mount image
-    ["sudo", "/usr/bin/mount", "-o", "loop,umask=000", "/tmp/vfat.img", "/tmp/vfat"],
+    ["sudo", "/usr/bin/mount", "-o", "loop,umask=000,utf8=no", "/tmp/vfat.img", "/tmp/vfat"],
 ]
 setenv = { TEMP = "/tmp/vfat" }
 commands = [["pytest", "-r", "s", "."]]
@@ -114,6 +114,33 @@ commands_post = [
     ["rm", "/tmp/vfat.img"]
 ]
 
+[tool.tox.env.py_filesystem_vfat_utf8]
+allowlist_externals = [ "sudo", "dd", "mkfs.vfat", "mkdir", "rm", "rmdir", "chmod" ]
+extras = ["test"]
+deps = [
+    "pytest"
+]
+commands_pre = [
+    # create 64 MByte disk image
+    ["dd", "if=/dev/zero", "of=/tmp/vfat-utf8.img", "bs=1M", "count=64"],
+    # create file system
+    ["mkfs.vfat", "/tmp/vfat-utf8.img", "-n", "VFAT"],
+    # unconditionally create mount point
+    ["mkdir", "-p", "/tmp/vfat-utf8"],
+    # mount image
+    ["sudo", "/usr/bin/mount", "-o", "loop,umask=000,utf8", "/tmp/vfat-utf8.img", "/tmp/vfat-utf8"],
+]
+setenv = { TEMP = "/tmp/vfat-utf8" }
+commands = [["pytest", "-r", "s", "."]]
+commands_post = [
+    # umount image
+    ["sudo", "/usr/bin/umount", "-d", "/tmp/vfat-utf8"],
+    # remove mount point
+    ["rmdir", "/tmp/vfat-utf8"],
+    # remove image
+    ["rm", "/tmp/vfat-utf8.img"]
+]
+
 [tool.tox.env.py_filesystem_hfsplus]
 # Fedora
 allowlist_externals = [ "sudo", "dd", "mkfs.hfsplus", "mkdir", "rm", "rmdir", "chmod" ]

radicale/app/__init__.py

@@ -42,6 +42,7 @@
 from typing import Iterable, List, Mapping, Tuple, Union
 
 from radicale import config, httputils, log, pathutils, types, utils
+from radicale.app import base as app_base
 from radicale.app.base import ApplicationBase
 from radicale.app.delete import ApplicationPartDelete
 from radicale.app.get import ApplicationPartGet
@@ -86,6 +87,8 @@ class Application(ApplicationPartDelete, ApplicationPartHead,
     _profiling_per_request: bool = False
     _profiling_per_request_method: bool = False
     _limit_content: int
+    _validate_user_value: str
+    _validate_path_value: str
     profiler_per_request_method: dict[str, cProfile.Profile] = {}
     profiler_per_request_method_counter: dict[str, int] = {}
     profiler_per_request_method_starttime: datetime.datetime
@@ -158,6 +161,19 @@ def __init__(self, configuration: config.Configuration) -> None:
             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)
+        # Format checks
+        self._validate_user_value = configuration.get("server", "validate_user_value")
+        self._validate_path_value = configuration.get("server", "validate_path_value")
+        if not self._storage._supports_unicode:
+            if self._validate_user_value not in ["strict", "no-unicode"]:
+                self._validate_user_value = "no-unicode"
+            if self._validate_path_value not in ["strict", "no-unicode"]:
+                self._validate_path_value = "no-unicode"
+            logger.notice("validate user value: %r (enforced by limited support of collection storage)", self._validate_user_value)
+            logger.notice("validate path value: %r (enforced by limited support of collection storage)", self._validate_path_value)
+        else:
+            logger.info("validate user value: %r", self._validate_user_value)
+            logger.info("validate path value: %r", self._validate_path_value)
         # Profiling options
         self._profiling = configuration.get("logging", "profiling")
         self._profiling_per_request_min_duration = configuration.get("logging", "profiling_per_request_min_duration")
@@ -338,15 +354,23 @@ def response(status: int, headers: types.WSGIResponseHeaders,
             else:
                 flags_text = ""
             if answer is not None:
-                logger.info("%s response status for %r%s in %.3f seconds %s %s bytes%s: %s",
+                message = "%s response status for %r%s in %.3f seconds %s %s bytes%s: %s" % (
                             request_method, unsafe_path, depthinfo,
                             time_delta_seconds, content_encoding, str(len(answer)),
                             flags_text,
                             status_text)
             else:
-                logger.info("%s response status for %r%s in %.3f seconds: %s",
+                message = "%s response status for %r%s in %.3f seconds: %s" % (
                             request_method, unsafe_path, depthinfo,
                             time_delta_seconds, status_text)
+            logger_method = logger.info  # default
+            if status == 401 or status == 404 or status == 412 or status == 409 or request_method in ["PROPPATCH", "MKCALENDAR", "MKCOL"]:
+                logger_method = logger.notice
+            elif status >= 500 and status < 500:
+                logger_method = logger.error
+            elif status >= 500:
+                logger_method = logger.critical
+            logger_method(message)
 
             # Profiling end
             if self._profiling_per_request:
@@ -459,6 +483,9 @@ def response(status: int, headers: types.WSGIResponseHeaders,
                     logger.warning("Called by reverse proxy, cannot remove base prefix %r from path: %r as not matching (may cause authentication issues using internal WebUI)", base_prefix, path)
                 else:
                     logger.debug("Called by reverse proxy, cannot remove base prefix %r from path: %r as not matching", base_prefix, path)
+        if not app_base._check_path_format(self._storage, path, self._validate_path_value):
+            logger.error("request contains invalid path: %r (not compliant to %r)", path, self._validate_path_value)
+            return response(*httputils.BAD_REQUEST)
 
         # Get function corresponding to method
         function = getattr(self, "do_%s" % request_method, None)
@@ -489,7 +516,11 @@ def response(status: int, headers: types.WSGIResponseHeaders,
                 self.configuration, environ, base64.b64decode(
                     authorization.encode("ascii"))).split(":", 1)
 
-        (user, info) = self._auth.login(login, password, context) or ("", "") if login else ("", "")
+        if login and not app_base._check_user_format(self._storage, login, self._validate_user_value):
+            info = "not compliant to %r" % self._validate_user_value
+            user = ""
+        else:
+            (user, info) = self._auth.login(login, password, context) or ("", "") if login else ("", "")
         if self.configuration.get("auth", "type") == "ldap":
             try:
                 logger.debug("Groups received from LDAP: %r", ",".join(self._auth._ldap_groups))
@@ -600,9 +631,9 @@ def response(status: int, headers: types.WSGIResponseHeaders,
 
             if (status, headers, answer, xml_request) == httputils.NOT_ALLOWED:
                 if path.startswith("/.token"):
-                    logger.info("Access to %r denied", path)
+                    logger.notice("Access to %r denied", path)
                 else:
-                    logger.info("Access to %r denied for %s", path, repr(user) if user else "anonymous user")
+                    logger.notice("Access to %r denied for %s", path, repr(user) if user else "anonymous user")
         else:
             status, headers, answer, xml_request = httputils.NOT_ALLOWED
 

radicale/app/base.py

@@ -17,7 +17,9 @@
 
 import io
 import logging
+import re
 import sys
+import unicodedata
 import xml.etree.ElementTree as ET
 from typing import Optional, Union
 
@@ -30,6 +32,82 @@
 import defusedxml.ElementTree as DefusedET  # isort:skip
 sys.modules["xml.etree"].ElementTree = ET  # type:ignore[attr-defined]
 
+USER_PATTERN_STRICT: str = "a-zA-Z0-9@\\.\\-_"
+PATH_PATTERN_STRICT: str = USER_PATTERN_STRICT + "\\/~"  # / as separator
+
+USER_PATTERN_STRICT_RE: str = "^[" + USER_PATTERN_STRICT + "]+$"
+PATH_PATTERN_STRICT_RE: str = "^[" + PATH_PATTERN_STRICT + "]+$"
+
+USER_BLACKLIST_MINIMAL: list = [":", "'", '"', '*', '?']
+PATH_BLACKLIST_MINIMAL: list = USER_BLACKLIST_MINIMAL
+
+USER_WHITELIST_UNICODE: list = ["-", ".", "@", "_"]  # from USER_PATTERN_STRICT
+PATH_WHITELIST_UNICODE: list = ["-", ".", "@", "_", "/", "~"]  # from PATH_PATTERN_STRICT
+
+
+def _check_format(self: storage.BaseStorage,
+                  string: str,
+                  blacklist_minimal: list[str],
+                  whitelist_unicode: list[str],
+                  validation_type: str,
+                  ) -> bool:
+    check_minimal = (validation_type == "minimal")
+    check_unicode_letter = (validation_type == "unicode-letter")
+    check_no_unicode = (validation_type == "no-unicode")
+    logger.trace("_check_format investigate %r (validation_type=%r check_minimal=%s check_unicode_letter=%s check_no_unicode=%s)", string, validation_type, check_minimal, check_unicode_letter, check_no_unicode)
+    if not self._supports_trailing_whitespace and string.endswith(' '):
+        return False
+    for c in string:
+        if c <= chr(31) or (c >= chr(127) and c <= chr(159)):
+            # ASCII: control char
+            return False
+        if unicodedata.category(c)[0] == "C":
+            # https://unicodeplus.com/category
+            # Unicode: control
+            return False
+        if check_minimal or not self._supports_problematic_chars:
+            if c in blacklist_minimal:
+                logger.trace("_check_format found %r", c)
+                return False
+        if check_unicode_letter:
+            if c not in whitelist_unicode:
+                if unicodedata.category(c)[0] != "L":
+                    return False
+        if check_no_unicode:
+            if ord(c) > 255:
+                return False
+    return True
+
+
+def _check_user_format(self: storage.BaseStorage,
+                       user: str,
+                       validation_type: str
+                       ) -> bool:
+    if validation_type == "strict":
+        return (re.search(USER_PATTERN_STRICT_RE, user) is not None)
+    else:
+        return _check_format(self,
+                             user,
+                             USER_BLACKLIST_MINIMAL,
+                             USER_WHITELIST_UNICODE,
+                             validation_type,
+                             )
+
+
+def _check_path_format(self: storage.BaseStorage,
+                       path: str,
+                       validation_type: str
+                       ) -> bool:
+    if validation_type == "strict":
+        return (re.search(PATH_PATTERN_STRICT_RE, path) is not None)
+    else:
+        return _check_format(self,
+                             path,
+                             PATH_BLACKLIST_MINIMAL,
+                             PATH_WHITELIST_UNICODE,
+                             validation_type,
+                             )
+
 
 class ApplicationBase:
 
@@ -44,6 +122,8 @@ class ApplicationBase:
     _permit_delete_collection: bool
     _permit_overwrite_collection: bool
     _strict_preconditions: bool
+    _validate_user_value: str
+    _validate_path_format: str
     _hook: hook.BaseHook
 
     def __init__(self, configuration: config.Configuration) -> None:
@@ -58,6 +138,8 @@ def __init__(self, configuration: config.Configuration) -> None:
         self._response_content_on_debug = configuration.get("logging", "response_content_on_debug")
         self._request_content_on_debug = configuration.get("logging", "request_content_on_debug")
         self._limit_content = configuration.get("logging", "limit_content")
+        self._validate_user_value = configuration.get("server", "validate_user_value")
+        self._validate_path_value = configuration.get("server", "validate_path_value")
         self._hook = hook.load(configuration)
 
     def _read_xml_request_body(self, environ: types.WSGIEnviron

radicale/app/move.py

@@ -25,6 +25,7 @@
 from urllib.parse import unquote, urlparse
 
 from radicale import httputils, pathutils, storage, types
+from radicale.app import base as app_base
 from radicale.app.base import Access, ApplicationBase
 from radicale.log import logger
 
@@ -82,6 +83,9 @@ def do_MOVE(self, environ: types.WSGIEnviron, base_prefix: str,
         if not access.check("w"):
             return httputils.NOT_ALLOWED
         to_path = pathutils.sanitize_path(to_url.path)
+        if not app_base._check_path_format(self._storage, to_path, self._validate_path_value):
+            logger.warning("request contains invalid path: %r (not compliant to %r)", to_path, self._validate_path_value)
+            return httputils.BAD_REQUEST
         if not (to_path + "/").startswith(base_prefix + "/"):
             logger.warning("Destination %r from MOVE request on %r doesn't "
                            "start with base prefix", to_path, path)