From eac9f8084831f7382a876ef25c81d2ab1b1f41ac Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Sun, 12 Jul 2026 11:39:21 +0100
Subject: [PATCH 1/5] fix(viewer): stream ZIP fallback in playground

---
 lib/Service/ZipEntryService.php            |  8 +++-
 tests/Unit/Service/ZipEntryServiceTest.php | 44 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/lib/Service/ZipEntryService.php b/lib/Service/ZipEntryService.php
index db55898..7700988 100644
--- a/lib/Service/ZipEntryService.php
+++ b/lib/Service/ZipEntryService.php
@@ -32,13 +32,17 @@ 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) {
diff --git a/tests/Unit/Service/ZipEntryServiceTest.php b/tests/Unit/Service/ZipEntryServiceTest.php
index e0f792d..2bc43c7 100644
--- a/tests/Unit/Service/ZipEntryServiceTest.php
+++ b/tests/Unit/Service/ZipEntryServiceTest.php
@@ -44,4 +44,48 @@ public function testRejectsEmptyAndNulTaintedPaths(): void {
 		self::assertNull($this->service->normalizeEntry(''));
 		self::assertNull($this->service->normalizeEntry("a\0b"));
 	}
+
+	public function testReadEntryFallsBackToStreamWhenLocalPathCannotBeOpened(): void {
+		if (!class_exists('OCP\\Files\\File')) {
+			eval('namespace OCP\\Files; class File {}');
+		}
+
+		$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->close();
+		$archive = file_get_contents($archivePath);
+		@unlink($archivePath);
+		self::assertIsString($archive);
+
+		$file = new class($archive) extends \OCP\Files\File {
+			public function __construct(
+				private readonly string $archive,
+			) {
+			}
+
+			public function getStorage(): object {
+				return new class {
+					public function getLocalFile(string $path): string {
+						return '/virtual/php-wasm/package.elpx';
+					}
+				};
+			}
+
+			public function getInternalPath(): string {
+				return 'files/package.elpx';
+			}
+
+			public function fopen(string $mode) {
+				$stream = fopen('php://temp', 'w+b');
+				fwrite($stream, $this->archive);
+				rewind($stream);
+				return $stream;
+			}
+		};
+
+		self::assertSame('<h1>Playground</h1>', $this->service->readEntry($file, 'index.html'));
+	}
 }

From 2df839f5f8b44e8c7945dd39445bacb07f14747f Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Tue, 28 Jul 2026 15:50:33 +0100
Subject: [PATCH 2/5] test(viewer): cover the empty-string local-path fallback
 branch

