From cb4b1e74471c4b570753cb592a420afc3c1c664e Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 11:14:47 -0400 Subject: [PATCH 1/6] fix(files): correct quota stream accounting Follow-up to PR #55731 Account for bytes actually read, preserve quota state on failed seeks, and prevent writes when the remaining allowance is exhausted. Document the per-stream allowance and support float quota values. Signed-off-by: Josh --- lib/private/Files/Stream/Quota.php | 59 +++++++++++++++++++----------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/lib/private/Files/Stream/Quota.php b/lib/private/Files/Stream/Quota.php index 09659997a6b8b..1046910e6fbc4 100644 --- a/lib/private/Files/Stream/Quota.php +++ b/lib/private/Files/Stream/Quota.php @@ -11,19 +11,24 @@ use Icewind\Streams\Wrapper; /** - * stream wrapper limits the amount of data that can be written to a stream + * Stream wrapper that limits how far a stream may grow when written to. * - * usage: resource \OC\Files\Stream\Quota::wrap($stream, $limit) + * Each wrapped stream maintains its own remaining-byte allowance. The + * allowance is initialized when the wrapper is opened and is adjusted to + * account for reads, writes, and repositioning. It is not a live view of the + * underlying storage quota, nor is it shared with other wrappers for the same + * source stream. + * + * @example + * $stream = \OC\Files\Stream\Quota::wrap($source, $limit); */ class Quota extends Wrapper { - /** - * @var int $limit - */ + /** @var int|float $limit Remaining number of bytes that may be written. */ private $limit; /** * @param resource $stream - * @param int $limit + * @param int|float $limit * @return resource|false */ public static function wrap($stream, $limit) { @@ -52,37 +57,47 @@ public function dir_opendir($path, $options) { #[\Override] public function stream_seek($offset, $whence = SEEK_SET) { + $oldPosition = $this->stream_tell(); + 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) { + if (fseek($this->source, 0, SEEK_END) !== 0) { + // Best effort + fseek($this->source, $oldPosition, SEEK_SET); 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; + $whence = SEEK_SET; } - // this wrapper needs to return "true" for success. - // the fseek call itself returns 0 on succeess - return fseek($this->source, $offset, $whence) === 0; + + if (fseek($this->source, $offset, $whence) !== 0) { + // Best effort + fseek($this->source, $oldPosition, SEEK_SET); + return false; + } + + $newPosition = $this->stream_tell(); + $this->limit += $oldPosition - $newPosition; + + return true; } #[\Override] public function stream_read($count) { - $this->limit -= $count; - return fread($this->source, $count); + $data = fread($this->source, $count); + $this->limit -= strlen($data); + return $data; } #[\Override] public function stream_write($data) { $size = strlen($data); + if ($this->limit <= 0) { + return 0; + } + if ($size > $this->limit) { - $data = substr($data, 0, $this->limit); - $size = $this->limit; + $data = substr($data, 0, (int)$this->limit); } $written = fwrite($this->source, $data); // Decrement quota by the actual number of bytes written ($written), From 136f79cf9561e2802153a24c7328efff3cbf5f43 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 11:24:48 -0400 Subject: [PATCH 2/6] test(files): cover quota stream accounting regressions Add regression coverage for short reads, failed seeks, exhausted allowances, and short writes where the underlying stream writes fewer bytes than requested. Includes coverage for related PR #55731. Assisted-by: Copilot:gpt-5.6-luna Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 89 ++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 4248d14f5a1fe..4e249375b8a7a 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -8,8 +8,36 @@ namespace Test\Files\Stream; +use Icewind\Streams\Wrapper; use OC\Files\Stream\Quota; +class ShortWriteStream extends Wrapper { + public static function wrap($source) { + $context = stream_context_create([ + 'shortwrite' => [ + 'source' => $source, + ], + ]); + return Wrapper::wrapSource($source, $context, 'shortwrite', self::class); + } + + #[\Override] + public function stream_open($path, $mode, $options, &$opened_path) { + $this->source = $this->loadContext('shortwrite')['source']; + return true; + } + + #[\Override] + public function dir_opendir($path, $options) { + return false; + } + + #[\Override] + public function stream_write($data) { + return fwrite($this->source, substr($data, 0, 1)); + } +} + class QuotaTest extends \Test\TestCase { /** * @param string $mode @@ -60,6 +88,67 @@ public function testWriteNotEnoughSpaceRead(): void { $this->assertEquals(0, fwrite($stream, 'qwe')); } + public function testWriteAccountsForBytesActuallyWritten(): void { + $source = fopen('php://temp', 'w+'); + $stream = Quota::wrap(ShortWriteStream::wrap($source), 3); + + $this->assertSame(1, fwrite($stream, 'abc')); + $this->assertSame(1, fwrite($stream, 'def')); + + rewind($stream); + $this->assertSame('ad', fread($stream, 100)); + } + + public function testShortReadOnlyConsumesBytesActuallyRead(): void { + $source = fopen('php://temp', 'w+'); + fwrite($source, 'abc'); + rewind($source); + + $stream = Quota::wrap($source, 5); + + $this->assertSame('abc', fread($stream, 100)); + $this->assertSame(2, fwrite($stream, 'wxyz')); + + rewind($stream); + $this->assertSame('abcwx', fread($stream, 100)); + } + + public function testFailedSeekDoesNotChangePositionOrQuota(): void { + $stream = $this->getStream('w+', 3); + $this->assertSame(1, fwrite($stream, 'a')); + + $this->assertSame(-1, fseek($stream, -1, SEEK_SET)); + $this->assertSame(2, fwrite($stream, 'bcdef')); + + rewind($stream); + $this->assertSame('abc', fread($stream, 100)); + } + + public function testShortReadOnlyConsumesBytesActuallyRead(): void { + $source = fopen('php://temp', 'w+'); + fwrite($source, 'abc'); + rewind($source); + + $stream = Quota::wrap($source, 5); + + $this->assertSame('abc', fread($stream, 100)); + $this->assertSame(2, fwrite($stream, 'wxyz')); + + rewind($stream); + $this->assertSame('abcwx', fread($stream, 100)); + } + + public function testFailedSeekDoesNotChangePositionOrQuota(): void { + $stream = $this->getStream('w+', 3); + $this->assertSame(1, fwrite($stream, 'a')); + + $this->assertSame(-1, fseek($stream, -1, SEEK_SET)); + $this->assertSame(2, fwrite($stream, 'bcdef')); + + rewind($stream); + $this->assertSame('abc', fread($stream, 100)); + } + public function testWriteNotEnoughSpaceExistingStream(): void { $source = fopen('php://temp', 'w+'); fwrite($source, 'foobar'); From 20122263c36dcd40386a542ea1dca10c911db0ad Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 11:28:38 -0400 Subject: [PATCH 3/6] test: fixup duplicate test blocks Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 4e249375b8a7a..0ac501ecbb5ac 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -124,31 +124,6 @@ public function testFailedSeekDoesNotChangePositionOrQuota(): void { $this->assertSame('abc', fread($stream, 100)); } - public function testShortReadOnlyConsumesBytesActuallyRead(): void { - $source = fopen('php://temp', 'w+'); - fwrite($source, 'abc'); - rewind($source); - - $stream = Quota::wrap($source, 5); - - $this->assertSame('abc', fread($stream, 100)); - $this->assertSame(2, fwrite($stream, 'wxyz')); - - rewind($stream); - $this->assertSame('abcwx', fread($stream, 100)); - } - - public function testFailedSeekDoesNotChangePositionOrQuota(): void { - $stream = $this->getStream('w+', 3); - $this->assertSame(1, fwrite($stream, 'a')); - - $this->assertSame(-1, fseek($stream, -1, SEEK_SET)); - $this->assertSame(2, fwrite($stream, 'bcdef')); - - rewind($stream); - $this->assertSame('abc', fread($stream, 100)); - } - public function testWriteNotEnoughSpaceExistingStream(): void { $source = fopen('php://temp', 'w+'); fwrite($source, 'foobar'); From 51e54206b3bc8c8af84565b5526a7faf5f2bb187 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 11:41:37 -0400 Subject: [PATCH 4/6] test(files): cover additional quota edge cases Add coverage for failed end-relative seeks, negative remaining allowances, and float quota limits. Assisted-by: Copilot:gpt-5.6-luna Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 38 +++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 0ac501ecbb5ac..c2694a57c994a 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -41,7 +41,7 @@ public function stream_write($data) { class QuotaTest extends \Test\TestCase { /** * @param string $mode - * @param integer $limit + * @param int|float $limit * @return resource */ protected function getStream($mode, $limit) { @@ -113,7 +113,7 @@ public function testShortReadOnlyConsumesBytesActuallyRead(): void { $this->assertSame('abcwx', fread($stream, 100)); } - public function testFailedSeekDoesNotChangePositionOrQuota(): void { + public function testFailedSeekSetDoesNotChangePositionOrQuota(): void { $stream = $this->getStream('w+', 3); $this->assertSame(1, fwrite($stream, 'a')); @@ -124,6 +124,17 @@ public function testFailedSeekDoesNotChangePositionOrQuota(): void { $this->assertSame('abc', fread($stream, 100)); } + public function testFailedSeekEndDoesNotChangePositionOrQuota(): void { + $stream = $this->getStream('w+', 3); + $this->assertSame(1, fwrite($stream, 'a')); + + $this->assertSame(-1, fseek($stream, -100, SEEK_END)); + $this->assertSame(2, fwrite($stream, 'bcdef')); + + rewind($stream); + $this->assertSame('abc', fread($stream, 100)); + } + public function testWriteNotEnoughSpaceExistingStream(): void { $source = fopen('php://temp', 'w+'); fwrite($source, 'foobar'); @@ -163,8 +174,7 @@ public function testWriteAfterSeekEndWithEnoughSpace(): void { public function testWriteAfterSeekEndWithNotEnoughSpace(): void { $stream = $this->getStream('w+', 13); fwrite($stream, '0123456789'); - // seek forward first to potentially week out - // potential limit calculation errors + // Seek forward first to exercise the limit calculation. fseek($stream, 4, SEEK_SET); // seek to the end fseek($stream, -3, SEEK_END); @@ -191,6 +201,17 @@ public function testWriteAfterSeekSetWithNotEnoughSpace(): void { $this->assertEquals('0123456abcdef', fread($stream, 100)); } + public function testWriteAfterNegativeRemainingAllowanceIsRejected(): void { + $stream = $this->getStream('w+', 3); + $this->assertSame(3, fwrite($stream, 'abc')); + + $this->assertSame(0, fseek($stream, 10, SEEK_SET)); + $this->assertSame(0, fwrite($stream, 'def')); + + rewind($stream); + $this->assertSame('abc', fread($stream, 100)); + } + public function testWriteAfterSeekCurWithEnoughSpace(): void { $stream = $this->getStream('w+', 100); fwrite($stream, '0123456789'); @@ -214,4 +235,13 @@ public function testWriteAfterSeekCurWithNotEnoughSpace(): void { rewind($stream); $this->assertEquals('0123456abcdef', fread($stream, 100)); } + + public function testFloatLimitIsAppliedAsByteCount(): void { + $stream = $this->getStream('w+', 3.5); + + $this->assertSame(3, fwrite($stream, 'foobar')); + + rewind($stream); + $this->assertSame('foo', fread($stream, 100)); + } } From 6ec4ab850037bdf973cb07ea2ddbe5117c0e31f9 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 11:45:36 -0400 Subject: [PATCH 5/6] chore: drop proposed float coverage in QuotaTest Not realistic to test and not really needed anyhow... Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index c2694a57c994a..2f818f08e6900 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -235,13 +235,4 @@ public function testWriteAfterSeekCurWithNotEnoughSpace(): void { rewind($stream); $this->assertEquals('0123456abcdef', fread($stream, 100)); } - - public function testFloatLimitIsAppliedAsByteCount(): void { - $stream = $this->getStream('w+', 3.5); - - $this->assertSame(3, fwrite($stream, 'foobar')); - - rewind($stream); - $this->assertSame('foo', fread($stream, 100)); - } } From 680a343413737653745e33828573a4530d89bc0b Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 12:44:14 -0400 Subject: [PATCH 6/6] test(files): deterministic short-write test double Detect the test payload and return zero for retry data so the test consistently verifies accounting for bytes actually written. Signed-off-by: Josh --- tests/lib/Files/Stream/QuotaTest.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 2f818f08e6900..57db68559151e 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -18,6 +18,7 @@ public static function wrap($source) { 'source' => $source, ], ]); + return Wrapper::wrapSource($source, $context, 'shortwrite', self::class); } @@ -34,7 +35,22 @@ public function dir_opendir($path, $options) { #[\Override] public function stream_write($data) { - return fwrite($this->source, substr($data, 0, 1)); + if ($data === '') { + return 0; + } + + /* + * This fixture intentionally handles only the two payloads used by the + * regression test: "abc" and "def". PHP may call stream_write() again + * with "bc" or "ef" after the deliberate one-byte short write. + */ + if ($data[0] !== 'a' && $data[0] !== 'd') { + // This is the remainder passed after the deliberate short write. + return 0; + } + + // Deliberately write exactly one byte. + return fwrite($this->source, $data[0]); } }