diff --git a/lib/Service/ZipEntryService.php b/lib/Service/ZipEntryService.php
index db55898..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.
@@ -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();
}
diff --git a/tests/Unit/Service/ZipEntryServiceTest.php b/tests/Unit/Service/ZipEntryServiceTest.php
index e0f792d..6522014 100644
--- a/tests/Unit/Service/ZipEntryServiceTest.php
+++ b/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', '
Playground
');
+ $file = $this->createFakeFile($archive, '/virtual/php-wasm/package.elpx');
+
+ self::assertSame('Playground
', $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', '');
+ $file = $this->createFakeFile($archive, '');
+
+ self::assertSame('', $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', 'Playground
');
+ $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', 'Playground
');
+ $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', 'Playground
');
+ $zip->addFromString('content.xml', '');
+ $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;
+ }
}
diff --git a/tests/bootstrap-standalone.php b/tests/bootstrap-standalone.php
index ae11387..0188c19 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($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 {} }');
}