Skip to content

[stable33] fix(files_external): S3 folder mtime updated on every read-only access#61978

Draft
backportbot[bot] wants to merge 4 commits into
stable33from
backport/59539/stable33
Draft

[stable33] fix(files_external): S3 folder mtime updated on every read-only access#61978
backportbot[bot] wants to merge 4 commits into
stable33from
backport/59539/stable33

Conversation

@backportbot

@backportbot backportbot Bot commented Jul 10, 2026

Copy link
Copy Markdown

Backport of #59539

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Review and verify the backported changes
  • Amend HEAD commit to remove the line stating to skip CI

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

Antreesy added 4 commits July 10, 2026 15:34
normalizePath('') returns '.' for S3 object keys, but the filecache stores the
storage root under the key ''. getDirectoryMetaData() was calling getCache()->get('.')
which looks up by md5('.'), never matching the root entry stored at md5('').

The cache miss caused getDirectoryMetaData to return synthetic data with time() as
mtime/storage_mtime and uniqid() as etag on every call. The scanner then saw a
storage_mtime mismatch and wrote the fabricated timestamps back to the cache on every
occ files:scan run, regardless of whether any S3 content had changed.

Introduce getCachePath() to translate the normalized root path '.' back to '' before
any filecache lookup.

Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
…rs it

Common::getMetaData (Common.php:667) always sets storage_mtime = mtime before
returning. For S3 virtual directories this is wrong: mtime can be bumped by
child propagation while storage_mtime should remain the actual last S3 change.

When the scanner later calls getMetaData() it compares data['storage_mtime']
against cacheData['storage_mtime']. If they differ it writes the value back,
triggering View::getCacheEntry to fire propagateChange on every read even when
nothing on S3 changed.

Override getMetaData() to restore storage_mtime from the live cache entry (or,
for non-root directories not yet in the cache, from the S3 directory marker
LastModified header) after the parent call.

Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
…changed

Storage backends like AmazonS3 whose hasUpdated() always returns true (S3 has
no cheap global change-detection mechanism) caused propagateChange() to be called
on every watcher->update() after a folder was browsed, even when the underlying
storage_mtime and etag were identical to the cached values.

Before the fix, getCacheEntry() would call watcher->update() and then
unconditionally call propagateChange() whenever the watcher reported a change.
For S3 directories this meant every read bumped the parent mtime chain.

After the fix, getCacheEntry() snapshots the cache entry before watcher->update()
and compares it with the entry after. propagateChange() is only invoked when at
least one metadata field (mtime, storage_mtime, size, or etag) actually changed.

Assisted-by: ClaudeCode:claude-sonnet-4-6
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
fix(AmazonS3): handle missing LastModified and ETag in S3 responses

Add null-safety checks to handle S3 responses that don't include

LastModified and ETag fields. This prevents 'Undefined array key'

warnings and deprecation notices when processing directory metadata

or incomplete S3 responses.

- objectToMetaData(): Check if LastModified/ETag exist before accessing

- getMetaData(): Check if LastModified exists before using in strtotime()

Fixes test failures in testStat where hasUpdated('/', time) would fail

when encountering S3 objects without complete metadata.

Assisted-by: ClaudeCode:claude-sonnet-4-6

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>

[skip ci]
@backportbot backportbot Bot added bug 3. to review Waiting for reviews feature: external storage hotspot: file time handling ctime, mtime, etc. handling during various operations AI assisted labels Jul 10, 2026
@backportbot backportbot Bot added this to the Nextcloud 33.0.7 milestone Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted bug feature: external storage hotspot: file time handling ctime, mtime, etc. handling during various operations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant