Merge pull request #2094 from pbiering/tox-test-filesystems

Tox test filesystems

.github/workflows/test.yml

@@ -30,7 +30,7 @@ on:
 jobs:
 
   test-ubuntu-python-newest:
-    name: Test Ubuntu:latest Python:newest
+    name: Test Python:newest Ubuntu:latest
     needs: lint
     strategy:
       matrix:
@@ -48,7 +48,7 @@ jobs:
         run: tox -c pyproject.toml -e py
 
   test-ubuntu-python-newest-with-passlib:
-    name: Test Ubuntu:latest Python:newest passlib
+    name: Test passlib Python:newest Ubuntu:latest
     needs: [lint, test-ubuntu-python-newest]
     strategy:
       matrix:
@@ -64,12 +64,48 @@ jobs:
         run: pip install tox
       - name: Switch back to passlib
         run: sed -i 's|libpass[^"]*|passlib|' pyproject.toml
-      - name: Test with newest Python on latest Ubuntu
+      - name: Test with newest Python on latest Ubuntu using passlib
         run: tox -c pyproject.toml -e py
 
+  test-ubuntu-python-newest-with-vfat:
+    name: Test VFAT Python:newest Ubuntu:latest
+    needs: [lint, test-ubuntu-python-newest]
+    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
+        run: tox -c pyproject.toml -e py_filesystem_vfat
+
+  test-ubuntu-python-newest-with-ntfs:
+    name: Test NTFS Python:newest Ubuntu:latest
+    needs: [lint, test-ubuntu-python-newest]
+    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 NTFS
+        run: tox -c pyproject.toml -e py_filesystem_ntfs
+
   test-python-32bit:
-    name: Test Ubuntu:latest Python:32-bit
+    name: Test 32-bit Python:3.11 Ubuntu:latest
-    needs: [lint, test-ubuntu-python-newest, integ-test]
+    needs: [lint, test-ubuntu-python-newest, integ-test, test-ubuntu-python-newest-with-passlib]
     strategy:
       matrix:
         os: [ubuntu-latest]
@@ -102,13 +138,13 @@ jobs:
           python3 -m venv venv
           . venv/bin/activate
           pip install --upgrade pip
-      - name: Test 32-bit
+      - name: Test with 32-bit Python on latest Ubuntu
         run: |
           . venv/bin/activate
           tox -c /__w/Radicale/Radicale/pyproject.toml -e py
 
   test-ubuntu-python-oldest:
-    name: Test Ubuntu:latest Python:oldest
+    name: Test Python:oldest Ubuntu:latest
     needs: [lint, test-ubuntu-python-newest, test-python-32bit]
     strategy:
       matrix:
@@ -127,7 +163,7 @@ jobs:
 
   test-otheros-python-newest:
     name: Test MacOS/Windows:latest Python:newest
-    needs: [lint, test-ubuntu-python-newest, test-python-32bit]
+    needs: [lint, test-ubuntu-python-newest, test-ubuntu-python-newest-with-ntfs, test-ubuntu-python-newest-with-vfat]
     strategy:
       matrix:
         os: [macos-latest, windows-latest]
@@ -229,7 +265,7 @@ jobs:
   js-test:
     name: JS Type Check
     runs-on: ubuntu-latest
-    needs: test-ubuntu-python-newest
+    needs: [test-ubuntu-python-newest]
     steps:
       - uses: actions/checkout@v5
       - uses: actions/setup-node@v5

CHANGELOG.md

@@ -5,6 +5,9 @@
 * Improve: `path_to_filesystem()` by pre-detection of collision-free file system
 * Adjustment: MKCOL/MKCALENDAR return now CONFLICT instead of BADREQUEST of file name collision
 * Improve: [auth] catch bcrypt>=5.0.0 enforced max password length early and support legacy "passlib" as well as "libpass" (rework 3.6.0, "packaging" not needed anymore)
+* 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)
 
 ## 3.7.1
 * Fix: share address book collection as birthday calendar not working on non-DEBUG level

pyproject.toml

@@ -87,6 +87,88 @@ deps = [
 ]
 commands = [["pytest", "-r", "s", "--log-level", "5", "."]]
 
