fix(annotations): compare the change time only between fstat reads
Python 3.12+ on Windows reports the change time from stat but the creation time from fstat, so a freshly written file whose creation and write straddle a clock tick failed the pre-read identity check with CaptureChanged. The path-versus-descriptor checks now compare size, mtime_ns and dev/ino; ctime_ns is compared between the fstat before and after the copy, where it still detects a same-size rewrite with a restored mtime on POSIX. The symlinked-root test asserted the locator against the unresolved temp path, which macOS (/var -> /private/var) and Windows (short names) resolve differently; it now compares against the real path.
This commit is contained in:
+4
-2
@@ -1373,8 +1373,10 @@ Capture store (#249, `utils/media_capture.py`, `tests/test_media_capture.py`):
|
||||
one explicit action captures one rooted media file into an owner-only
|
||||
`prompt-manager-captures-*` temp directory, streaming it in 1 MiB chunks while
|
||||
hashing, with the opened descriptor compared to the path (`dev`, `ino` where
|
||||
meaningful, `size`, `mtime_ns`, `ctime_ns`) before and after the copy; any
|
||||
disagreement is `CaptureChanged` and the copy is removed. Aliases (a symlinked
|
||||
meaningful, `size`, `mtime_ns`) before and after the copy and `ctime_ns`
|
||||
compared between the two `fstat` reads (Python 3.12+ on Windows reports the
|
||||
change time from `stat` but the creation time from `fstat`); any disagreement
|
||||
is `CaptureChanged` and the copy is removed. Aliases (a symlinked
|
||||
file or directory, even inside the root), escapes, non-media names and
|
||||
directories are `CaptureDenied`; the locator is
|
||||
`normcase(join(realpath(root), relative))`, so a symlinked root shares the
|
||||
|
||||
@@ -247,7 +247,9 @@ class CaptureStoreTestCase(unittest.TestCase):
|
||||
self.assertEqual(
|
||||
cap.locator, os.path.normcase(os.path.realpath(link_root / "sub/a.png"))
|
||||
)
|
||||
self.assertTrue(cap.locator.startswith(os.path.normcase(str(real_root))))
|
||||
self.assertTrue(
|
||||
cap.locator.startswith(os.path.normcase(os.path.realpath(real_root)))
|
||||
)
|
||||
self.assertEqual(cap.sha256, hashlib.sha256(source_bytes).hexdigest())
|
||||
|
||||
# -- change detection ----------------------------------------------------
|
||||
|
||||
+19
-10
@@ -146,13 +146,18 @@ class _Entry:
|
||||
self.doomed = False
|
||||
|
||||
|
||||
def _same_file(before: os.stat_result, after: os.stat_result) -> bool:
|
||||
"""Size, mtime and change time must agree; device/inode where meaningful."""
|
||||
if (
|
||||
before.st_size != after.st_size
|
||||
or before.st_mtime_ns != after.st_mtime_ns
|
||||
or before.st_ctime_ns != after.st_ctime_ns
|
||||
):
|
||||
def _same_file(
|
||||
before: os.stat_result, after: os.stat_result, *, change_time: bool
|
||||
) -> bool:
|
||||
"""Size and mtime must agree, device/inode where meaningful.
|
||||
|
||||
The change time is compared only between two reads of the same kind
|
||||
(``fstat`` before and after the copy): Windows reports the creation time
|
||||
from ``fstat`` but, on Python 3.12+, the change time from ``stat``.
|
||||
"""
|
||||
if before.st_size != after.st_size or before.st_mtime_ns != after.st_mtime_ns:
|
||||
return False
|
||||
if change_time and before.st_ctime_ns != after.st_ctime_ns:
|
||||
return False
|
||||
if before.st_ino and after.st_ino and before.st_ino != after.st_ino:
|
||||
return False
|
||||
@@ -483,7 +488,7 @@ class CaptureStore:
|
||||
raise CaptureChanged(f"The file could not be opened: {error}") from None
|
||||
with os.fdopen(fd, "rb") as stream:
|
||||
opened = os.fstat(stream.fileno())
|
||||
if not _same_file(before, opened):
|
||||
if not _same_file(before, opened, change_time=False):
|
||||
raise CaptureChanged("The file changed before it was read")
|
||||
while True:
|
||||
chunk = self._read(stream, CHUNK_SIZE)
|
||||
@@ -496,13 +501,17 @@ class CaptureStore:
|
||||
if sink is not None:
|
||||
sink.write(chunk)
|
||||
after = os.fstat(stream.fileno())
|
||||
if total != before.st_size or not _same_file(before, after):
|
||||
if (
|
||||
total != before.st_size
|
||||
or not _same_file(opened, after, change_time=True)
|
||||
or not _same_file(before, after, change_time=False)
|
||||
):
|
||||
raise CaptureChanged("The file changed while it was read")
|
||||
try:
|
||||
current = os.stat(candidate)
|
||||
except OSError:
|
||||
raise CaptureChanged("The file is no longer there") from None
|
||||
if not _same_file(before, current):
|
||||
if not _same_file(before, current, change_time=False):
|
||||
raise CaptureChanged("The path names another file now")
|
||||
if os.path.normcase(os.path.realpath(candidate)) != locator:
|
||||
raise CaptureChanged("The path resolves elsewhere now")
|
||||
|
||||
Reference in New Issue
Block a user