Fix: Sanitize cache keys to avoid reserved characters validation error (#39)

* Fix: Sanitize cache keys to avoid reserved characters validation error

---------

Co-authored-by: Xavier Lacot <[email protected]>

Author: camilleislasse

src/Storage/CacheKeySanitizer.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace JoliCode\MediaBundle\Storage;
+
+use Symfony\Contracts\Cache\ItemInterface;
+
+/**
+ * Sanitizes cache keys to ensure they don't contain PSR-6 reserved characters.
+ *
+ * @see ItemInterface::RESERVED_CHARACTERS
+ */
+final class CacheKeySanitizer
+{
+    /**
+     * Sanitizes a string by replacing PSR-6 reserved characters with underscores.
+     *
+     * PSR-6 reserved characters are: {}()/\@:
+     *
+     * @param string $value The value to sanitize
+     *
+     * @return string The sanitized value safe to use in cache keys
+     */
+    public static function sanitize(string $value): string
+    {
+        return str_replace(str_split(ItemInterface::RESERVED_CHARACTERS), '_', $value);
+    }
+}

src/Storage/MediaPropertyAccessor.php

@@ -85,22 +85,22 @@ function (ItemInterface $item) use ($path): int {
 
     private function getCacheKey(string $path, string $property): string
     {
-        return \sprintf(
+        return CacheKeySanitizer::sanitize(\sprintf(
             'joli_media_property_%s_%s_%s_%s',
             $this->libraryName,
             Resolver::normalizePath($path),
             $this->getLastModified($path),
             $property,
-        );
+        ));
     }
 
     private function getLastModifiedCacheKey(string $path): string
     {
-        return \sprintf(
+        return CacheKeySanitizer::sanitize(\sprintf(
             'joli_media_property_%s_%s_lastModified',
             $this->libraryName,
             Resolver::normalizePath($path),
-        );
+        ));
     }
 
     private function guessFilesize(string $path): int

src/Storage/MediaVariationPropertyAccessor.php

@@ -85,24 +85,24 @@ function (ItemInterface $item) use ($path, $variation): int {
 
     private function getCacheKey(string $path, Variation $variation, string $property): string
     {
-        return \sprintf(
+        return CacheKeySanitizer::sanitize(\sprintf(
             'joli_media_property_%s_%s_%s_%s_%s',
             $this->libraryName,
             $variation->getName(),
             Resolver::normalizePath($path),
             $this->getLastModified($path, $variation),
             $property,
-        );
+        ));
     }
 
     private function getLastModifiedCacheKey(string $path, Variation $variation): string
     {
-        return \sprintf(
+        return CacheKeySanitizer::sanitize(\sprintf(
             'joli_media_property_%s_%s_%s_lastModified',
             $this->libraryName,
             $variation->getName(),
             Resolver::normalizePath($path),
-        );
+        ));
     }
 
     private function guessFilesize(string $path, Variation $variation): int

tests/infrastructure/php-configuration/mods-available/app-default.ini

@@ -13,6 +13,8 @@ always_populate_raw_post_data = -1
 upload_max_filesize = 20M
 post_max_size = 20M
 zend.max_allowed_stack_size = -1
+[Assertion]
+zend.assertions = 1
 [Date]
 date.timezone = UTC
 [Phar]

tests/src/Storage/CacheKeySanitizerTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace JoliCode\MediaBundle\Tests\Storage;
+
+use JoliCode\MediaBundle\Storage\CacheKeySanitizer;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+class CacheKeySanitizerTest extends TestCase
+{
+    #[DataProvider('provideSanitizationCases')]
+    public function testSanitize(string $input, string $expected): void
+    {
+        $this->assertSame($expected, CacheKeySanitizer::sanitize($input));
+    }
+
+    /**
+     * @return array<string, array{0: string, 1: string}>
+     */
+    public static function provideSanitizationCases(): array
+    {
+        return [
+            'simple path with slash' => ['folder/file.jpg', 'folder_file.jpg'],
+            'path with all reserved characters' => ['path/with@special:chars{test}(1).jpg', 'path_with_special_chars_test__1_.jpg'],
+            'backslash' => ['path\file.jpg', 'path_file.jpg'],
+            'colon' => ['namespace:key', 'namespace_key'],
+            'at symbol' => ['user@host', 'user_host'],
+            'curly braces' => ['prefix{suffix}', 'prefix_suffix_'],
+            'parentheses' => ['name(variant)', 'name_variant_'],
+            'mixed slashes' => ['folder/subfolder\file.jpg', 'folder_subfolder_file.jpg'],
+            'no reserved characters' => ['simple_key', 'simple_key'],
+            'empty string' => ['', ''],
+        ];
+    }
+}

tests/src/Storage/CacheKeyWithSubfolderTest.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace JoliCode\MediaBundle\Tests\Storage;
+
+use JoliCode\MediaBundle\Binary\MimeTypeGuesser;
+use JoliCode\MediaBundle\Model\Format;
+use JoliCode\MediaBundle\Storage\MediaPropertyAccessor;
+use JoliCode\MediaBundle\Storage\MediaVariationPropertyAccessor;
+use JoliCode\MediaBundle\Storage\Strategy\FolderStorageStrategy;
+use JoliCode\MediaBundle\Transformer\TransformerChain;
+use JoliCode\MediaBundle\Variation\Variation;
+use League\Flysystem\Filesystem;
+use League\Flysystem\InMemory\InMemoryFilesystemAdapter;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\Mime\FileBinaryMimeTypeGuesser;
+use Symfony\Component\Mime\MimeTypes;
+
+/**
+ * Tests that cache keys are properly sanitized when using paths with reserved PSR-6 characters.
+ *
+ * This test ensures that files in subfolders (containing forward slashes) don't cause
+ * cache key validation errors.
+ */
+class CacheKeyWithSubfolderTest extends TestCase
+{
+    private ArrayAdapter $cache;
+
+    private Filesystem $filesystem;
+
+    private MimeTypeGuesser $mimeTypeGuesser;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->cache = new ArrayAdapter();
+        $this->filesystem = new Filesystem(new InMemoryFilesystemAdapter());
+        $this->mimeTypeGuesser = new MimeTypeGuesser(new MimeTypes(), new FileBinaryMimeTypeGuesser());
+    }
+
+    public function testSubfolderPath(): void
+    {
+        // Create a file in a subfolder (path contains '/' which is a reserved PSR-6 character)
+        $path = 'subfolder/test.jpg';
+        $this->filesystem->write($path, 'dummy image content');
+
+        $accessor = new MediaPropertyAccessor(
+            'default',
+            $this->filesystem,
+            $this->mimeTypeGuesser,
+            $this->cache,
+        );
+
+        $this->expectNotToPerformAssertions();
+
+        // This should NOT throw an exception about reserved characters
+        $accessor->getMimeType($path);
+        $accessor->getFormat($path);
+        $accessor->getFileSize($path);
+
+        // Test clearCache doesn't throw either
+        $accessor->clearCache($path);
+    }
+
+    public function testVariationSubfolderPath(): void
+    {
+        $strategy = new FolderStorageStrategy();
+
+        // Create a file in a nested subfolder
+        $path = 'folder/subfolder/image.png';
+        $this->filesystem->write($path, 'dummy image content');
+
+        $variation = new Variation('thumbnail', Format::PNG, new TransformerChain([]));
+
+        $accessor = new MediaVariationPropertyAccessor(
+            'my-library',
+            $strategy,
+            $this->filesystem,
+            $this->mimeTypeGuesser,
+            $this->cache,
+        );
+
+        $this->expectNotToPerformAssertions();
+
+        // This should NOT throw an exception about reserved characters
+        $accessor->getMimeType($path, $variation);
+        $accessor->getFormat($path, $variation);
+        $accessor->getFileSize($path, $variation);
+
+        // Test clearCache doesn't throw either
+        $accessor->clearCache($path, $variation);
+    }
+
+    public function testNestedSubfolderPath(): void
+    {
+        // Path with forward slash (subfolder)
+        $path = 'user-uploads/2024/document.pdf';
+        $this->filesystem->write($path, 'dummy pdf content');
+
+        $accessor = new MediaPropertyAccessor(
+            'default',
+            $this->filesystem,
+            $this->mimeTypeGuesser,
+            $this->cache,
+        );
+
+        // Should work without throwing PSR-6 cache key validation errors
+        $mimeType = $accessor->getMimeType($path);
+
+        // Verify the cache was actually used by calling again
+        $mimeType2 = $accessor->getMimeType($path);
+        $this->assertSame($mimeType, $mimeType2);
+    }
+}