[stable32] fix(SecureViewService): handle fopen() returning false for non-existent paths#5584
Merged
[stable32] fix(SecureViewService): handle fopen() returning false for non-existent paths#5584
Conversation
…nt paths When shouldSecure() is called with tryOpen=true on a path that does not exist yet (e.g. a rename target or a files_versions snapshot path), the underlying fopen() call returns false. Calling fclose(false) on that value throws a TypeError in PHP 8, which propagates as an uncaught exception and aborts the entire DAV operation with HTTP 500. To reproduce: enable Secure View watermarking for a group, add your user to that group, then rename or overwrite any Office document. The rename fails immediately with HTTP 500 and the following error appears in nextcloud.log: fclose(): Argument #1 ($stream) must be of type resource, bool given in …/richdocuments/lib/Service/SecureViewService.php:37 Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… on delete The previous fix (returning true when fopen() returns false) resolved the rename/copy TypeError, but delete still failed. When files_trashbin moves a watermarked file to trash it calls: trashStorage->moveFromStorage(sourceStorage, srcPath, trashPath) The SecureViewWrapper on trashStorage calls shouldSecure(srcPath, sourceStorage, tryOpen=true). shouldSecure then calls sourceStorage->fopen(), which hits SecureViewWrapper::fopen() → checkFileAccess() → ForbiddenException because the file is watermarked and the request is not a WOPI request. SecureViewWrapper does not override file_exists(), so replacing fopen() with file_exists() goes straight to the underlying storage without any access check. When the path does not exist we return true early (same semantic: assume target is in a secure context); when it does exist we fall through to the cache-based shouldWatermark() check as before. Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
chrip
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of PR #5577