[FIX] 48047: Änderungen im Content Style wirken sich nicht aus

components/ILIAS/Filesystem/src/Stream/Streams.php

@@ -83,16 +83,24 @@ public static function ofReattachableResource($resource): ReattachableStream
 
     public static function ofFileInsideZIP(string $path_to_zip, string $path_inside_zip): ZIPStream
     {
-        // we try to open the zip file with the path inside the zip file, once with a leading slash and once without
-        try {
-            $resource = @fopen('zip://' . $path_to_zip . '#/' . $path_inside_zip, 'rb');
-        } catch (\Throwable) {
-            $resource = null;
-        }
-        try {
-            $resource = $resource ?: @fopen('zip://' . $path_to_zip . '#' . $path_inside_zip, 'rb');
-        } catch (\Throwable) {
-            $resource = null;
+        // Entries are stored relative since Mantis 45580 / 47237, therefore that variant
+        // is tried first. Containers written before still hold entries with a leading
+        // slash, they are covered by the second candidate. If a container holds both
+        // variants of a file, the relative one is the up to date one (Mantis 48047):
+        // the ZIP stream wrapper matches names literally, so the entry we do not ask for
+        // is never returned by accident.
+        $relative_path = ltrim($path_inside_zip, '/');
+        $resource = null;
+
+        foreach ([$relative_path, '/' . $relative_path] as $candidate) {
+            try {
+                $resource = @fopen('zip://' . $path_to_zip . '#' . $candidate, 'rb') ?: null;
+            } catch (\Throwable) {
+                $resource = null;
+            }
+            if ($resource !== null) {
+                break;
+            }
         }
 
         if (!is_resource($resource)) {

components/ILIAS/Filesystem/tests/Stream/FileInsideZIPTest.php

@@ -0,0 +1,105 @@
+<?php
+
+/**
+ * This file is part of ILIAS, a powerful learning management system
+ * published by ILIAS open source e-Learning e.V.
+ *
+ * ILIAS is licensed with the GPL-3.0,
+ * see https://www.gnu.org/licenses/gpl-3.0.en.html
+ * You should have received a copy of said license along with the
+ * source code, too.
+ *
+ * If this is not the case or you just want to try ILIAS, you'll find
+ * us at:
+ * https://www.ilias.de
+ * https://github.com/ILIAS-eLearning
+ *
+ *********************************************************************/
+
+declare(strict_types=1);
+
+namespace ILIAS\Filesystem\Stream;
+
+use PHPUnit\Framework\TestCase;
+use ZipArchive;
+
+/**
+ * Regression tests for Mantis 48047: entries inside a container are stored relative
+ * since Mantis 45580 / 47237. Containers written before that may hold both variants
+ * of the same file, and in that case the relative entry is the up to date one.
+ *
+ * @author Fabian Schmid <fabian@sr.solutions>
+ */
+final class FileInsideZIPTest extends TestCase
+{
+    private string $zip_file;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->zip_file = tempnam(sys_get_temp_dir(), 'irss_zip_read_test_');
+    }
+
+    protected function tearDown(): void
+    {
+        parent::tearDown();
+        @unlink($this->zip_file);
+    }
+
+    /**
+     * @param array<string, string> $entries
+     */
+    private function buildZip(array $entries): void
+    {
+        $zip = new ZipArchive();
+        $zip->open($this->zip_file, ZipArchive::OVERWRITE);
+        foreach ($entries as $name => $content) {
+            $zip->addFromString($name, $content);
+        }
+        $zip->close();
+    }
+
+    public function testRelativeEntryIsRead(): void
+    {
+        $this->buildZip(['style.css' => 'relative']);
+
+        $stream = Streams::ofFileInsideZIP($this->zip_file, 'style.css');
+
+        $this->assertSame('relative', (string) $stream);
+    }
+
+    public function testLegacyEntryIsStillRead(): void
+    {
+        $this->buildZip(['/style.css' => 'legacy']);
+
+        $stream = Streams::ofFileInsideZIP($this->zip_file, 'style.css');
+
+        $this->assertSame('legacy', (string) $stream);
+    }
+
+    public function testRelativeEntryWinsOverLegacyEntry(): void
+    {
+        $this->buildZip(['/style.css' => 'legacy', 'style.css' => 'relative']);
+
+        $stream = Streams::ofFileInsideZIP($this->zip_file, 'style.css');
+
+        $this->assertSame('relative', (string) $stream);
+    }
+
+    public function testRelativeEntryWinsAlsoIfTheRequestedPathHasALeadingSlash(): void
+    {
+        $this->buildZip(['/style.css' => 'legacy', 'style.css' => 'relative']);
+
+        $stream = Streams::ofFileInsideZIP($this->zip_file, '/style.css');
+
+        $this->assertSame('relative', (string) $stream);
+    }
+
+    public function testUnknownEntryThrows(): void
+    {
+        $this->buildZip(['style.css' => 'relative']);
+
+        $this->expectException(\InvalidArgumentException::class);
+        Streams::ofFileInsideZIP($this->zip_file, 'does_not_exist.css');
+    }
+}

components/ILIAS/ResourceStorage/src/Resource/ResourceBuilder.php

@@ -633,10 +633,33 @@ private function ensurePathInZIP(\ZipArchive $zip, string $path, bool $is_file):
         return $path_inside_container . '/' . $filename;
     }
 
+    /**
+     * Entries have been stored with a leading slash before Mantis 45580 / 47237,
+     * e.g. "/style.css" instead of "style.css". Both variants of an entry can exist
+     * side by side in containers of that age, so removing an entry has to cover both -
+     * otherwise the legacy entry stays in the container forever (Mantis 48047).
+     *
+     * @return string[] empty if the given path does not address an entry at all
+     */
+    private function pathVariantsInZIP(string $path): array
+    {
+        $path = ltrim($path, '/');
+        if ($path === '') {
+            return [];
+        }
+
+        return [$path, '/' . $path];
+    }
+
     public function removePathInsideContainer(
         StorableContainerResource $container,
         string $path_inside_container,
     ): bool {
+        $paths_to_remove = $this->pathVariantsInZIP($path_inside_container);
+        if ($paths_to_remove === []) {
+            return false;
+        }
+
         $revision = $container->getCurrentRevisionIncludingDraft();
         $stream = $this->extractStream($revision);
 
@@ -645,15 +668,21 @@ public function removePathInsideContainer(
             $zip = new \ZipArchive();
             $zip->open($stream->getMetadata()['uri']);
 
-            $return = $zip->deleteName($path_inside_container);
+            $return = false;
+            foreach ($paths_to_remove as $path_to_remove) {
+                $return = $zip->deleteName($path_to_remove) || $return;
+            }
             // remove all files inside the directory
             for ($i = 0; $i < $zip->numFiles; $i++) {
                 $path = $zip->getNameIndex($i);
                 if ($path === false) {
                     continue;
                 }
-                if (str_starts_with($path, $path_inside_container)) {
-                    $zip->deleteIndex($i);
+                foreach ($paths_to_remove as $path_to_remove) {
+                    if (str_starts_with($path, $path_to_remove)) {
+                        $zip->deleteIndex($i);
+                        break;
+                    }
                 }
             }
 
@@ -686,7 +715,9 @@ public function addUploadToContainer(
 
             $parent_path_inside_container = $this->ensurePathInZIP($zip, $parent_path_inside_container, false);
 
-            $path_inside_zip = rtrim($parent_path_inside_container, '/') . '/' . $result->getName();
+            // an empty parent path must not result in an entry like "/file.txt",
+            // entries are stored relative since Mantis 45580 / 47237
+            $path_inside_zip = ltrim(rtrim($parent_path_inside_container, '/') . '/' . $result->getName(), '/');
 
             $return = $zip->addFile(
                 $result->getPath(),

components/ILIAS/ResourceStorage/tests/Resource/EnsurePathInZipTest.php

@@ -108,4 +108,28 @@ public function testWritingRootFileProducesRelativeEntry(): void
         }
         $this->assertContains('index.html', $names);
     }
+
+    /**
+     * Mantis 48047: containers written before the fix above still contain entries
+     * like "/style.css" next to the relative entry of the same file. Removing an
+     * entry has to cover both variants, otherwise the legacy one stays forever.
+     */
+    #[DataProvider('pathVariantProvider')]
+    public function testPathVariantsCoverLegacyEntries(string $input, array $expected): void
+    {
+        $method = new \ReflectionMethod(ResourceBuilder::class, 'pathVariantsInZIP');
+        $builder = (new \ReflectionClass(ResourceBuilder::class))->newInstanceWithoutConstructor();
+
+        $this->assertSame($expected, $method->invoke($builder, $input));
+    }
+
+    public static function pathVariantProvider(): \Iterator
+    {
+        yield 'root file' => ['style.css', ['style.css', '/style.css']];
+        yield 'root file, leading slash' => ['/style.css', ['style.css', '/style.css']];
+        yield 'nested file' => ['images/header.png', ['images/header.png', '/images/header.png']];
+        yield 'directory' => ['images/', ['images/', '/images/']];
+        yield 'empty path' => ['', []];
+        yield 'slash only' => ['/', []];
+    }
 }