Add a test for the S3/object-storage case that motivated widening the
readEntry() guard: getLocalFile() returning an empty string rather than
false/null. Extract the archive/fake-File setup shared with the existing
playground test into two small helpers, and drop the class doc comment's
now-stale claim that the reading methods are too heavy to fake here.
---
 tests/Unit/Service/ZipEntryServiceTest.php | 63 +++++++++++++++++-----
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/tests/Unit/Service/ZipEntryServiceTest.php b/tests/Unit/Service/ZipEntryServiceTest.php
index 2bc43c7..d861daf 100644
--- a/tests/Unit/Service/ZipEntryServiceTest.php
+++ b/tests/Unit/Service/ZipEntryServiceTest.php
@@ -8,9 +8,10 @@
 use PHPUnit\Framework\TestCase;
 
 /**
- * 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 faked here via a minimal anonymous subclass 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;
@@ -46,30 +47,70 @@ public function testRejectsEmptyAndNulTaintedPaths(): void {
 	}
 
 	public function testReadEntryFallsBackToStreamWhenLocalPathCannotBeOpened(): void {
-		if (!class_exists('OCP\\Files\\File')) {
-			eval('namespace OCP\\Files; class File {}');
-		}
+		// 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'));
+	}
 
+	/**
+	 * 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('index.html', '<h1>Playground</h1>');
+		$zip->addFromString($entryName, $entryContents);
 		$zip->close();
 		$archive = file_get_contents($archivePath);
 		@unlink($archivePath);
 		self::assertIsString($archive);
+		return $archive;
+	}
+
+	/**
+	 * Fakes 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 {
+		if (!class_exists('OCP\\Files\\File')) {
+			eval('namespace OCP\\Files; class File {}');
+		}
 
-		$file = new class($archive) extends \OCP\Files\File {
+		return new class($archive, $localPath) extends \OCP\Files\File {
 			public function __construct(
 				private readonly string $archive,
+				private readonly string $localPath,
 			) {
 			}
 
 			public function getStorage(): object {
-				return new class {
+				return new class($this->localPath) {
+					public function __construct(
+						private readonly string $localPath,
+					) {
+					}
+
 					public function getLocalFile(string $path): string {
-						return '/virtual/php-wasm/package.elpx';
+						return $this->localPath;
 					}
 				};
 			}
@@ -85,7 +126,5 @@ public function fopen(string $mode) {
 				return $stream;
 			}
 		};
-
-		self::assertSame('<h1>Playground</h1>', $this->service->readEntry($file, 'index.html'));
 	}
 }

From 6dfce09097b02da9c3675f937e663a073e465254 Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Tue, 28 Jul 2026 19:54:51 +0100
Subject: [PATCH 3/5] test(viewer): mock Nextcloud file interface correctly

OCP\Files\File is an interface in all supported Nextcloud versions, so
the previous class_exists() guard never detected it and the eval()'d
class would collide with the real API outside the standalone bootstrap.
Stub the interface in bootstrap-standalone.php (guarded by
interface_exists) and build the fake file with createMock() instead.
---
 tests/Unit/Service/ZipEntryServiceTest.php | 47 +++++++++-------------
 tests/bootstrap-standalone.php             | 13 ++++++
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/tests/Unit/Service/ZipEntryServiceTest.php b/tests/Unit/Service/ZipEntryServiceTest.php
index d861daf..134725c 100644
--- a/tests/Unit/Service/ZipEntryServiceTest.php
+++ b/tests/Unit/Service/ZipEntryServiceTest.php
@@ -9,9 +9,9 @@
 
 /**
  * Tests for {@see ZipEntryService}. `readEntry()` takes a Nextcloud `File`,
- * which is faked here via a minimal anonymous subclass so the local-path and
- * stream-fallback branches can be exercised without a real Nextcloud server
- * or storage backend.
+ * 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;
@@ -85,46 +85,35 @@ private function createTestArchive(string $entryName, string $entryContents): st
 	}
 
 	/**
-	 * Fakes a Nextcloud `File` whose storage reports `$localPath` as the
+	 * 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 {
-		if (!class_exists('OCP\\Files\\File')) {
-			eval('namespace OCP\\Files; class File {}');
-		}
-
-		return new class($archive, $localPath) extends \OCP\Files\File {
+		$storage = new class($localPath) {
 			public function __construct(
-				private readonly string $archive,
 				private readonly string $localPath,
 			) {
 			}
 
-			public function getStorage(): object {
-				return new class($this->localPath) {
-					public function __construct(
-						private readonly string $localPath,
-					) {
-					}
-
-					public function getLocalFile(string $path): string {
-						return $this->localPath;
-					}
-				};
-			}
-
-			public function getInternalPath(): string {
-				return 'files/package.elpx';
+			public function getLocalFile(string $path): string {
+				return $this->localPath;
 			}
+		};
 
-			public function fopen(string $mode) {
+		$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, $this->archive);
+				fwrite($stream, $archive);
 				rewind($stream);
 				return $stream;
-			}
-		};
+			},
+		);
+
+		return $file;
 	}
 }
diff --git a/tests/bootstrap-standalone.php b/tests/bootstrap-standalone.php
index ae11387..43a0418 100644
--- a/tests/bootstrap-standalone.php
+++ b/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(string $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 {} }');
 }

From 2c73fd7c8ac6b7aef43307d924fcadbaee4be131 Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Tue, 28 Jul 2026 19:55:04 +0100
Subject: [PATCH 4/5] fix(viewer): enforce ZIP limits across storage backends

The File::fopen() fallback returned entries without applying
MAX_UNCOMPRESSED_SIZE_BYTES, and widening the fallback to every
ZipArchive::open() failure widened that gap too. Extract the entry read
into a shared extractEntry() so the local-path and stream branches
enforce identical limits and agree on missing entries.

The read is bounded by the declared entry size (validated against the
limit first) rather than the limit itself: getFromName() preallocates
its whole $len buffer, so passing the 500MB limit would allocate 500MB
per request. The extra byte past the declared size exposes archives
whose central directory understates the real entry size.

The limits are constructor-injectable (defaulting to the class
constants) so tests can exercise them with small archives.
---
 lib/Service/ZipEntryService.php            | 61 ++++++++++++++++------
 tests/Unit/Service/ZipEntryServiceTest.php | 55 +++++++++++++++++++
 2 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/lib/Service/ZipEntryService.php b/lib/Service/ZipEntryService.php
index 7700988..93ceec8 100644
--- a/lib/Service/ZipEntryService.php
+++ b/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.
@@ -45,22 +55,43 @@ public function readEntry(File $file, string $entry): ?string {
 			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.
@@ -78,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)) {
@@ -136,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();
 			}
diff --git a/tests/Unit/Service/ZipEntryServiceTest.php b/tests/Unit/Service/ZipEntryServiceTest.php
index 134725c..6522014 100644
--- a/tests/Unit/Service/ZipEntryServiceTest.php
+++ b/tests/Unit/Service/ZipEntryServiceTest.php
@@ -6,6 +6,7 @@
 
 use OCA\ExeLearning\Service\ZipEntryService;
 use PHPUnit\Framework\TestCase;
+use RuntimeException;
 
 /**
  * Tests for {@see ZipEntryService}. `readEntry()` takes a Nextcloud `File`,
@@ -68,6 +69,60 @@ public function testReadEntryFallsBackToStreamWhenLocalFileIsEmptyString(): void
 		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.
 	 */

From dd49c2b31864886f3574baec2e5530ac7d000ba1 Mon Sep 17 00:00:00 2001
From: erseco <info@ernesto.es>
Date: Tue, 28 Jul 2026 20:05:43 +0100
Subject: [PATCH 5/5] test(viewer): match the real untyped fopen() signature in
 the File stub

The real OCP\Files\File declares fopen($mode) with the type only in
the docblock, and the stub's own comment already promised untyped
methods.
---
 tests/bootstrap-standalone.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/bootstrap-standalone.php b/tests/bootstrap-standalone.php
index 43a0418..0188c19 100644
--- a/tests/bootstrap-standalone.php
+++ b/tests/bootstrap-standalone.php
@@ -53,7 +53,7 @@ public function boot(IBootContext $context): void;
 		interface File {
 			public function getStorage();
 			public function getInternalPath();
-			public function fopen(string $mode);
+			public function fopen($mode);
 		}
 	');
 }