fix(files): never mutate the shared PROPFIND property lists

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to this project will be documented in this file.
 
+## [0.30.3 - 2026-08-03]
+
+### Fixed
+
+- PROPFIND property lists are no longer mutated in place. `get_propfind_properties()` and both `trashbin_list()` implementations extended the shared `PROPFIND_PROPERTIES` constant with `+=`, so it grew on every call (7 entries per call against servers advertising `files.locking`, 3 per `trashbin_list()`). Long-running clients ended up sending multi-megabyte PROPFIND bodies that could exhaust the server's workers. Both property constants are now immutable tuples, so this class of bug cannot come back. #453 Thanks to @ciberkids
+
 ## [0.30.2 - 2026-06-02]
 
 ### Changed

nc_py_api/files/_files.py

@@ -2,6 +2,8 @@
 
 import contextlib
 import enum
+import typing
+from collections.abc import Sequence
 from datetime import datetime, timezone
 from io import BytesIO
 from json import dumps, loads
@@ -16,7 +18,7 @@
 from .._misc import check_capabilities, clear_from_params_empty
 from . import FsNode, SystemTag
 
-PROPFIND_PROPERTIES = [
+PROPFIND_PROPERTIES: typing.Final[tuple[str, ...]] = (
     "d:resourcetype",
     "d:getlastmodified",
     "d:creationdate",
@@ -34,17 +36,17 @@
     "oc:share-types",
     "oc:favorite",
     "nc:is-encrypted",
-]
+)
 
