fix(files): correct quota stream accounting

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),

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');