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

Fix public shared folder page when quota includes external storages #28911

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Sep 20, 2021

Fixes #27322

When the quota storage wrapper is used (that is, when the share owner has a quota set) and quota_include_external_storage is enabled when checking the free space Filesystem::getFileInfo('', 'ext') is called, which requires the file system to have been initialized. This is explicitly done now in showShare similar to what is done in downloadShare.

Pending:

  • Fix the issue :-) For now this just includes an integration test that shows it.
  • Verify that the fix is the proper one and not just a dirty hack. Maybe something should be fixed instead in the Quota wrapper or the Filesystem itself?

How to reproduce

  • Log in to Nextcloud as an admin
  • Create a user
  • Set a quota for that user (for example, 1 GB)
  • Enable quota_include_external_storage, for example, with occ config:system:set quota_include_external_storage --value=true --type boolean
    • Note that you do not need to setup an external storage, only to enable the setting
  • Log in as the user
  • Create a new folder
  • Create a public share for that folder
  • Open the link to the public share

Result with this pull request

The public shared folder page is opened

Result without this pull request

The server returns an internal error

@danxuliu danxuliu added bug 2. developing Work in progress labels Sep 20, 2021
@danxuliu danxuliu added this to the Nextcloud 23 milestone Sep 20, 2021
Comment on lines +410 to +411
OC_Util::tearDownFS();
OC_Util::setupFS($share->getShareOwner());
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this might have some performance implications. So probably @icewind1991 should have a look when he is back from vacation.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the quota storage wrapper is used (that is, when the share owner
has a quota set) and "quota_include_external_storage" is enabled when
checking the free space "Filesystem::getFileInfo('', 'ext')" is called,
which requires the file system to have been initialized. This is
explicitly done now in "showShare" similar to what is done in
"downloadShare".

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-public-shared-folder-page-when-quota-includes-external-storages branch from 686abcc to a43d664 Compare September 22, 2021 14:24
@danxuliu
Copy link
Member Author

/backport to stable22

@danxuliu
Copy link
Member Author

/backport to stable21

@danxuliu danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 22, 2021
@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv
Copy link
Member

What's the status here?

@szaimen
Copy link
Contributor

szaimen commented Oct 22, 2021

It needs feedback from @icewind1991 ...

This was referenced Oct 25, 2021
@blizzz blizzz mentioned this pull request Nov 3, 2021
18 tasks
@skjnldsv skjnldsv mentioned this pull request Nov 8, 2021
23 tasks
@blizzz blizzz modified the milestones: Nextcloud 23, Nextcloud 24 Nov 30, 2021
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
This was referenced Apr 7, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
@skjnldsv skjnldsv mentioned this pull request Aug 12, 2022
@blizzz blizzz mentioned this pull request Sep 9, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

As there is no feedback since a while I will close this ticket.
If you will decide to work on this feature again and if it hasn't been fixed or implemented already, feel free to re-open and solve the various conflicts.

@skjnldsv skjnldsv closed this Feb 27, 2024
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 27, 2024
@skjnldsv skjnldsv deleted the fix-public-shared-folder-page-when-quota-includes-external-storages branch February 27, 2024 17:48
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal server error with shared links as user
4 participants