From 9643b0833bdf253280fcdf2f37762071524c78b9 Mon Sep 17 00:00:00 2001 From: Arthur Perrot Date: Sat, 16 May 2026 15:39:12 +0200 Subject: [PATCH] fix: do not apply user quota to writes outside files/ 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 --- lib/private/Files/Storage/Wrapper/Quota.php | 25 ++++++--- tests/lib/Files/Storage/Wrapper/QuotaTest.php | 51 ++++++++++++++++++- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Quota.php b/lib/private/Files/Storage/Wrapper/Quota.php index 11b992dbf9727..e085136579725 100644 --- a/lib/private/Files/Storage/Wrapper/Quota.php +++ b/lib/private/Files/Storage/Wrapper/Quota.php @@ -100,7 +100,11 @@ public function file_put_contents(string $path, mixed $data): int|float|false { return $this->getWrapperStorage()->file_put_contents($path, $data); } $free = $this->free_space($path); - if ($free < 0 || strlen($data) < $free) { + // Only apply quota for files under the user's "files/" tree. + // Writes to metadata locations (files_trashbin/, files_versions/, ...) + // must not be blocked, otherwise features like the trashbin break + // for users whose quota happens to be exhausted (notably quota=0). + if ($free < 0 || !$this->shouldApplyQuota($path) || strlen($data) < $free) { return $this->getWrapperStorage()->file_put_contents($path, $data); } else { return false; @@ -113,7 +117,7 @@ public function copy(string $source, string $target): bool { return $this->getWrapperStorage()->copy($source, $target); } $free = $this->free_space($target); - if ($free < 0 || $this->getSize($source) < $free) { + if ($free < 0 || !$this->shouldApplyQuota($target) || $this->getSize($source) < $free) { return $this->getWrapperStorage()->copy($source, $target); } else { return false; @@ -167,7 +171,12 @@ public function copyFromStorage(IStorage $sourceStorage, string $sourceInternalP return $this->getWrapperStorage()->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } $free = $this->free_space($targetInternalPath); - if ($free < 0 || $this->getSize($sourceInternalPath, $sourceStorage) < $free) { + // Skip the quota check when the target lives outside of "files/" + // (e.g. files_trashbin/, files_versions/). This is essential so that + // the trashbin can store deleted items even when the user's quota is + // fully consumed: otherwise DELETE operations on external mounts fail + // with HTTP 403 because the move-to-trash copy returns false. + if ($free < 0 || !$this->shouldApplyQuota($targetInternalPath) || $this->getSize($sourceInternalPath, $sourceStorage) < $free) { return $this->getWrapperStorage()->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } else { return false; @@ -180,7 +189,7 @@ public function moveFromStorage(IStorage $sourceStorage, string $sourceInternalP return $this->getWrapperStorage()->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } $free = $this->free_space($targetInternalPath); - if ($free < 0 || $this->getSize($sourceInternalPath, $sourceStorage) < $free) { + if ($free < 0 || !$this->shouldApplyQuota($targetInternalPath) || $this->getSize($sourceInternalPath, $sourceStorage) < $free) { return $this->getWrapperStorage()->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } else { return false; @@ -206,7 +215,9 @@ public function touch(string $path, ?int $mtime = null): bool { return $this->getWrapperStorage()->touch($path, $mtime); } $free = $this->free_space($path); - if ($free == 0) { + // Same rule as the other write paths: only block when the target is + // actually under the user-quota controlled "files/" tree. + if ($free == 0 && $this->shouldApplyQuota($path)) { return false; } @@ -219,12 +230,12 @@ public function enableQuota(bool $enabled): void { #[\Override] public function writeStream(string $path, $stream, ?int $size = null): int { - if (!$this->hasQuota()) { + if (!$this->hasQuota() || !$this->shouldApplyQuota($path)) { return parent::writeStream($path, $stream, $size); } $free = $this->free_space($path); - if ($this->shouldApplyQuota($path) && $free == 0) { + if ($free == 0) { throw new NotEnoughSpaceException(); } diff --git a/tests/lib/Files/Storage/Wrapper/QuotaTest.php b/tests/lib/Files/Storage/Wrapper/QuotaTest.php index ac658031ff3fe..ac499c636889a 100644 --- a/tests/lib/Files/Storage/Wrapper/QuotaTest.php +++ b/tests/lib/Files/Storage/Wrapper/QuotaTest.php @@ -225,9 +225,13 @@ public function testMkdirQuotaZeroTrashbin(): void { $this->assertTrue($instance->mkdir('cache')); } - public function testNoTouchQuotaZero(): void { + public function testTouchBlockedUnderFilesWhenQuotaIsZero(): void { $instance = $this->getLimitedStorage(0.0); - $this->assertFalse($instance->touch('foobar')); + // touch is blocked only for paths under files/ (user quota area) + $this->assertFalse($instance->touch('files/foobar')); + // touch outside files/ (trashbin, versions, ...) must remain allowed + $this->assertTrue($instance->mkdir('files_trashbin')); + $this->assertTrue($instance->touch('files_trashbin/foobar')); } public function testNoFopenQuotaZero(): void { @@ -256,4 +260,47 @@ public function testNoWriteStreamQuotaZero(): void { $this->expectException(Files\NotEnoughSpaceException::class); $instance->writeStream('files/test.txt', $stream); } + + /** + * writeStream outside of files/ (trashbin, versions, ...) must succeed + * even when the user quota is exhausted. + */ + public function testWriteStreamAllowedOutsideFilesWhenQuotaIsZero(): void { + $instance = $this->getLimitedStorage(0.0); + $this->assertTrue($instance->mkdir('files_trashbin')); + $stream = fopen('php://temp', 'w+'); + fwrite($stream, 'foo'); + rewind($stream); + $this->assertEquals(3, $instance->writeStream('files_trashbin/test.txt', $stream)); + } + + /** + * Writes under "files/" must still be blocked when quota is 0, but writes + * outside that prefix (trashbin metadata, versions, ...) must not be + * blocked, otherwise features like the trashbin break for users whose + * quota happens to be exhausted (notably quota=0). + */ + public function testFilePutContentsBlockedUnderFilesWhenQuotaIsZero(): void { + $instance = $this->getLimitedStorage(0.0); + $this->assertFalse($instance->file_put_contents('files/foo', 'x')); + $this->assertTrue($instance->mkdir('files_trashbin')); + $this->assertNotFalse($instance->file_put_contents('files_trashbin/foo.json', '{}')); + } + + /** + * Copying from another storage (e.g. an external SMB mount) into the + * trashbin must succeed even when the quota is exhausted: this is what + * happens on every DELETE when files_trashbin is enabled. Conversely, + * copying into "files/" must still be blocked. + */ + public function testCopyFromStorageAllowedToTrashbinWhenQuotaIsZero(): void { + $source = new Local(['datadir' => $this->tmpDir]); + $source->file_put_contents('source.txt', 'hello'); + + $instance = $this->getLimitedStorage(0.0); + $this->assertTrue($instance->mkdir('files_trashbin')); + $this->assertTrue($instance->copyFromStorage($source, 'source.txt', 'files_trashbin/source.txt')); + + $this->assertFalse($instance->copyFromStorage($source, 'source.txt', 'files/source.txt')); + } }