+[tool.tox.env.py_filesystem_vfat]
+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.img", "bs=1M", "count=64"],
+    # create file system
+    ["mkfs.vfat", "/tmp/vfat.img", "-n", "VFAT"],
+    # unconditionally create mount point
+    ["mkdir", "-p", "/tmp/vfat"],
+    # mount image
+    ["sudo", "/usr/bin/mount", "-o", "loop,umask=000", "/tmp/vfat.img", "/tmp/vfat"],
+]
+setenv = { TEMP = "/tmp/vfat" }
+commands = [["pytest", "-r", "s", "."]]
+commands_post = [
+    # umount image
+    ["sudo", "/usr/bin/umount", "-d", "/tmp/vfat"],
+    # remove mount point
+    ["rmdir", "/tmp/vfat"],
+    # remove image
+    ["rm", "/tmp/vfat.img"]
+]
+
+[tool.tox.env.py_filesystem_hfsplus]
+# Fedora
+allowlist_externals = [ "sudo", "dd", "mkfs.hfsplus", "mkdir", "rm", "rmdir", "chmod" ]
+extras = ["test"]
+deps = [
+    "pytest"
+]
+commands_pre = [
+    # create 64 MByte disk image
+    ["dd", "if=/dev/zero", "of=/tmp/hfsp.img", "bs=1M", "count=64"],
+    # create file system
+    ["mkfs.hfsplus", "/tmp/hfsp.img", "-v", "HFSP"],
+    # unconditionally create mount point
+    ["mkdir", "-p", "/tmp/hfsp"],
+    # mount image
+    ["sudo", "/usr/bin/mount", "-o", "loop,umask=000", "/tmp/hfsp.img", "/tmp/hfsp", "-t", "hfsplus"],
+]
+setenv = { TEMP = "/tmp/hfsp" }
+commands = [["pytest", "-r", "s", "."]]
+commands_post = [
+    # umount image
+    ["sudo", "/usr/bin/umount", "-d", "/tmp/hfsp"],
+    # remove mount point
+    ["rmdir", "/tmp/hfsp"],
+    # remove image
+    ["rm", "/tmp/hfsp.img"]
+]
+
+[tool.tox.env.py_filesystem_ntfs]
+allowlist_externals = [ "sudo", "dd", "mkfs.ntfs", "mkdir", "rm", "rmdir", "chmod" ]
+extras = ["test"]
+deps = [
+    "pytest"
+]
+commands_pre = [
+    # create 64 MByte disk image
+    ["dd", "if=/dev/zero", "of=/tmp/ntfs.img", "bs=1M", "count=64"],
+    # create file system
+    ["mkfs.ntfs", "/tmp/ntfs.img", "-L", "NTFS", "-F"],
+    # unconditionally create mount point
+    ["mkdir", "-p", "/tmp/ntfs"],
+    # mount image
+    ["sudo", "/usr/bin/mount", "-o", "loop,umask=0000", "/tmp/ntfs.img", "/tmp/ntfs"],
+]
+setenv = { TEMP = "/tmp/ntfs" }
+commands = [["pytest", "-r", "s", "."]]
+commands_post = [
+    # umount image
+    ["sudo", "/usr/bin/umount", "-d", "/tmp/ntfs"],
+    # remove mount point
+    ["rmdir", "/tmp/ntfs"],
+    # remove image
+    ["rm", "/tmp/ntfs.img"]
+]
+
 [tool.tox.env.flake8]
 deps = ["flake8==7.1.0"]
 commands = [["flake8", "."]]

radicale/app/__init__.py

