-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Do not add SharedMount if it is invalid #1013
Conversation
@rullzer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @icewind1991, @PVince81 and @schiessle to be potential reviewers |
Nope I was to quick... |
@icewind1991 maybe you have a 'somewhat' clean solution. Basically it fails in https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/sharedstorage.php#L87 because the file is in the trash (so not in |
$ownerView = new View('/' . $this->superShare->getShareOwner() . '/files'); | ||
$arguments['ownerView'] = $ownerView; | ||
|
||
// Check if the fileid is in the owners home (and not in the trash) see #938 |
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'd prefer full link to avoid problems with numbers of another project
Since this exception can be thrown when a shared file is in the trashbin there is no need to spam the log like crazy. Fixes #938
Rebased on @icewind1991 branch. Lets see what CI tells us. |
Maybe also fixes #710 |
👍 works as expected. Changes look good. |
Seems to do 👍 |
@rullzer Could you create the backport PR? |
Ah yes forgot. will do |
Ah no I did #1029 |
Ah got it ... it was merged into a different branch and not into master. |
If a shared file is moved to the trash the share entry is still alive.
However the sharee can't initialise the share since it is not in the
owners dir /files.
So check if it is there and if not log a debug and carry on.
Fixes #938
@icewind1991 This is safe right (we do it later in the code otherwise anyways that is where the error comes from). But this will not trigger depenency setup right?
CC: @LukasReschke @nickvergessen @MorrisJobke @blizzz