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), diff --git a/tests/lib/Files/Stream/QuotaTest.php b/tests/lib/Files/Stream/QuotaTest.php index 4248d14f5a1fe..57db68559151e 100644 --- a/tests/lib/Files/Stream/QuotaTest.php +++ b/tests/lib/Files/Stream/QuotaTest.php @@ -8,12 +8,56 @@ 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) { + 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]); + } +} + class QuotaTest extends \Test\TestCase { /** * @param string $mode - * @param integer $limit + * @param int|float $limit * @return resource */ protected function getStream($mode, $limit) { @@ -60,6 +104,53 @@ 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 testFailedSeekSetDoesNotChangePositionOrQuota(): 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 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'); @@ -99,8 +190,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); @@ -127,6 +217,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');