From 46d1a5019df07addd17649407933e012f73233ff Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 9 Jan 2026 12:11:49 -0500 Subject: [PATCH 1/3] test(files): add tests to confirm reads don't impact quota in stream_read Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 4248d14f5a1fe..c3fc0ceec1a26 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -28,6 +28,25 @@ public function testWriteEnoughSpace(): void { $this->assertEquals('foobar', fread($stream, 100)); } + public function testQuotaNotReducedByReadBeforeWrite() { + $stream = $this->getStream('w+', 6); // Initial quota: 6 + // Read 3 bytes before writing, quota should remain 6 + $this->assertEquals('', fread($stream, 3)); + // Should allow writing 6 bytes, as reads do NOT affect write quota + $this->assertEquals(6, fwrite($stream, 'foobar')); + } + + public function testQuotaWriteFollowedByReadDoesNotAffectWriteQuota() { + $stream = $this->getStream('w+', 6); // Initial quota: 6 + // Write exactly up to quota + $this->assertEquals(6, fwrite($stream, 'foobar')); + // Read from stream - should NOT affect quota or prevent further (failing) writes + rewind($stream); + $this->assertEquals('foobar', fread($stream, 6)); + // Try another write - should fail (quota exhausted) + $this->assertEquals(0, fwrite($stream, 'abc')); + } + public function testWriteNotEnoughSpace(): void { $stream = $this->getStream('w+', 3); $this->assertEquals(3, fwrite($stream, 'foobar')); From 4de2d4da579bb89bc79b2fb2be1f158307f9c999 Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 9 Jan 2026 21:43:16 -0500 Subject: [PATCH 2/3] fix(files/stream): enforce quota only on file growth in Quota stream Refactors Quota stream logic so that quota is only decremented when writes grow the file, not on overwrites, reads, or seeks. This ensures quota enforcement is robust and matches expected storage semantics. Adds extra safety checks after writing to prevent quota drift. Follow-up to nextcloud/server#55731 Signed-off-by: Josh --- lib/private/Files/Stream/Quota.php | 57 ++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/lib/private/Files/Stream/Quota.php b/lib/private/Files/Stream/Quota.php index 3c7e09c633bc7..79d651d400edb 100644 --- a/lib/private/Files/Stream/Quota.php +++ b/lib/private/Files/Stream/Quota.php @@ -48,40 +48,59 @@ public function dir_opendir($path, $options) { } public function stream_seek($offset, $whence = SEEK_SET) { - if ($whence === SEEK_END) { - // go to the end to find out last position's offset - $oldOffset = $this->stream_tell(); - if (fseek($this->source, 0, $whence) !== 0) { - return false; - } - $whence = SEEK_SET; - $offset = $this->stream_tell() + $offset; - $this->limit += $oldOffset - $offset; - } elseif ($whence === SEEK_SET) { - $this->limit += $this->stream_tell() - $offset; - } else { - $this->limit -= $offset; - } // this wrapper needs to return "true" for success. // the fseek call itself returns 0 on succeess return fseek($this->source, $offset, $whence) === 0; } public function stream_read($count) { - $this->limit -= $count; + // Do not decrement $this->limit for reads return fread($this->source, $count); } public function stream_write($data) { $size = strlen($data); - if ($size > $this->limit) { - $data = substr($data, 0, $this->limit); - $size = $this->limit; + + // Get current pointer and file size + $curPos = ftell($this->source); + fseek($this->source, 0, SEEK_END); + $fileSize = ftell($this->source); + fseek($this->source, $curPos, SEEK_SET); + + $writeEnd = $curPos + $size; + + // Calculate how many bytes are "new" (beyond end of existing) + $newBytes = max(0, $writeEnd - $fileSize); + + // Enforce quota for new bytes only + if ($newBytes > $this->limit) { + // Only this many new bytes are permitted: + $allowedNewBytes = $this->limit; + // Adjust write size to fit quota + // Calculate max amount we can write, given cursor position and allowed new bytes + $allowedSize = $size - ($newBytes - $allowedNewBytes); + + if ($allowedSize <= 0) { + return 0; // No new bytes allowed + } + $data = substr($data, 0, $allowedSize); + $size = $allowedSize; + // Recalculate position/write end after truncation (for safety) + $writeEnd = $curPos + $size; + $newBytes = max(0, $writeEnd - $fileSize); } + $written = fwrite($this->source, $data); + + // Decrement limit by actually written new bytes + // (Extra safety: recalculate actual new bytes in case fwrite was truncated) + $actualWriteEnd = ftell($this->source); + $actualFileSize = max($fileSize, $actualWriteEnd); + $actualNewBytes = max(0, $actualFileSize - $fileSize); // Decrement quota by the actual number of bytes written ($written), // not the intended size - $this->limit -= $written; + $this->limit -= $actualNewBytes; + return $written; } } From 92e8288c78c6e9fcaca1bdab7f01e451e5487c5a Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 9 Jan 2026 21:45:02 -0500 Subject: [PATCH 3/3] test(files/stream): add and expand quota stream tests for seeks and overwrites Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 45 +++++++++++++++++++--------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index c3fc0ceec1a26..c1ddcd1546d13 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -28,23 +28,40 @@ public function testWriteEnoughSpace(): void { $this->assertEquals('foobar', fread($stream, 100)); } - public function testQuotaNotReducedByReadBeforeWrite() { - $stream = $this->getStream('w+', 6); // Initial quota: 6 - // Read 3 bytes before writing, quota should remain 6 - $this->assertEquals('', fread($stream, 3)); - // Should allow writing 6 bytes, as reads do NOT affect write quota - $this->assertEquals(6, fwrite($stream, 'foobar')); + // Quota is not reduced by reads or seeks + public function testReadsAndSeeksDoNotAffectQuota() { + $stream = $this->getStream('w+', 6); + $this->assertEquals('', fread($stream, 3)); // Read before write + $this->assertEquals(6, fwrite($stream, 'foobar')); // Write to fill quota + rewind($stream); + $this->assertEquals('foo', fread($stream, 3)); + fseek($stream, 1, SEEK_CUR); // Seek should not affect quota + $this->assertEquals(0, fwrite($stream, 'baz')); // No quota left for growth + rewind($stream); + $this->assertEquals('foobar', fread($stream, 100)); } - public function testQuotaWriteFollowedByReadDoesNotAffectWriteQuota() { - $stream = $this->getStream('w+', 6); // Initial quota: 6 - // Write exactly up to quota - $this->assertEquals(6, fwrite($stream, 'foobar')); - // Read from stream - should NOT affect quota or prevent further (failing) writes + // Overwriting after quota is exhausted should succeed + public function testOverwriteAfterQuotaExhausted() { + $stream = $this->getStream('w+', 3); + fwrite($stream, 'abc'); // Fill quota + $this->assertEquals(0, fwrite($stream, 'd')); // No quota for growth rewind($stream); - $this->assertEquals('foobar', fread($stream, 6)); - // Try another write - should fail (quota exhausted) - $this->assertEquals(0, fwrite($stream, 'abc')); + $this->assertEquals(3, fwrite($stream, 'xyz')); // Overwrite: should succeed + rewind($stream); + $this->assertEquals('xyz', fread($stream, 100)); + } + + // Quota is only used for file growth + public function testQuotaOnlyUsedForGrowth() { + $stream = $this->getStream('w+', 5); + $this->assertEquals(5, fwrite($stream, '12345')); // Fill quota + rewind($stream); + $this->assertEquals(3, fwrite($stream, 'abc')); // Overwrite allowed + fseek($stream, 5, SEEK_SET); + $this->assertEquals(0, fwrite($stream, 'xx')); // No quota for extension + rewind($stream); + $this->assertEquals('abc45', fread($stream, 100)); } public function testWriteNotEnoughSpace(): void {