Skip to content
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

Use the unjailed-path in OC_Helper::getStorageInfo() for files located in SharedStorage. #30985

Merged

Conversation

rotdrop
Copy link
Contributor

@rotdrop rotdrop commented Feb 3, 2022

The current implementation already switches the storage-backend to $storage->getSourceStorage(). However, it then calls
$rootInfo->getInternalPath() which returns the internal path relative to the storage where the share is mounted. This is wrong, we need also to unjail the path. Compare, e.g., with OCA\Files_Sharing\SharedStorage::file_get/put_contents() for the
"logic".

Fix #30992

Signed-off-by: Claus-Justus Heine himself@claus-justus-heine.de

…d in SharedStorage.

The current implementation already switches the storage-backend to
$storage->getSourceStorage(). However, it then calls
$rootInfo->getInternalPath() which returns the internal path relative to
the storage where the share is mounted. This is wrong, we need also to
unjail the path. Compare, e.g., with
OCA\Files_Sharing\SharedStorage::file_get/put_contents() for the
"logic".

Signed-off-by: Claus-Justus Heine <himself@claus-justus-heine.de>
@szaimen szaimen added the 3. to review Waiting for reviews label Feb 3, 2022
@szaimen szaimen added this to the Nextcloud 24 milestone Feb 3, 2022
@szaimen szaimen requested review from a team, PVince81, ArtificialOwl and CarlSchwan and removed request for a team February 3, 2022 09:17
@PVince81 PVince81 requested a review from icewind1991 February 3, 2022 16:23
@CarlSchwan
Copy link
Member

Do you know a test case there this previously broke and now is fixed?

@rotdrop
Copy link
Contributor Author

rotdrop commented Feb 3, 2022

Do you know a test case there this previously broke and now is fixed?

When listing any shared folder deeper than 1 level I have error messages in my logs like

Error: disk_free_space(): No such file or directory at /var/www/orgacloud/nextcloud/lib/private/Files/Storage/Local.php#381 /var/www/orgacloud/nextcloud/lib/private/Log/ErrorHandler.php:95

That they do not show up more often is probably caused by the following fall-back to the dirname:

if (!is_dir($sourcePath)) {
// disk_free_space doesn't work on files
$sourcePath = dirname($sourcePath);
}

which means that just listing a shared folder will not trigger the error messages, but give wrong results.

Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@szaimen
Copy link
Contributor

szaimen commented Feb 3, 2022

samba failure unrelated

@szaimen szaimen merged commit f2cd30d into nextcloud:master Feb 3, 2022
@szaimen
Copy link
Contributor

szaimen commented Feb 3, 2022

/backport to stable23

@solracsf

This comment was marked as resolved.

@CarlSchwan
Copy link
Member

/backport to stable22

@szaimen

This comment was marked as resolved.

@micah

This comment was marked as resolved.

@nils-schween
Copy link

nils-schween commented Feb 20, 2022

Me too, I also experience the same issue with nextcloud 23.0.2. I added the patch manually. I will report if it worked.

@update: It worked. The errors disappeared.
Thanks for fixing it.

@justinlimbo

This comment was marked as resolved.

@detoxhby

This comment was marked as off-topic.

@syuo7
Copy link

syuo7 commented Mar 2, 2022

Me too, I also experience the same issue with nextcloud 23.0.2. I added the patch manually. I will report if it worked.

How to apply this hot fix?
Thank you.

@nils-schween
Copy link

@syuo7 At the top of this page, there is tab "Commits". If you click it, you will see which file is changed and how it is changed.
In this case it is the file 'lib/private/legacy/OC_Helper.php'. I manually edited this file of my Nextcloud installation on my server, i.e. I added the green parts and removed the red parts. If you do so, make a copy of 'OC_Helper.php', in case it does not work you can restore the file.

@AndyXheli

This comment was marked as resolved.

@psit-kr

This comment was marked as resolved.

@szaimen
Copy link
Contributor

szaimen commented Mar 9, 2022

Yes, the issue will be fixed with 23.0.3 and 22.2.6

@nextcloud nextcloud locked as too heated and limited conversation to collaborators Mar 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3. to review Waiting for reviews
Projects
None yet