-
Notifications
You must be signed in to change notification settings - Fork 138
fix(SecureView): node cannot be found when within groupfolder #5058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
AndyScherzinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
58908fd to
950d054
Compare
lib/Storage/SecureViewWrapper.php
Outdated
| if ($parent === '' || $parent === '.') { | ||
| throw new NotFoundException(sprintf('Could not find cache entry for path of %s within storage %s ', $path, $storage->getId())); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think is correct, I think that would then fail when trying to move a file to the root of a storage.
if ($parent === '.') {
$parent = '';
}should be enough to handle the edge case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a move to user root worked in my test, but would not count out there are other scenarios 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjusted to your suggestions anyways
950d054 to
604420b
Compare
604420b to
eeeb83b
Compare
Signed-off-by: Arthur Schiwon <[email protected]>
eeeb83b to
07cad90
Compare
|
/backport to stable32 |
|
/backport to stable31 |
|
Does this apply to v30 as well? @blizzz ? |
Nope. |
Summary
The node cannot be properly figured out when the affected file stems from within a groupfolder. But we can use the filecache entry instead.
TODO
PermissionManager::shouldWatermark()as it now accepts Node or ICacheEntry which makes it harder to work with. – refactor: untangle Node|ICacheEntry parameter in shouldWatermark #5074Checklist