Fix/57375 zero quota external storage folder delete#59986
Conversation
huntervcx
requested review from
ArtificialOwl,
come-nc,
provokateurin and
sorbaugh
and removed request for
a team
Behavior Matrix After Full Fix
I'm still not entirely certain whether the behavior for trashbin and versions is fully aligned with the intended design. That said, with this fix, it is now possible to create directories in the user's home even when the quota is set to 0. From Nextcloud’s perspective, this is technically consistent, as empty directories do not consume any storage (0 bytes), and therefore do not violate the quota constraint. |
huntervcx
force-pushed
the
fix/57375-zero-quota-external-storage-folder-delete
branch
from
8a910a1 to
e4ce8ad
Compare
| // Creating an empty directory (MKCOL) does not consume user quota bytes: | ||
| // it is only a cache / metadata entry. Leave the final decision to the | ||
| // target storage (the Quota wrapper is only attached to the home storage | ||
| // and can still refuse if needed; external mounts have no quota wrapper). | ||
| // Without this short-circuit, a user whose quota is intentionally set to 0 | ||
| // (so that all content lives on external mounts) cannot create folders at | ||
| // all, including paths that actually resolve to an external mount. | ||
| if ($isDir) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
If I remember correctly this is not an acceptable behaviour, because it means a user with 0 quota can fill your storage space by creating empty directories, which still use up disk space.
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
When the user quota is exhausted (notably quota=0), writes to metadata locations like files_trashbin/ and files_versions/ were incorrectly gated on free_space == 0 the same way as real files under files/. The most visible symptom is on DELETE of an item on an external mount when files_trashbin is enabled: the move-to-trash copy lands in files_trashbin/ which has no quota meaning, yet Quota::copyFromStorage refused it because the home free_space is 0. DAV then returned HTTP 403. Honor shouldApplyQuota() in the remaining write paths so the existing "only files/ counts against user quota" intent is consistent across all of them: file_put_contents, copy, copyFromStorage, moveFromStorage, touch and writeStream. mkdir is left as-is because empty directories still consume disk inodes and must not be allowed on the home when the quota is 0 (would otherwise let a user fill the underlying disk). Signed-off-by: Arthur Perrot <aperrot@dyb.fr>
huntervcx
force-pushed
the
fix/57375-zero-quota-external-storage-folder-delete
branch
from
e4ce8ad to
9643b08
Compare
2 participants
Summary
Users with quota set to 0 (intentionally, so nothing is stored under their home
files/except structure / external mounts) hit incorrect WebDAV failures:MKCOL / uploads / moves could fail with
507 Insufficient StoragebecauseQuotaPlugintreated directory creation like a fixed byte write (4096) againstfree_space == 0, and the homeQuotawrapper blockedmkdirwhen quota was exhausted.With
files_trashbinenabled, DELETE on items on external storage could return 403 Forbidden (Directory::delete→rmdirfalse) becauseQuota::copyFromStorage/ related write paths did not honorshouldApplyQuota()— writes underfiles_trashbin/were incorrectly subject to the samefree_space == 0gate as real user files underfiles/.This PR aligns quota enforcement with the existing intent of
shouldApplyQuota()(only paths underfiles/count toward user quota):QuotaPlugin: skip pre-quota checks for directory creation (MKCOL); empty dirs do not consume user quota bytes.Quotawrapper: allowmkdirwhen quota is exactly0for empty directories; delegate real file blocking tofopen/file_put_contents/ etc.Quotawrapper: applyshouldApplyQuota()tocopyFromStorage,moveFromStorage,copy,file_put_contents, andtouchso trashbin / versions / metadata paths are not blocked when home quota is0.Unit tests updated in
tests/lib/Files/Storage/Wrapper/QuotaTest.phpandapps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php.TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)