From 0388051046e1f4bfa96750df4f7c5d661ca08980 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Fri, 23 Jan 2026 16:44:55 +0100 Subject: [PATCH] Fix unclosed scandir iterator in path_to_filesystem Use os.scandir() as a context manager to ensure the iterator is properly closed. This fixes ResourceWarning: unclosed scandir iterator that occurs when the iterator is garbage collected without being closed. Fixes #1972 Co-Authored-By: Claude Opus 4.5 --- radicale/pathutils.py | 7 +-- radicale/tests/test_pathutils.py | 91 ++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 radicale/tests/test_pathutils.py diff --git a/radicale/pathutils.py b/radicale/pathutils.py index e4e65928..3193e4a2 100644 --- a/radicale/pathutils.py +++ b/radicale/pathutils.py @@ -286,9 +286,10 @@ def path_to_filesystem(root: str, sane_path: str) -> str: safe_path = os.path.join(safe_path, part) # Check for conflicting files (e.g. case-insensitive file systems # or short names on Windows file systems) - if (os.path.lexists(safe_path) and - part not in (e.name for e in os.scandir(safe_path_parent))): - raise CollidingPathError(part) + if os.path.lexists(safe_path): + with os.scandir(safe_path_parent) as entries: + if part not in (e.name for e in entries): + raise CollidingPathError(part) return safe_path diff --git a/radicale/tests/test_pathutils.py b/radicale/tests/test_pathutils.py new file mode 100644 index 00000000..ebe92de8 --- /dev/null +++ b/radicale/tests/test_pathutils.py @@ -0,0 +1,91 @@ +# This file is part of Radicale - CalDAV and CardDAV server +# Copyright © 2025 Tobias Brox +# +# 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 +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Radicale. If not, see . + +""" +Tests for pathutils module. + +""" + +import gc +import os +import tempfile + +import pytest + +from radicale import pathutils + + +class TestPathToFilesystem: + """Tests for path_to_filesystem function.""" + + @pytest.mark.filterwarnings("error::ResourceWarning") + @pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning") + def test_scandir_iterator_closed(self) -> None: + """Verify that os.scandir iterator is properly closed. + + This test catches ResourceWarning: unclosed scandir iterator + which occurs when os.scandir() is used without a context manager. + See: https://github.com/Kozea/Radicale/issues/1972 + + The ResourceWarning is emitted during garbage collection when an + unclosed scandir iterator is finalized. We use pytest.mark.filterwarnings + to convert both ResourceWarning and PytestUnraisableExceptionWarning + to errors. + """ + with tempfile.TemporaryDirectory() as tmpdir: + # Create a subdirectory so path_to_filesystem has something + # to scan (the scandir check is for case-insensitive filesystems) + subdir = os.path.join(tmpdir, "testdir") + os.makedirs(subdir) + + # Call path_to_filesystem - if scandir iterator is not closed, + # a ResourceWarning will be emitted during garbage collection + result = pathutils.path_to_filesystem(tmpdir, "testdir") + assert result == subdir + + # Force garbage collection to trigger any ResourceWarning + # from unclosed iterators + gc.collect() + + def test_path_to_filesystem_basic(self) -> None: + """Test basic path_to_filesystem functionality.""" + with tempfile.TemporaryDirectory() as tmpdir: + # Test empty path + result = pathutils.path_to_filesystem(tmpdir, "") + assert result == tmpdir + + # Test single component + subdir = os.path.join(tmpdir, "test") + os.makedirs(subdir) + result = pathutils.path_to_filesystem(tmpdir, "test") + assert result == subdir + + # Test nested path + nested = os.path.join(subdir, "nested") + os.makedirs(nested) + result = pathutils.path_to_filesystem(tmpdir, "test/nested") + assert result == nested + + def test_unsafe_path_raises(self) -> None: + """Test that unsafe path components raise UnsafePathError.""" + with tempfile.TemporaryDirectory() as tmpdir: + # Hidden files (starting with .) are not safe + with pytest.raises(pathutils.UnsafePathError): + pathutils.path_to_filesystem(tmpdir, ".hidden") + + # Backup files (ending with ~) are not safe + with pytest.raises(pathutils.UnsafePathError): + pathutils.path_to_filesystem(tmpdir, "backup~")