-PROPFIND_LOCKING_PROPERTIES = [
+PROPFIND_LOCKING_PROPERTIES: typing.Final[tuple[str, ...]] = (
     "nc:lock",
     "nc:lock-owner-displayname",
     "nc:lock-owner",
     "nc:lock-owner-type",
     "nc:lock-owner-editor",  # App id of an app owned lock
     "nc:lock-time",  # Timestamp of the log creation time
     "nc:lock-timeout",  # TTL of the lock in seconds staring from the creation time
-]
+)
 
 SEARCH_PROPERTIES_MAP = {
     "name": "d:displayname",  # like, eq
@@ -66,8 +68,8 @@ class PropFindType(enum.IntEnum):
     VERSIONS_FILE_ID = 3
 
 
-def get_propfind_properties(capabilities: dict) -> list:
-    r = PROPFIND_PROPERTIES
+def get_propfind_properties(capabilities: dict) -> list[str]:
+    r = list(PROPFIND_PROPERTIES)
     if not check_capabilities("files.locking", capabilities):
         r += PROPFIND_LOCKING_PROPERTIES
     return r
@@ -222,7 +224,7 @@ def build_update_tag_req(
 
 
 def build_listdir_req(
-    user: str, path: str, properties: list[str], prop_type: PropFindType
+    user: str, path: str, properties: Sequence[str], prop_type: PropFindType
 ) -> tuple[ElementTree.Element, str]:
     root = ElementTree.Element(
         "d:propfind",
@@ -245,7 +247,7 @@ def build_listdir_response(
     webdav_response: Response,
     user: str,
     path: str,
-    properties: list[str],
+    properties: Sequence[str],
     exclude_self: bool,
     prop_type: PropFindType,
 ) -> list[FsNode]:

nc_py_api/files/files.py

@@ -2,6 +2,7 @@
 
 import builtins
 import os
+from collections.abc import Sequence
 from pathlib import Path
 from urllib.parse import quote
 
@@ -278,8 +279,12 @@ def setfav(self, path: str | FsNode, value: int | bool) -> None:
 
     def trashbin_list(self) -> list[FsNode]:
         """Returns a list of all entries in the TrashBin."""
-        properties = PROPFIND_PROPERTIES
-        properties += ["nc:trashbin-filename", "nc:trashbin-original-location", "nc:trashbin-deletion-time"]
+        properties = [
+            *PROPFIND_PROPERTIES,
+            "nc:trashbin-filename",
+            "nc:trashbin-original-location",
+            "nc:trashbin-deletion-time",
+        ]
         return self._listdir(
             self._session.user, "", properties=properties, depth=1, exclude_self=False, prop_type=PropFindType.TRASHBIN
         )
@@ -457,7 +462,7 @@ def _listdir(
         self,
         user: str,
         path: str,
-        properties: list[str],
+        properties: Sequence[str],
         depth: int,
         exclude_self: bool,
         prop_type: PropFindType = PropFindType.DEFAULT,

nc_py_api/files/files_async.py

@@ -2,6 +2,7 @@
 
 import builtins
 import os
+from collections.abc import Sequence
 from pathlib import Path
 from urllib.parse import quote
 
@@ -282,8 +283,12 @@ async def setfav(self, path: str | FsNode, value: int | bool) -> None:
 
     async def trashbin_list(self) -> list[FsNode]:
         """Returns a list of all entries in the TrashBin."""
-        properties = PROPFIND_PROPERTIES
-        properties += ["nc:trashbin-filename", "nc:trashbin-original-location", "nc:trashbin-deletion-time"]
+        properties = [
+            *PROPFIND_PROPERTIES,
+            "nc:trashbin-filename",
+            "nc:trashbin-original-location",
+            "nc:trashbin-deletion-time",
+        ]
         return await self._listdir(
             await self._session.user,
             "",
@@ -466,7 +471,7 @@ async def _listdir(
         self,
         user: str,
         path: str,
-        properties: list[str],
+        properties: Sequence[str],
         depth: int,
         exclude_self: bool,
         prop_type: PropFindType = PropFindType.DEFAULT,

tests_unit/test_propfind_properties.py

@@ -0,0 +1,105 @@
+"""Tests that the shared PROPFIND property lists are never mutated in place."""
+
+import types
+
+import pytest
+
+from nc_py_api.files._files import (
+    PROPFIND_LOCKING_PROPERTIES,
+    PROPFIND_PROPERTIES,
+    PropFindType,
+    get_propfind_properties,
+)
+from nc_py_api.files.files import FilesAPI
+from nc_py_api.files.files_async import AsyncFilesAPI
+
+CAPS_WITH_LOCKING = {"files": {"locking": "1.0"}}
+CAPS_WITHOUT_LOCKING = {"files": {}}
+TRASHBIN_PROPERTIES = ["nc:trashbin-filename", "nc:trashbin-original-location", "nc:trashbin-deletion-time"]
+# real copies taken at import: comparing against the live constants would be vacuous
+# if they are ever turned back into lists, since the name would alias the same object
+EXPECTED_PROPERTIES = tuple(PROPFIND_PROPERTIES)
+EXPECTED_LOCKING_PROPERTIES = tuple(PROPFIND_LOCKING_PROPERTIES)
+
+
+def test_propfind_constants_are_immutable():
+    assert isinstance(PROPFIND_PROPERTIES, tuple)
+    assert isinstance(PROPFIND_LOCKING_PROPERTIES, tuple)
+    with pytest.raises(AttributeError):
+        PROPFIND_PROPERTIES.append("nc:not-allowed")
+
+
+def test_get_propfind_properties_does_not_mutate_constants():
+    for _ in range(10):
+        get_propfind_properties(CAPS_WITH_LOCKING)
+        get_propfind_properties(CAPS_WITHOUT_LOCKING)
+    assert tuple(PROPFIND_PROPERTIES) == EXPECTED_PROPERTIES
+    assert tuple(PROPFIND_LOCKING_PROPERTIES) == EXPECTED_LOCKING_PROPERTIES
+
+
+def test_get_propfind_properties_returns_fresh_list():
+    first = get_propfind_properties(CAPS_WITH_LOCKING)
+    second = get_propfind_properties(CAPS_WITH_LOCKING)
+    assert isinstance(first, list)
+    assert first == second
+    assert first is not second
+    first.append("nc:added-by-caller")
+    assert "nc:added-by-caller" not in second
+    assert "nc:added-by-caller" not in PROPFIND_PROPERTIES
+    assert tuple(PROPFIND_PROPERTIES) == EXPECTED_PROPERTIES
+
+
+def test_get_propfind_properties_locking_capability():
+    with_locking = get_propfind_properties(CAPS_WITH_LOCKING)
+    without_locking = get_propfind_properties(CAPS_WITHOUT_LOCKING)
+    assert with_locking == [*EXPECTED_PROPERTIES, *EXPECTED_LOCKING_PROPERTIES]
+    assert without_locking == list(EXPECTED_PROPERTIES)
+
+
+def test_trashbin_list_does_not_mutate_constants(monkeypatch):
+    requested = []
+
+    def _fake_listdir(_self, _user, _path, **kwargs):
+        requested.append(list(kwargs["properties"]))
+        return []
+
+    monkeypatch.setattr(FilesAPI, "_listdir", _fake_listdir)
+    files_api = FilesAPI(types.SimpleNamespace(user="admin"))
+    for _ in range(3):
+        files_api.trashbin_list()
+    assert tuple(PROPFIND_PROPERTIES) == EXPECTED_PROPERTIES
+    for properties in requested:
+        assert properties == [*EXPECTED_PROPERTIES, *TRASHBIN_PROPERTIES]
+
+
+async def test_trashbin_list_async_does_not_mutate_constants(monkeypatch):
+    requested = []
+
+    async def _fake_listdir(_self, _user, _path, **kwargs):
+        requested.append(list(kwargs["properties"]))
+        return []
+
+    class _StubSession:
+        @property
+        async def user(self) -> str:
+            return "admin"
+
+    monkeypatch.setattr(AsyncFilesAPI, "_listdir", _fake_listdir)
+    files_api = AsyncFilesAPI(_StubSession())
+    for _ in range(3):
+        await files_api.trashbin_list()
+    assert tuple(PROPFIND_PROPERTIES) == EXPECTED_PROPERTIES
+    for properties in requested:
+        assert properties == [*EXPECTED_PROPERTIES, *TRASHBIN_PROPERTIES]
+
+
+def test_trashbin_list_requests_trashbin_prop_type(monkeypatch):
+    calls = []
+
+    def _fake_listdir(_self, _user, _path, **kwargs):
+        calls.append(kwargs["prop_type"])
+        return []
+
+    monkeypatch.setattr(FilesAPI, "_listdir", _fake_listdir)
+    FilesAPI(types.SimpleNamespace(user="admin")).trashbin_list()
+    assert calls == [PropFindType.TRASHBIN]