fix(viewer): stream ZIP fallback in playground

lib/Service/ZipEntryService.php

@@ -19,6 +19,16 @@ class ZipEntryService {
 	public const MAX_ENTRIES = 5000;
 	public const MAX_UNCOMPRESSED_SIZE_BYTES = 500 * 1024 * 1024;
 
+	/**
+	 * The limits are injectable so tests can exercise them with small
+	 * archives; production callers rely on the defaults.
+	 */
+	public function __construct(
+		private readonly int $maxEntries = self::MAX_ENTRIES,
+		private readonly int $maxUncompressedSizeBytes = self::MAX_UNCOMPRESSED_SIZE_BYTES,
+	) {
+	}
+
 	/**
 	 * Reads a single entry from the package. Returns the raw bytes or null if
 	 * the entry is not present.
@@ -32,31 +42,56 @@ public function readEntry(File $file, string $entry): ?string {
 		}
 
 		$localPath = $file->getStorage()->getLocalFile($file->getInternalPath());
-		if ($localPath === false || $localPath === null) {
+		if (!is_string($localPath) || $localPath === '') {
 			return $this->readEntryFromStream($file, $normalized);
 		}
 
 		$zip = new ZipArchive();
 		if ($zip->open($localPath) !== true) {
-			return null;
+			// Some virtual storage implementations (notably the php-wasm
+			// Playground filesystem) expose a nominal local path that native
+			// ZipArchive still cannot open. Treat it like any other non-local
+			// storage and retry through the portable File::fopen() path.
+			return $this->readEntryFromStream($file, $normalized);
 		}
 		try {
-			if ($zip->numFiles > self::MAX_ENTRIES) {
-				return null;
-			}
-			$contents = $zip->getFromName($normalized);
-			if ($contents === false) {
-				return null;
-			}
-			if (strlen($contents) > self::MAX_UNCOMPRESSED_SIZE_BYTES) {
-				throw new RuntimeException('Uncompressed entry exceeds limit');
-			}
-			return $contents;
+			return $this->extractEntry($zip, $normalized);
 		} finally {
 			$zip->close();
 		}
 	}
 
+	/**
+	 * Reads one entry from an opened archive. Both the local-path branch and
+	 * the stream fallback go through here so they enforce identical limits
+	 * and agree on returning null for missing entries.
+	 */
+	private function extractEntry(ZipArchive $zip, string $entry): ?string {
+		if ($zip->numFiles > $this->maxEntries) {
+			return null;
+		}
+		$stat = $zip->statName($entry);
+		if ($stat === false) {
+			return null;
+		}
+		$declaredSize = $stat['size'];
+		if ($declaredSize > $this->maxUncompressedSizeBytes) {
+			throw new RuntimeException('Uncompressed entry exceeds limit');
+		}
+		// getFromName() allocates its whole $len buffer up front, so the read
+		// must be bounded by the (already limit-checked) declared size rather
+		// than the limit itself. The extra byte exposes archives whose
+		// central directory understates the real entry size.
+		$contents = $zip->getFromName($entry, $declaredSize + 1);
+		if ($contents === false) {
+			return null;
+		}
+		if (strlen($contents) > $declaredSize) {
+			throw new RuntimeException('Entry is larger than its declared size');
+		}
+		return $contents;
+	}
+
 	/**
 	 * Lists entry names (no contents). Useful for validating that a package
 	 * is shaped like an eXeLearning project.
@@ -74,7 +109,7 @@ public function listEntries(File $file): array {
 		}
 		$entries = [];
 		try {
-			$count = min($zip->numFiles, self::MAX_ENTRIES);
+			$count = min($zip->numFiles, $this->maxEntries);
 			for ($i = 0; $i < $count; $i++) {
 				$name = $zip->getNameIndex($i);
 				if (is_string($name)) {
@@ -132,11 +167,7 @@ private function readEntryFromStream(File $file, string $entry): ?string {
 				return null;
 			}
 			try {
-				if ($zip->numFiles > self::MAX_ENTRIES) {
-					return null;
-				}
-				$contents = $zip->getFromName($entry);
-				return $contents === false ? null : $contents;
+				return $this->extractEntry($zip, $entry);
 			} finally {
 				$zip->close();
 			}

tests/Unit/Service/ZipEntryServiceTest.php

@@ -6,11 +6,13 @@
 
 use OCA\ExeLearning\Service\ZipEntryService;
 use PHPUnit\Framework\TestCase;
+use RuntimeException;
 
 /**
- * Tests for {@see ZipEntryService::normalizeEntry}. The reading methods need a
- * Nextcloud File handle which is too heavy to fake here — they are exercised
- * by integration tests that run against a real server.
+ * Tests for {@see ZipEntryService}. `readEntry()` takes a Nextcloud `File`,
+ * which is mocked here (against the interface stub from
+ * `bootstrap-standalone.php`) so the local-path and stream-fallback branches
+ * can be exercised without a real Nextcloud server or storage backend.
  */
 final class ZipEntryServiceTest extends TestCase {
 	private ZipEntryService $service;
@@ -44,4 +46,129 @@ public function testRejectsEmptyAndNulTaintedPaths(): void {
 		self::assertNull($this->service->normalizeEntry(''));
 		self::assertNull($this->service->normalizeEntry("a\0b"));
 	}
+
+	public function testReadEntryFallsBackToStreamWhenLocalPathCannotBeOpened(): void {
+		// Some virtual storage implementations (notably the php-wasm
+		// Playground filesystem) expose a nominal local path that native
+		// ZipArchive still cannot open.
+		$archive = $this->createTestArchive('index.html', '<h1>Playground</h1>');
+		$file = $this->createFakeFile($archive, '/virtual/php-wasm/package.elpx');
+
+		self::assertSame('<h1>Playground</h1>', $this->service->readEntry($file, 'index.html'));
+	}
+
+	public function testReadEntryFallsBackToStreamWhenLocalFileIsEmptyString(): void {
+		// S3/object storage and other non-local primary storages commonly
+		// return an empty string — not false/null — from getLocalFile() when
+		// there is no local path at all. That empty string must not be
+		// handed to ZipArchive::open(), which would emit a PHP warning and
+		// leave the entry unreadable.
+		$archive = $this->createTestArchive('content.xml', '<content/>');
+		$file = $this->createFakeFile($archive, '');
+
+		self::assertSame('<content/>', $this->service->readEntry($file, 'content.xml'));
+	}
+
+	public function testReadsZeroByteEntriesAsEmptyString(): void {
+		// A declared size of 0 means the bounded read asks for a single byte;
+		// getFromName() must still report the empty entry as '' — not false.
+		$archive = $this->createTestArchive('empty.txt', '');
+		$file = $this->createFakeFile($archive, '');
+
+		self::assertSame('', $this->service->readEntry($file, 'empty.txt'));
+	}
+
+	public function testStreamFallbackRejectsEntriesOverTheUncompressedSizeLimit(): void {
+		// Both branches must enforce the same limits: an oversized entry has
+		// to be rejected whether the package is opened from a local path or
+		// through the File::fopen() fallback.
+		$service = new ZipEntryService(maxUncompressedSizeBytes: 8);
+		$archive = $this->createTestArchive('index.html', '<h1>Playground</h1>');
+		$file = $this->createFakeFile($archive, '');
+
+		$this->expectException(RuntimeException::class);
+		$service->readEntry($file, 'index.html');
+	}
+
+	public function testLocalPathRejectsEntriesOverTheUncompressedSizeLimit(): void {
+		$service = new ZipEntryService(maxUncompressedSizeBytes: 8);
+		$archive = $this->createTestArchive('index.html', '<h1>Playground</h1>');
+		$archivePath = tempnam(sys_get_temp_dir(), 'elpx_test_');
+		self::assertNotFalse($archivePath);
+		file_put_contents($archivePath, $archive);
+		$file = $this->createFakeFile($archive, $archivePath);
+
+		try {
+			$this->expectException(RuntimeException::class);
+			$service->readEntry($file, 'index.html');
+		} finally {
+			@unlink($archivePath);
+		}
+	}
+
+	public function testStreamFallbackReturnsNullWhenArchiveHasTooManyEntries(): void {
+		$service = new ZipEntryService(maxEntries: 1);
+		$archivePath = tempnam(sys_get_temp_dir(), 'elpx_test_');
+		self::assertNotFalse($archivePath);
+		$zip = new \ZipArchive();
+		self::assertTrue($zip->open($archivePath, \ZipArchive::OVERWRITE) === true);
+		$zip->addFromString('index.html', '<h1>Playground</h1>');
+		$zip->addFromString('content.xml', '<content/>');
+		$zip->close();
+		$archive = file_get_contents($archivePath);
+		@unlink($archivePath);
+		self::assertIsString($archive);
+		$file = $this->createFakeFile($archive, '');
+
+		self::assertNull($service->readEntry($file, 'index.html'));
+	}
+
+	/**
+	 * Builds a minimal in-memory ZIP archive and returns its raw bytes.
+	 */
+	private function createTestArchive(string $entryName, string $entryContents): string {
+		$archivePath = tempnam(sys_get_temp_dir(), 'elpx_test_');
+		self::assertNotFalse($archivePath);
+		$zip = new \ZipArchive();
+		self::assertTrue($zip->open($archivePath, \ZipArchive::OVERWRITE) === true);
+		$zip->addFromString($entryName, $entryContents);
+		$zip->close();
+		$archive = file_get_contents($archivePath);
+		@unlink($archivePath);
+		self::assertIsString($archive);
+		return $archive;
+	}
+
+	/**
+	 * Mocks a Nextcloud `File` whose storage reports `$localPath` as the
+	 * local path (any value `ZipEntryService` should NOT be able to open
+	 * directly — e.g. a virtual path or an empty string) and whose
+	 * `fopen()` streams the given archive bytes.
+	 */
+	private function createFakeFile(string $archive, string $localPath): \OCP\Files\File {
+		$storage = new class($localPath) {
+			public function __construct(
+				private readonly string $localPath,
+			) {
+			}
+
+			public function getLocalFile(string $path): string {
+				return $this->localPath;
+			}
+		};
+
+		$file = $this->createMock(\OCP\Files\File::class);
+		$file->method('getStorage')->willReturn($storage);
+		$file->method('getInternalPath')->willReturn('files/package.elpx');
+		$file->method('fopen')->with('rb')->willReturnCallback(
+			static function () use ($archive) {
+				$stream = fopen('php://temp', 'w+b');
+				fwrite($stream, $archive);
+				rewind($stream);
+				return $stream;
+			},
+		);
+
+		return $file;
+	}
 }

tests/bootstrap-standalone.php

@@ -44,6 +44,19 @@ public function boot(IBootContext $context): void;
 if (!interface_exists('OCP\\IPreview', false)) {
 	eval('namespace OCP; interface IPreview {}');
 }
+if (!interface_exists('OCP\\Files\\File', false)) {
+	// Like the real OCP\Files\File (an interface extending Node), but reduced
+	// to the members our code touches. Methods stay untyped as in the real
+	// API so PHPUnit mocks can implement them freely.
+	eval('
+		namespace OCP\\Files;
+		interface File {
+			public function getStorage();
+			public function getInternalPath();
+			public function fopen($mode);
+		}
+	');
+}
 if (!class_exists('OCP\\Util', false)) {
 	eval('namespace OCP; class Util { public static function addInitScript(string $app, string $script): void {} public static function addScript(string $app, string $script): void {} }');
 }