From 162b0a9827d690d80fe44abe5a2b20fa2f7f9503 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 5 Sep 2026 10:55:20 -0400 Subject: [PATCH 1/5] fix(files): bound seekable HTTP reads by remote size Limit reads to the remaining bytes reported by `Content-Range` and determine EOF from the logical resource offset instead of the underlying HTTP stream. This avoids reading beyond the remote resource boundary when the response connection remains open. The change is safe because this wrapper already requires `Content-Range` and uses its total-size component to establish the remote resource boundary; the read cap is consistent with the existing contract. Signed-off-by: Josh --- .../Files/Stream/SeekableHttpStream.php | 51 +++++++++++++------ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/lib/private/Files/Stream/SeekableHttpStream.php b/lib/private/Files/Stream/SeekableHttpStream.php index 6b6ab08cbbc1c..7c8792c18cfc4 100644 --- a/lib/private/Files/Stream/SeekableHttpStream.php +++ b/lib/private/Files/Stream/SeekableHttpStream.php @@ -56,12 +56,13 @@ public static function open(callable $callback) { /** @var ?resource|closed-resource */ private $current; - /** @var int $offset offset of the current chunk */ + + /** Absolute offset within the remote resource represented by this stream */ private int $offset = 0; - /** @var int $length length of the current chunk */ - private int $length = 0; - /** @var int $totalSize size of the full stream */ + + /** Total size of the remote resource represented by this stream. */ private int $totalSize = 0; + private bool $needReconnect = false; private function reconnect(int $start): bool { @@ -104,7 +105,7 @@ private function reconnect(int $start): bool { $content = trim(explode(':', $contentRange)[1]); $range = trim(explode(' ', $content)[1]); $begin = intval(explode('-', $range)[0]); - $length = intval(explode('/', $range)[1]); + $totalSize = intval(explode('/', $range)[1]); if ($begin !== $start) { $this->current = null; @@ -112,9 +113,11 @@ private function reconnect(int $start): bool { } $this->offset = $begin; - $this->length = $length; if ($start === 0) { - $this->totalSize = $length; + $this->totalSize = $totalSize; + } elseif ($this->totalSize !== $totalSize) { + $this->current = null; + return false; } return true; @@ -152,11 +155,27 @@ public function stream_open($path, $mode, $options, &$opened_path) { #[\Override] public function stream_read($count) { - if (!$this->getCurrent()) { + $stream = $this->getCurrent(); + if (!$stream) { + return false; + } + + if ($count <= 0) { + return ''; + } + + $remaining = $this->totalSize - $this->offset; + if ($remaining <= 0) { + return ''; + } + + $ret = fread($stream, min($count, $remaining)); + if ($ret === false) { return false; } - $ret = fread($this->getCurrent(), $count); + $this->offset += strlen($ret); + return $ret; } @@ -178,12 +197,12 @@ public function stream_seek($offset, $whence = SEEK_SET) { } break; case SEEK_END: - if ($this->length === 0) { + if ($this->totalSize === 0) { return false; - } elseif ($this->length + $offset === $this->offset) { + } elseif ($this->totalSize + $offset === $this->offset) { return true; } else { - $this->offset = $this->length + $offset; + $this->offset = $this->totalSize + $offset; } break; } @@ -216,11 +235,11 @@ public function stream_stat() { #[\Override] public function stream_eof() { - if ($this->getCurrent()) { - return feof($this->getCurrent()); - } else { + if (!$this->getCurrent()) { return true; - } + } + + return $this->offset >= $this->totalSize; } #[\Override] From d963480cd48ab94728db7d778508b24eddfdb34a Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 09:01:00 -0400 Subject: [PATCH 2/5] refactor(files): clean up seekable HTTP range handling Replace the ambiguous range-length state with an explicit total size, validate Content-Range responses, and improve stream cleanup during reconnect failures. Signed-off-by: Josh --- .../Files/Stream/SeekableHttpStream.php | 184 +++++++++++------- 1 file changed, 116 insertions(+), 68 deletions(-) diff --git a/lib/private/Files/Stream/SeekableHttpStream.php b/lib/private/Files/Stream/SeekableHttpStream.php index 7c8792c18cfc4..213502e115ed1 100644 --- a/lib/private/Files/Stream/SeekableHttpStream.php +++ b/lib/private/Files/Stream/SeekableHttpStream.php @@ -11,17 +11,17 @@ use Icewind\Streams\Wrapper; /** - * A stream wrapper that uses http range requests to provide a seekable stream for http reading + * A stream wrapper that uses HTTP range requests to provide a seekable stream + * for HTTP reading. */ class SeekableHttpStream implements File { private const string PROTOCOL = 'httpseek'; /** - * Registers the stream wrapper using the `httpseek://` url scheme - * $return void + * Registers the stream wrapper using the `httpseek://` URL scheme. */ - private static function registerIfNeeded() { - if (!in_array(self::PROTOCOL, stream_get_wrappers())) { + private static function registerIfNeeded(): void { + if (!in_array(self::PROTOCOL, stream_get_wrappers(), true)) { stream_wrapper_register( self::PROTOCOL, self::class @@ -30,34 +30,37 @@ private static function registerIfNeeded() { } /** - * Open a readonly-seekable http stream + * Opens a read-only, seekable HTTP stream. * - * The provided callback will be called with byte range and should return an http stream for the requested range + * The callback is called with a byte range and must return an HTTP stream + * for that range. * - * @param callable $callback - * @return false|resource + * @param callable(string): resource|false $callback + * + * @return resource|false */ public static function open(callable $callback) { $context = stream_context_create([ - SeekableHttpStream::PROTOCOL => [ + self::PROTOCOL => [ 'callback' => $callback ], ]); - SeekableHttpStream::registerIfNeeded(); - return fopen(SeekableHttpStream::PROTOCOL . '://', 'r', false, $context); + self::registerIfNeeded(); + + return fopen(self::PROTOCOL . '://', 'r', false, $context); } /** @var resource */ public $context; - /** @var callable */ + /** @var callable(string): resource|false */ private $openCallback; /** @var ?resource|closed-resource */ - private $current; + private $current = null; - /** Absolute offset within the remote resource represented by this stream */ + /** Absolute offset within the remote resource represented by this stream. */ private int $offset = 0; /** Total size of the remote resource represented by this stream. */ @@ -65,19 +68,56 @@ public static function open(callable $callback) { private bool $needReconnect = false; + /** + * @param array $responseHeaders + * @return array{begin: int, end: int, totalSize: int}|null + */ + private function parseContentRange(array $responseHeaders): ?array { + foreach ($responseHeaders as $header) { + if (!is_string($header)) { + continue; + } + + if (preg_match( + '/^content-range:\s*bytes\s+(\d+)-(\d+)\/(\d+)\s*$/i', + $header, + $matches + ) !== 1) { + continue; + } + + $begin = (int)$matches[1]; + $end = (int)$matches[2]; + $totalSize = (int)$matches[3]; + + if ($end < $begin || $totalSize <= $end) { + return null; + } + + return [ + 'begin' => $begin, + 'end' => $end, + 'totalSize' => $totalSize, + ]; + } + + return null; + } + private function reconnect(int $start): bool { - $this->needReconnect = false; - $range = $start . '-'; - if ($this->hasOpenStream()) { - fclose($this->current); + if ($start < 0) { + return false; } - $stream = ($this->openCallback)($range); + $this->closeCurrent(); + $range = $start . '-'; + $stream = ($this->openCallback)($range); if ($stream === false) { - $this->current = null; + $this->closeCurrent(); return false; } + $this->current = $stream; $responseHead = stream_get_meta_data($this->current)['wrapper_data']; @@ -90,61 +130,66 @@ private function reconnect(int $start): bool { continue 2; } } - throw new \Exception('Failed to get source stream from stream wrapper of ' . get_class($responseHead)); + + $this->closeCurrent(); + throw new \Exception( + 'Failed to get source stream from stream wrapper of ' . get_class($responseHead) + ); } - $rangeHeaders = array_values(array_filter($responseHead, function ($v) { - return preg_match('#^content-range:#i', $v) === 1; - })); - if (!$rangeHeaders) { - $this->current = null; + if (!is_array($responseHead)) { + $this->closeCurrent(); return false; } - $contentRange = $rangeHeaders[0]; - $content = trim(explode(':', $contentRange)[1]); - $range = trim(explode(' ', $content)[1]); - $begin = intval(explode('-', $range)[0]); - $totalSize = intval(explode('/', $range)[1]); + $contentRange = $this->parseContentRange($responseHead); - if ($begin !== $start) { - $this->current = null; + if ($contentRange === null || $contentRange['begin'] !== $start) { + $this->closeCurrent(); return false; } - $this->offset = $begin; if ($start === 0) { - $this->totalSize = $totalSize; - } elseif ($this->totalSize !== $totalSize) { - $this->current = null; + $this->totalSize = $contentRange['totalSize']; + } elseif ($this->totalSize !== $contentRange['totalSize']) { + $this->closeCurrent(); return false; } + $this->offset = $contentRange['begin']; + $this->needReconnect = false; + return true; } /** - * @return ?resource + * @return resource|null */ private function getCurrent() { - if ($this->needReconnect) { - $this->reconnect($this->offset); - } - if (is_resource($this->current)) { - return $this->current; - } else { + if ($this->needReconnect && !$this->reconnect($this->offset)) { return null; } + + return $this->hasOpenStream() ? $this->current : null; } /** * @return bool + * * @psalm-assert-if-true resource $this->current */ private function hasOpenStream(): bool { return is_resource($this->current); } + private function closeCurrent(): void { + if ($this->hasOpenStream()) { + fclose($this->current); + } + + $this->current = null; + } + #[\Override] public function stream_open($path, $mode, $options, &$opened_path) { $options = stream_context_get_options($this->context)[self::PROTOCOL]; @@ -155,11 +200,6 @@ public function stream_open($path, $mode, $options, &$opened_path) { #[\Override] public function stream_read($count) { - $stream = $this->getCurrent(); - if (!$stream) { - return false; - } - if ($count <= 0) { return ''; } @@ -169,6 +209,13 @@ public function stream_read($count) { return ''; } + $stream = $this->getCurrent(); + if (!$stream) { + return false; + } + + // Bound reads by Content-Range; premature underlying EOF is not + // explicitly detected if fewer bytes than expected are returned. $ret = fread($stream, min($count, $remaining)); if ($ret === false) { return false; @@ -207,11 +254,9 @@ public function stream_seek($offset, $whence = SEEK_SET) { break; } - if ($this->hasOpenStream()) { - fclose($this->current); - } - $this->current = null; + $this->closeCurrent(); $this->needReconnect = true; + return true; } @@ -222,32 +267,35 @@ public function stream_tell() { #[\Override] public function stream_stat() { - if ($this->getCurrent()) { - $stat = fstat($this->getCurrent()); - if ($stat) { - $stat['size'] = $this->totalSize; - } - return $stat; - } else { + $stream = $this->getCurrent(); + if (!$stream) { return false; } + + $stat = fstat($stream); + if ($stat !== false) { + $stat['size'] = $this->totalSize; + } + + return $stat; } #[\Override] public function stream_eof() { + if ($this->offset >= $this->totalSize) { + return true; + } + if (!$this->getCurrent()) { return true; - } + } - return $this->offset >= $this->totalSize; + return false; } #[\Override] public function stream_close() { - if ($this->hasOpenStream()) { - fclose($this->current); - } - $this->current = null; + $this->closeCurrent(); } #[\Override] @@ -272,6 +320,6 @@ public function stream_lock($operation) { #[\Override] public function stream_flush() { - return; //noop because readonly stream + // No-op because this is a read-only stream. } } From da5e3d057b32c474857afe8ead8e2a820b30cd31 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 09:11:35 -0400 Subject: [PATCH 3/5] test(files): add coverage of SeekableHttpStream Assisted-by: Copilot:gpt-5.6-luna Signed-off-by: Josh --- .../Files/Stream/SeekableHttpStreamTest.php | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 tests/lib/Files/Stream/SeekableHttpStreamTest.php diff --git a/tests/lib/Files/Stream/SeekableHttpStreamTest.php b/tests/lib/Files/Stream/SeekableHttpStreamTest.php new file mode 100644 index 0000000000000..a9f9122288437 --- /dev/null +++ b/tests/lib/Files/Stream/SeekableHttpStreamTest.php @@ -0,0 +1,112 @@ +setPrivateProperty($stream, 'current', $current); + $this->setPrivateProperty($stream, 'offset', 8); + $this->setPrivateProperty($stream, 'totalSize', 10); + + $this->assertSame('89', $stream->stream_read(100)); + $this->assertSame(10, $stream->stream_tell()); + $this->assertTrue($stream->stream_eof()); + + fclose($current); + } + + public function testReadAtLogicalEndReturnsEmptyString(): void { + $stream = new SeekableHttpStream(); + + $this->setPrivateProperty($stream, 'offset', 10); + $this->setPrivateProperty($stream, 'totalSize', 10); + + $this->assertSame('', $stream->stream_read(100)); + $this->assertTrue($stream->stream_eof()); + } + + public function testEofAtLogicalEndDoesNotReconnect(): void { + $stream = new SeekableHttpStream(); + + $this->setPrivateProperty($stream, 'offset', 10); + $this->setPrivateProperty($stream, 'totalSize', 10); + $this->setPrivateProperty($stream, 'needReconnect', true); + + /* + * If stream_eof() attempted to reconnect before checking the logical + * end, this would try to invoke the unset callback. + */ + $this->assertTrue($stream->stream_eof()); + } + + public function testParsesContentRange(): void { + $stream = new SeekableHttpStream(); + + $result = $this->invokePrivate( + $stream, + 'parseContentRange', + [[ + 'HTTP/1.1 206 Partial Content', + 'Content-Range: bytes 10-19/100', + ]] + ); + + $this->assertSame([ + 'begin' => 10, + 'end' => 19, + 'totalSize' => 100, + ], $result); + } + + #[\PHPUnit\Framework\Attributes\DataProvider('invalidContentRangeProvider')] + public function testRejectsInvalidContentRange(string $contentRange): void { + $stream = new SeekableHttpStream(); + + $result = $this->invokePrivate( + $stream, + 'parseContentRange', + [[$contentRange]] + ); + + $this->assertNull($result); + } + + public static function invalidContentRangeProvider(): array { + return [ + 'missing header' => ['HTTP/1.1 200 OK'], + 'missing total size' => ['Content-Range: bytes 0-9/*'], + 'descending range' => ['Content-Range: bytes 9-0/10'], + 'total smaller than end' => ['Content-Range: bytes 0-9/9'], + 'not bytes' => ['Content-Range: items 0-9/10'], + 'malformed range' => ['Content-Range: bytes 0-9'], + ]; + } + + /** + * @param mixed $value + */ + private function setPrivateProperty( + SeekableHttpStream $stream, + string $property, + mixed $value, + ): void { + $reflection = new \ReflectionClass($stream); + $reflection->getProperty($property)->setValue($stream, $value); + } +} From 933d518a4f32d488609dae0f5e4e0d1d95aae418 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 10:13:11 -0400 Subject: [PATCH 4/5] chore: fix psalm annotation in SeekableHttpStream Signed-off-by: Josh --- lib/private/Files/Stream/SeekableHttpStream.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Stream/SeekableHttpStream.php b/lib/private/Files/Stream/SeekableHttpStream.php index 213502e115ed1..dcfeac7077b0b 100644 --- a/lib/private/Files/Stream/SeekableHttpStream.php +++ b/lib/private/Files/Stream/SeekableHttpStream.php @@ -35,7 +35,7 @@ private static function registerIfNeeded(): void { * The callback is called with a byte range and must return an HTTP stream * for that range. * - * @param callable(string): resource|false $callback + * @psalm-param impure-callable(string): resource|false $callback * * @return resource|false */ @@ -54,7 +54,7 @@ public static function open(callable $callback) { /** @var resource */ public $context; - /** @var callable(string): resource|false */ + /** @var impure-callable(string): resource|false */ private $openCallback; /** @var ?resource|closed-resource */ From 95741ae6418ac428763205a46629e41e8634368f Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 6 Sep 2026 12:08:45 -0400 Subject: [PATCH 5/5] chore(files): fix invalid callable-type syntax in the new docblocks Signed-off-by: Josh --- lib/private/Files/Stream/SeekableHttpStream.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/Stream/SeekableHttpStream.php b/lib/private/Files/Stream/SeekableHttpStream.php index dcfeac7077b0b..edd639fb2a9a1 100644 --- a/lib/private/Files/Stream/SeekableHttpStream.php +++ b/lib/private/Files/Stream/SeekableHttpStream.php @@ -35,7 +35,7 @@ private static function registerIfNeeded(): void { * The callback is called with a byte range and must return an HTTP stream * for that range. * - * @psalm-param impure-callable(string): resource|false $callback + * @psalm-param impure-callable(string): (resource|false) $callback * * @return resource|false */ @@ -54,7 +54,7 @@ public static function open(callable $callback) { /** @var resource */ public $context; - /** @var impure-callable(string): resource|false */ + /** @var impure-callable(string): (resource|false) */ private $openCallback; /** @var ?resource|closed-resource */