@@ -100,6 +100,11 @@ class Application(ApplicationPartDelete, ApplicationPartHead,
 
         """
         super().__init__(configuration)
+        if 'TEMP' in os.environ:
+            if not os.path.isdir(os.environ['TEMP']):
+                raise RuntimeError("TEMP found in environment, but directory is not existing: %r" % os.environ['TEMP'])
+            if not os.access(os.environ['TEMP'], os.W_OK):
+                raise RuntimeError("TEMP found in environment, but not writable: %r" % os.environ['TEMP'])
         self._mask_passwords = configuration.get("logging", "mask_passwords")
         self._delay_on_error = configuration.get("server", "delay_on_error")
         logger.info("delay_on_error set to: %.3f seconds", self._delay_on_error)
@@ -535,9 +540,13 @@ class Application(ApplicationPartDelete, ApplicationPartHead,
                                     except ValueError as e:
                                         logger.warning("Failed to create predefined collection %r: %s", name_coll, e)
                         except ValueError as e:
-                            logger.warning("Failed to create principal "
+                            logger.error("Failed to create principal "
-                                           "collection %r: %s", user, e)
+                                         "collection for user %r: ValueError %s", user, e)
-                            user = ""
+                            return response(*httputils.INTERNAL_SERVER_ERROR)
+                        except OSError as e:
+                            logger.error("Failed to create principal "
+                                         "collection for user %r: OSerror %s", user, e)
+                            return response(*httputils.INTERNAL_SERVER_ERROR)
                 else:
                     logger.warning("Access to principal path %r denied by "
                                    "rights backend", principal_path)

radicale/pathutils.py

@@ -407,7 +407,7 @@ def path_supports_symlink(path):
 
 
 def path_is_collision_free_case_sensitive(path):
-    # Test: case sensitive
+    """Check whether path supports case sensitive entries."""
     if not os.path.isdir(path):
         raise ValueError("%r is not a path" % (path))
     base_dir = tempfile.mkdtemp(dir=path)
@@ -426,7 +426,7 @@ def path_is_collision_free_case_sensitive(path):
     # cleanup
     os.rmdir(test_dir_uc)
     os.rmdir(base_dir)
-    logger.debug("path_is_collision_free (case-sensitive): path=%r result=%s", path, result)
+    logger.trace("path_is_collision_free (case-sensitive): path=%r result=%s", path, result)
     return result
 
 
@@ -450,5 +450,78 @@ def path_is_collision_free_no_short_filename(path):
     # cleanup
     os.rmdir(test_dir_long)
     os.rmdir(base_dir)
-    logger.debug("path_is_collision_free (no short-filename): path=%r result=%s", path, result)
+    logger.trace("path_is_collision_free (no short-filename): path=%r result=%s", path, result)
+    return result
+
+
+def path_supports_unicode(path):
+    """Check whether path supports unicode."""
+    if not os.path.isdir(path):
+        raise ValueError("%r is not a path" % (path))
+    base_dir = tempfile.mkdtemp(dir=path)
+    part = "TEST😀"
+    test_dir = os.path.join(base_dir, part)
+    result = True
+    try:
+        os.mkdir(test_dir)
+    except OSError:
+        result = False
+    else:
+        with os.scandir(base_dir) as entries:
+            if part not in (e.name for e in entries):
+                result = False
+        # cleanup
+        os.rmdir(test_dir)
+    # cleanup
+    os.rmdir(base_dir)
+    logger.trace("path_supports_unicode: path=%r result=%s", path, result)
+    return result
+
+
+def path_supports_trailing_whitespace(path):
+    """Check whether path supports trailing whitespace."""
+    if not os.path.isdir(path):
+        raise ValueError("%r is not a path" % (path))
+    base_dir = tempfile.mkdtemp(dir=path)
+    part = "TEST "
+    test_dir = os.path.join(base_dir, part)
+    result = True
+    try:
+        os.mkdir(test_dir)
+    except OSError:
+        result = False
+    else:
+        with os.scandir(base_dir) as entries:
+            if part not in (e.name for e in entries):
+                result = False
+        # cleanup
+        os.rmdir(test_dir)
+    # cleanup
+    os.rmdir(base_dir)
+    logger.trace("path_supports_trailing_whitespace: path=%r result=%s", path, result)
+    return result
+
+
+def path_supports_problematic_chars(path):
+    """Check whether path supports problematic chars."""
+    if not os.path.isdir(path):
+        raise ValueError("%r is not a path" % (path))
+    base_dir = tempfile.mkdtemp(dir=path)
+    result = True
+    for char in ['*', '?']:
+        part = "TES" + char + "T"
+        test_dir = os.path.join(base_dir, part)
+        try:
+            os.mkdir(test_dir)
+        except OSError:
+            result = False
+        else:
+            with os.scandir(base_dir) as entries:
+                if part not in (e.name for e in entries):
+                    result = False
+            # cleanup
+            os.rmdir(test_dir)
+    # cleanup
+    os.rmdir(base_dir)
+    logger.trace("path_supports_problematic chars: path=%r result=%s", path, result)
     return result

radicale/storage/multifilesystem/__init__.py

@@ -110,7 +110,7 @@ class Storage(
         except Exception as e:
             logger.warning("Storage item mtime resolution test not possible, cannot set utime on file: %r (%s)", path, e)
             os.remove(path)
-            raise
+            raise ValueError  # do not raise a hard PermissionError
         logger.debug("Storage item mtime resoultion test set: %d ns" % MTIME_NS_TEST)
         mtime_ns = os.stat(path).st_mtime_ns - mtime_ns
         logger.debug("Storage item mtime resoultion test get: %d ns" % mtime_ns)
@@ -180,6 +180,9 @@ class Storage(
                     self._filesystem_root_folder_is_collision_free,
                     filesystem_root_folder_is_collision_free_case_sensitive,
                     filesystem_root_folder_is_collision_free_no_short_filename)
+        logger.info("Storage location subfolder suppports unicode: %s", pathutils.path_supports_unicode(self._get_collection_root_folder()))
+        logger.info("Storage location subfolder suppports trailing whitespace: %s", pathutils.path_supports_trailing_whitespace(self._get_collection_root_folder()))
+        logger.info("Storage location subfolder suppports problematic chars: %s", pathutils.path_supports_problematic_chars(self._get_collection_root_folder()))
         logger.info("Storage cache subfolder usage for 'item': %s", self._use_cache_subfolder_for_item)
         logger.info("Storage cache subfolder usage for 'history': %s", self._use_cache_subfolder_for_history)
         logger.info("Storage cache subfolder usage for 'sync-token': %s", self._use_cache_subfolder_for_synctoken)

radicale/tests/test_auth.py

@@ -31,7 +31,7 @@ from typing import Iterable, Tuple, Union
 
 import pytest
 
-from radicale import xmlutils
+from radicale import pathutils, xmlutils
 from radicale.auth import htpasswd
 from radicale.tests import BaseTest
 
@@ -64,7 +64,7 @@ class TestBaseAuthRequests(BaseTest):
 
     def _test_htpasswd(self, htpasswd_encryption: str, htpasswd_content: str,
                        test_matrix: Union[str, Iterable[Tuple[str, str, bool]]]
-                       = "ascii", delay: float = 0) -> None:
+                       = "ascii", delay: float = 0, check: int = 207) -> None:
         """Test htpasswd authentication with user "tmp" and password "bepo" for
            ``test_matrix`` "ascii" or user "😀" and password "🔑" for
            ``test_matrix`` "unicode"."""
@@ -88,7 +88,7 @@ class TestBaseAuthRequests(BaseTest):
         elif isinstance(test_matrix, str):
             raise ValueError("Unknown test matrix %r" % test_matrix)
         for user, password, valid in test_matrix:
-            self.propfind("/", check=207 if valid else 401,
+            self.propfind("/", check=check if valid else 401,
                           login="%s:%s" % (user, password))
 
     def test_htpasswd_plain(self) -> None:
@@ -102,7 +102,11 @@ class TestBaseAuthRequests(BaseTest):
             ("tmp", "be:po", True), ("tmp", "bepo", False)))
 
     def test_htpasswd_plain_unicode(self) -> None:
-        self._test_htpasswd("plain", "😀:🔑", "unicode")
+        if not pathutils.path_supports_unicode(self.colpath):
+            check = 500
+        else:
+            check = 207
+        self._test_htpasswd("plain", "😀:🔑", "unicode", check=check)
 
     def test_htpasswd_md5(self) -> None:
         self._test_htpasswd("md5", "tmp:$apr1$BI7VKCZh$GKW4vq2hqDINMr8uv7lDY/")
@@ -111,8 +115,12 @@ class TestBaseAuthRequests(BaseTest):
         self._test_htpasswd("autodetect", "tmp:$apr1$BI7VKCZh$GKW4vq2hqDINMr8uv7lDY/")
 
     def test_htpasswd_md5_unicode(self):
+        if not pathutils.path_supports_unicode(self.colpath):
+            check = 500
+        else:
+            check = 207
         self._test_htpasswd(
-            "md5", "😀:$apr1$w4ev89r1$29xO8EvJmS2HEAadQ5qy11", "unicode")
+            "md5", "😀:$apr1$w4ev89r1$29xO8EvJmS2HEAadQ5qy11", "unicode", check=check)
 
     def test_htpasswd_sha256(self) -> None:
         self._test_htpasswd("sha256", "tmp:$5$i4Ni4TQq6L5FKss5$ilpTjkmnxkwZeV35GB9cYSsDXTALBn6KtWRJAzNlCL/")
@@ -166,7 +174,11 @@ class TestBaseAuthRequests(BaseTest):
 
     @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
     def test_htpasswd_bcrypt_unicode(self) -> None:
-        self._test_htpasswd("bcrypt", "😀:$2y$10$Oyz5aHV4MD9eQJbk6GPemOs4T6edK6U9Sqlzr.W1mMVCS8wJUftnW", "unicode")
+        if not pathutils.path_supports_unicode(self.colpath):
+            check = 500
+        else:
+            check = 207
+        self._test_htpasswd("bcrypt", "😀:$2y$10$Oyz5aHV4MD9eQJbk6GPemOs4T6edK6U9Sqlzr.W1mMVCS8wJUftnW", "unicode", check=check)
 
     @pytest.mark.skipif(has_bcrypt == 0, reason="No bcrypt module installed")
     def test_htpasswd_bcrypt_long(self) -> None:
@@ -280,12 +292,23 @@ class TestBaseAuthRequests(BaseTest):
             else:
                 raise
 
-    @pytest.mark.skipif(sys.platform == "win32", reason="leading and trailing "
-                        "whitespaces not allowed in file names")
     def test_htpasswd_whitespace_user(self) -> None:
         for user in (" tmp", "tmp ", " tmp "):
+            if not pathutils.path_supports_trailing_whitespace(self.colpath) and user.endswith(' '):
+                check = 500
+            else:
+                check = 207
             self._test_htpasswd("plain", "%s:bepo" % user, (
-                (user, "bepo", True), ("tmp", "bepo", False)))
+                (user, "bepo", True), ("tmp", "bepo", False)), check=check)
+
+    def test_htpasswd_problem_user(self) -> None:
+        for user in ("tm*p", "tm?p"):
+            if not pathutils.path_supports_problematic_chars(self.colpath):
+                check = 500
+            else:
+                check = 207
+            self._test_htpasswd("plain", "%s:bepo" % user, (
+                (user, "bepo", True), ("tmp", "bepo", False)), check=check)
 
     def test_htpasswd_whitespace_password(self) -> None:
         for password in (" bepo", "bepo ", " bepo "):

radicale/tests/test_storage.py

@@ -190,6 +190,26 @@ class TestMultiFileSystem(BaseTest):
             assert answer is not None
             assert "\r\nUID:%s\r\n" % uid in answer
 
+    @pytest.mark.skipif(not pathutils.path_is_collision_free_case_sensitive(tempfile.mkdtemp()), reason="TEMP is not case sensitive")
+    def test_collection_storage_dummy_case_sensitivity(self) -> None:
+        """Test collection storage case sensitivity."""
+
+    @pytest.mark.skipif(not pathutils.path_is_collision_free_no_short_filename(tempfile.mkdtemp()), reason="TEMP has short filename")
+    def test_collection_storage_dummy_no_short_filename(self) -> None:
+        """Test collection storage no short filename."""
+
+    @pytest.mark.skipif(not pathutils.path_supports_unicode(tempfile.mkdtemp()), reason="TEMP is not supporting unicode")
+    def test_collection_storage_dummy_no_support_of_unicode(self) -> None:
+        """Test collection storage no support of unicode."""
+
+    @pytest.mark.skipif(not pathutils.path_supports_trailing_whitespace(tempfile.mkdtemp()), reason="TEMP is not supporting trailing whitespace")
+    def test_collection_storage_dummy_no_support_of_trailing_whitespace(self) -> None:
+        """Test collection storage no support of trailing space."""
+
+    @pytest.mark.skipif(not pathutils.path_supports_problematic_chars(tempfile.mkdtemp()), reason="TEMP is not supporting problematic chars")
+    def test_collection_storage_dummy_no_support_of_problematic_chars(self) -> None:
+        """Test collection storage no support of problematic chars."""
+
     @pytest.mark.skipif(not pathutils.path_supports_symlink(tempfile.mkdtemp()), reason="TEMP is not supporting symlink")
     def test_collection_sharing_by_softlink(self) -> None:
         """Test collection sharing by softlink."""