Skip to content

Commit c6b96fd

Browse files
authored
fix(files): add FsNode.etag_unquoted, keep etag as the server sent it (#456)
`FsNode.etag` keeps what the server sent, quotes included, so it can be passed to an `If-Match`/`If-None-Match` header unchanged. Stripping them there would break that, measured against NC 35: the server answers 412 to a conditional write and silently ignores the precondition on a conditional read (200 instead of 304). The quotes are part of the entity tag (RFC 9110 §8.8.3). - add `FsNode.etag_unquoted` for callers that want the bare tag, which is what #448 asks for - `FsNode.etag` is always a `str` now: the trashbin sends an empty `<d:getetag/>`, which used to arrive as `None` - 9 unit tests over every source shape, plus an integration test that sends the unmodified etag back to the server and expects 304/2xx Closes #448 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `etag_unquoted` for retrieving ETags without surrounding quotation marks. * `etag` now preserves the server-provided format for direct use in conditional request headers. * Missing ETags consistently return as empty strings, including for trashbin entries. * **Bug Fixes** * Standardized ETag handling across file listings, responses, and file metadata. * Improved coverage for quoted, unquoted, and missing ETag values. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
1 parent b014f93 commit c6b96fd

4 files changed

Lines changed: 126 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ All notable changes to this project will be documented in this file.
44

55
## [0.30.3 - 2026-08-03]
66

7+
### Added
8+
9+
- `FsNode.etag_unquoted` returning the entity tag without the double quotes the server wraps it in, for comparing or storing the bare value. `FsNode.etag` keeps what the server sent, so it can still be passed to an `If-Match`/`If-None-Match` header unchanged. #448 Thanks to @kyteinsky
10+
711
### Fixed
812

13+
- `FsNode.etag` is always a string now; trashbin entries used to yield `None`, because the server sends an empty `<d:getetag/>` there. #448
914
- 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
1015

1116
## [0.30.2 - 2026-06-02]

nc_py_api/files/__init__.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,12 @@ class FsNode:
210210
"""File ID + NC instance ID"""
211211

212212
etag: str
213-
"""An entity tag (ETag) of the object"""
213+
"""An entity tag (ETag) of the object, exactly as the server sent it, including the double quotes around it.
214+
215+
Send it back as-is, e.g. ``{"If-Match": fs_node.etag}``: the quotes are part of the entity tag
216+
(:rfc:`9110#section-8.8.3`), and a server rejects the precondition without them.
217+
Use :py:attr:`~nc_py_api.files.FsNode.etag_unquoted` to compare or store the bare value.
218+
"""
214219

215220
info: FsNodeInfo
216221
"""Additional extra information for the object"""
@@ -221,7 +226,8 @@ class FsNode:
221226
def __init__(self, full_path: str, **kwargs):
222227
self.full_path = full_path
223228
self.file_id = kwargs.get("file_id", "")
224-
self.etag = kwargs.get("etag", "")
229+
# the trashbin sends an empty `<d:getetag/>`, which arrives here as None
230+
self.etag = kwargs.get("etag") or ""
225231
self.info = FsNodeInfo(**kwargs)
226232
self.lock_info = FsNodeLockInfo(**kwargs)
227233

@@ -230,6 +236,14 @@ def is_dir(self) -> bool:
230236
"""Returns ``True`` for the directories, ``False`` otherwise."""
231237
return self.full_path.endswith("/")
232238

239+
@property
240+
def etag_unquoted(self) -> str:
241+
""":py:attr:`~nc_py_api.files.FsNode.etag` without the surrounding double quotes.
242+
243+
For comparing or storing the bare tag; use :py:attr:`~nc_py_api.files.FsNode.etag` in request headers.
244+
"""
245+
return self.etag.strip('"')
246+
233247
def __str__(self):
234248
if self.info.is_version:
235249
return (

tests/actual_tests/files_test.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,3 +1300,20 @@ async def test_file_locking_async(anc_any):
13001300
with pytest.raises(NextcloudException) as e:
13011301
await anc_any.files.unlock(test_file)
13021302
assert e.value.status_code == 412
1303+
1304+
1305+
def test_etag_is_accepted_by_server_as_is(nc_any):
1306+
"""`FsNode.etag` must be usable in a request header without the caller touching it."""
1307+
nc_any.files.delete("test_etag_as_is.txt", not_fail=True)
1308+
node = nc_any.files.upload("test_etag_as_is.txt", b"content")
1309+
listed = nc_any.files.by_path("test_etag_as_is.txt")
1310+
assert node.etag == listed.etag
1311+
assert listed.etag_unquoted == listed.etag.strip('"')
1312+
dav_path = f"/files/{nc_any.user}/test_etag_as_is.txt"
1313+
unchanged = nc_any._session.adapter_dav.request("GET", dav_path, headers={"If-None-Match": listed.etag})
1314+
assert unchanged.status_code == 304
1315+
overwritten = nc_any._session.adapter_dav.request(
1316+
"PUT", dav_path, data=b"new content", headers={"If-Match": listed.etag}
1317+
)
1318+
assert overwritten.status_code in (200, 204)
1319+
nc_any.files.delete("test_etag_as_is.txt", not_fail=True)
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Tests for FsNode.etag: kept exactly as the server sent it, with a bare variant next to it."""
2+
3+
from nc_py_api.files import ActionFileInfo, FsNode
4+
from nc_py_api.files._files import _parse_record, etag_fileid_from_response
5+
6+
7+
class _FakeResponse:
8+
def __init__(self, headers: dict):
9+
self.headers = headers
10+
11+
12+
def _prop_stat(etag) -> dict:
13+
return {
14+
"d:status": "HTTP/1.1 200 OK",
15+
"d:prop": {"oc:id": "00000123", "oc:fileid": "123", "oc:permissions": "RGDNVW", "d:getetag": etag},
16+
}
17+
18+
19+
def test_etag_is_kept_as_the_server_sent_it():
20+
# the quotes are part of the entity tag, so `etag` stays usable in a request header as-is
21+
assert FsNode("files/admin/a.txt", etag='"6a351fb28bebc"').etag == '"6a351fb28bebc"'
22+
23+
24+
def test_etag_unquoted_strips_the_quotes():
25+
assert FsNode("files/admin/a.txt", etag='"6a351fb28bebc"').etag_unquoted == "6a351fb28bebc"
26+
27+
28+
def test_unquoted_etag_passes_through_both_ways():
29+
# versions endpoints answer with a bare timestamp instead of a quoted tag
30+
node = FsNode("files/admin/a.txt", etag="1785767946")
31+
assert node.etag == "1785767946"
32+
assert node.etag_unquoted == "1785767946"
33+
34+
35+
def test_missing_and_empty_etag_become_empty_string():
36+
for node in (FsNode("files/admin/a.txt"), FsNode("files/admin/a.txt", etag=""), FsNode("f/a", etag=None)):
37+
assert node.etag == ""
38+
assert node.etag_unquoted == ""
39+
40+
41+
def test_propfind_record_keeps_the_quoted_etag():
42+
assert _parse_record("files/admin/a.txt", [_prop_stat('"6a351fb28bebc"')]).etag == '"6a351fb28bebc"'
43+
assert _parse_record("files/admin/a.txt", [_prop_stat('"6a351fb28bebc"')]).etag_unquoted == "6a351fb28bebc"
44+
# the trashbin sends `<d:getetag/>`, which arrives as None
45+
assert _parse_record("files/admin/a.txt", [_prop_stat(None)]).etag == ""
46+
47+
48+
def test_oc_etag_header_keeps_the_quoted_etag():
49+
response = _FakeResponse({"OC-Etag": '"e9673fb8e3e49ff7cbbff9f21e9c60d1"', "OC-FileId": "00000123"})
50+
node = FsNode("files/admin/a.txt", **etag_fileid_from_response(response))
51+
assert node.etag == '"e9673fb8e3e49ff7cbbff9f21e9c60d1"'
52+
assert node.etag_unquoted == "e9673fb8e3e49ff7cbbff9f21e9c60d1"
53+
54+
55+
def test_etag_missing_from_headers():
56+
response = _FakeResponse({"OC-FileId": "00000123"})
57+
assert FsNode("files/admin/a.txt", **etag_fileid_from_response(response)).etag == ""
58+
59+
60+
def test_both_sources_agree_for_the_same_file():
61+
from_propfind = _parse_record("files/admin/a.txt", [_prop_stat('"e9673fb8e3e49ff7cbbff9f21e9c60d1"')])
62+
from_header = FsNode(
63+
"files/admin/a.txt",
64+
**etag_fileid_from_response(
65+
_FakeResponse({"OC-Etag": '"e9673fb8e3e49ff7cbbff9f21e9c60d1"', "OC-FileId": "00000123"})
66+
),
67+
)
68+
assert from_propfind.etag == from_header.etag
69+
assert from_propfind.etag_unquoted == from_header.etag_unquoted == "e9673fb8e3e49ff7cbbff9f21e9c60d1"
70+
71+
72+
def test_action_file_info_to_fs_node_keeps_etag():
73+
# the ExApp UI file actions build FsNode from data the server posts to the ExApp
74+
action_file = ActionFileInfo(
75+
fileId=123,
76+
name="a.txt",
77+
directory="/",
78+
etag='"6a351fb28bebc"',
79+
mime="text/plain",
80+
fileType="file",
81+
size=7,
82+
favorite="false",
83+
permissions=27,
84+
mtime=1785767946,
85+
userId="admin",
86+
)
87+
assert action_file.to_fs_node().etag == '"6a351fb28bebc"'
88+
assert action_file.to_fs_node().etag_unquoted == "6a351fb28bebc"

0 commit comments

Comments
 (0)