From b545b1562fda555a5e6b09a2c7f337f48861e60c Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 6 Jan 2026 11:14:52 -0500 Subject: [PATCH 1/3] fix(SecureView): robustly determine file/directory before forcing filecache update in shouldSecure() fix(richdocuments): robustly determine file/directory before forcing filecache update in shouldSecure() Signed-off-by: Josh --- lib/Service/SecureViewService.php | 47 ++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/Service/SecureViewService.php b/lib/Service/SecureViewService.php index 894dc2f2f2..74fd32f21b 100644 --- a/lib/Service/SecureViewService.php +++ b/lib/Service/SecureViewService.php @@ -28,16 +28,40 @@ public function isEnabled(): bool { } /** - * @throws NotFoundException + * Prepares metadata and context needed to decide if SecureView restrictions (such as watermarking or + * download blocking) may apply to a given file or folder. Ensures filecache metadata is available by + * triggering backend cache updates for files if requested, and supports external storages. Delegates + * the actual policy decision to shouldWatermark(). + * + * @param string $path Relative storage path to check. + * @param IStorage $storage Storage backend instance. + * @param bool $tryOpen Whether to attempt opening files to force/cache refresh file metadata. + * @return bool True if SecureView restrictions may apply (per shouldWatermark()), false otherwise. + * @throws NotFoundException If neither the file nor its parent are present in the filecache/remote storage. */ public function shouldSecure(string $path, IStorage $storage, bool $tryOpen = true): bool { - if ($tryOpen) { - // pity… fopen() does not document any possible Exceptions + // First: try to get the cache entry for $path + $cacheEntry = $storage->getCache()->get($path); + + $isDir = false; + // Prefer cache for fast directory check; fall back to (potentially slower) is_dir() if needed + if ($cacheEntry && !empty($cacheEntry['mimetype']) && $cacheEntry['mimetype'] === 'httpd/unix-directory') { + $isDir = true; + } elseif (!$cacheEntry && method_exists($storage, 'is_dir') && $storage->is_dir($path)) { + $isDir = true; + } + + if ($tryOpen && !isDir) { + // Attempt to open the file to potentially trigger cache entry creation $fp = $storage->fopen($path, 'r'); - fclose($fp); + if ($fp !== false && is_resource($fp)) { + fclose($fp); + } + // Re-fetch the cache entry after the 'poke' + $cacheEntry = $storage->getCache()->get($path); } - $cacheEntry = $storage->getCache()->get($path); + // Fallback to parent *only if cache entry still missing after poke* if (!$cacheEntry) { $parent = dirname($path); if ($parent === '.') { @@ -45,16 +69,25 @@ public function shouldSecure(string $path, IStorage $storage, bool $tryOpen = tr } $cacheEntry = $storage->getCache()->get($parent); if (!$cacheEntry) { - throw new NotFoundException(sprintf('Could not find cache entry for path and parent of %s within storage %s ', $path, $storage->getId())); + throw new NotFoundException(sprintf( + 'Could not find cache entry for path and parent of %s within storage %s', + $path, $storage->getId() + )); } } + // Now carry on with original SecureView logic... $isSharedStorage = $storage->instanceOfStorage(ISharedStorage::class); /** @noinspection PhpPossiblePolymorphicInvocationInspection */ /** @psalm-suppress UndefinedMethod **/ $share = $isSharedStorage ? $storage->getShare() : null; $userId = $this->userSession->getUser()?->getUID(); - return $this->permissionManager->shouldWatermark($cacheEntry, $userId, $share, $storage->getOwner($path) ?: null); + return $this->permissionManager->shouldWatermark( + $cacheEntry, + $userId, + $share, + $storage->getOwner($path) ?: null + ); } } From dc9122315afd59f4609006b7bbd3dd321adb97da Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 6 Jan 2026 12:34:42 -0500 Subject: [PATCH 2/3] chore: typo (fixup) Signed-off-by: Josh --- lib/Service/SecureViewService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/SecureViewService.php b/lib/Service/SecureViewService.php index 74fd32f21b..a39b6e21de 100644 --- a/lib/Service/SecureViewService.php +++ b/lib/Service/SecureViewService.php @@ -51,7 +51,7 @@ public function shouldSecure(string $path, IStorage $storage, bool $tryOpen = tr $isDir = true; } - if ($tryOpen && !isDir) { + if ($tryOpen && !$isDir) { // Attempt to open the file to potentially trigger cache entry creation $fp = $storage->fopen($path, 'r'); if ($fp !== false && is_resource($fp)) { From 9ad95240811fbd953b0b43c67bb2466da1e2812f Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 6 Jan 2026 12:41:14 -0500 Subject: [PATCH 3/3] chore: fixup for lint Signed-off-by: Josh --- lib/Service/SecureViewService.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Service/SecureViewService.php b/lib/Service/SecureViewService.php index a39b6e21de..ce2c6bd027 100644 --- a/lib/Service/SecureViewService.php +++ b/lib/Service/SecureViewService.php @@ -33,10 +33,10 @@ public function isEnabled(): bool { * triggering backend cache updates for files if requested, and supports external storages. Delegates * the actual policy decision to shouldWatermark(). * - * @param string $path Relative storage path to check. + * @param string $path Relative storage path to check. * @param IStorage $storage Storage backend instance. - * @param bool $tryOpen Whether to attempt opening files to force/cache refresh file metadata. - * @return bool True if SecureView restrictions may apply (per shouldWatermark()), false otherwise. + * @param bool $tryOpen Whether to attempt opening files to force/cache refresh file metadata. + * @return bool True if SecureView restrictions may apply (per shouldWatermark()), false otherwise. * @throws NotFoundException If neither the file nor its parent are present in the filecache/remote storage. */ public function shouldSecure(string $path, IStorage $storage, bool $tryOpen = true): bool {