[TASK] Ensure valid cache ids

Solr extensions are hooking into the DataHandler observing record
changes and partly these hooks are using the TwoLevelCache and the
DataHandler record ids to cache already processed elements.

As the NEW ids processed by the DataHandler can be set by extensions
and may be invalid for the usage with the cache frontend, we should
sanitize the ids to avoid exceptions. This is now ensured by adding
a sanitize method to the TwoLevelCache.

Resolves: #3031

Classes/System/Cache/TwoLevelCache.php

@@ -29,6 +29,7 @@
 
 use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
 
 /**
  * Provides a two level cache that uses an in memory cache as the first level cache and
@@ -38,7 +39,6 @@
  */
 class TwoLevelCache
 {
-
     /**
      * @var string
      */
@@ -50,15 +50,15 @@ class TwoLevelCache
     protected static $firstLevelCache = [];
 
     /**
-     * @var \TYPO3\CMS\Core\Cache\Frontend\FrontendInterface
+     * @var FrontendInterface
      */
     protected $secondLevelCache = null;
 
     /**
      * @param string $cacheName
-     * @param \TYPO3\CMS\Core\Cache\Frontend\FrontendInterface $secondaryCacheFrontend
+     * @param FrontendInterface $secondaryCacheFrontend
      */
-    public function __construct($cacheName, $secondaryCacheFrontend = null)
+    public function __construct(string $cacheName, FrontendInterface $secondaryCacheFrontend = null)
     {
         $this->cacheName = $cacheName;
         if ($secondaryCacheFrontend == null) {
@@ -75,7 +75,7 @@ public function __construct($cacheName, $secondaryCacheFrontend = null)
      * @param string $cacheId
      * @return mixed|null
      */
-    protected function getFromFirstLevelCache($cacheId)
+    protected function getFromFirstLevelCache(string $cacheId)
     {
         if (!empty(self::$firstLevelCache[$this->cacheName][$cacheId])) {
             return self::$firstLevelCache[$this->cacheName][$cacheId];
@@ -90,7 +90,7 @@ protected function getFromFirstLevelCache($cacheId)
      * @param string $cacheId
      * @param mixed $value
      */
-    protected function setToFirstLevelCache($cacheId, $value)
+    protected function setToFirstLevelCache(string $cacheId, $value): void
     {
         self::$firstLevelCache[$this->cacheName][$cacheId] = $value;
     }
@@ -102,8 +102,10 @@ protected function setToFirstLevelCache($cacheId, $value)
      * @param string $cacheId
      * @return mixed
      */
-    public function get($cacheId)
+    public function get(string $cacheId)
     {
+        $cacheId = $this->sanitizeCacheId($cacheId);
+
         $firstLevelResult = $this->getFromFirstLevelCache($cacheId);
         if ($firstLevelResult !== null) {
             return $firstLevelResult;
@@ -121,18 +123,33 @@ public function get($cacheId)
      * @param string $cacheId
      * @param mixed $value
      */
-    public function set($cacheId, $value)
+    public function set(string $cacheId, $value): void
     {
+        $cacheId = $this->sanitizeCacheId($cacheId);
+
         $this->setToFirstLevelCache($cacheId, $value);
         $this->secondLevelCache->set($cacheId, $value);
     }
 
     /**
-     * @return void
+     * Flushes the cache
      */
-    public function flush()
+    public function flush(): void
     {
         self::$firstLevelCache[$this->cacheName] = [];
         $this->secondLevelCache->flush();
     }
+
+    /**
+     * Sanitizes the cache id to ensure compatibility with the FrontendInterface::PATTERN_ENTRYIDENTIFIER
+     *
+     * @see \TYPO3\CMS\Core\Cache\Frontend\AbstractFrontend::isValidEntryIdentifier()
+     *
+     * @param string $cacheEntryIdentifier
+     * @return string
+     */
+    protected function sanitizeCacheId(string $cacheId): string
+    {
+        return preg_replace('/[^a-zA-Z0-9_%\\-&]/', '-', $cacheId);
+    }
 }

Tests/Unit/System/Cache/TwoLevelCacheTest.php

@@ -27,6 +27,8 @@
 use ApacheSolrForTypo3\Solr\System\Cache\TwoLevelCache;
 use ApacheSolrForTypo3\Solr\Tests\Unit\UnitTest;
 use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
+use TYPO3\CMS\Core\Cache\Frontend\VariableFrontend;
+use TYPO3\CMS\Core\Cache\Backend\BackendInterface;
 
 /**
  * Unit testcase to check if the two level cache is working as expected.
@@ -42,23 +44,37 @@ class TwoLevelCacheTest extends UnitTest
     protected $twoLevelCache;
 
     /**
-     * @var  \TYPO3\CMS\Core\Cache\Frontend\FrontendInterface
+     * @var  FrontendInterface
      */
     protected $secondLevelCacheMock;
 
     /**
-     * @return false
+     * Prepare
+     *
+     * @see \PHPUnit\Framework\TestCase::setUp()
      */
-    public function setUp()
+    protected function setUp(): void
     {
         $this->secondLevelCacheMock = $this->getDumbMock(FrontendInterface::class);
         $this->twoLevelCache = new TwoLevelCache('test', $this->secondLevelCacheMock);
     }
 
+    /**
+     * Cleanup
+     *
+     * {@inheritDoc}
+     * @see \Nimut\TestingFramework\TestCase\UnitTestCase::tearDown()
+     */
+    protected function tearDown(): void
+    {
+        $this->twoLevelCache->flush();
+        parent::tearDown();
+    }
+
     /**
      * @test
      */
-    public function getOnSecondaryCacheIsNeverCalledWhenValueIsPresentInFirstLevelCache()
+    public function getOnSecondaryCacheIsNeverCalledWhenValueIsPresentInFirstLevelCache(): void
     {
         $this->secondLevelCacheMock->expects($this->never())->method('get');
 
@@ -70,4 +86,30 @@ public function getOnSecondaryCacheIsNeverCalledWhenValueIsPresentInFirstLevelCa
         $value = $this->twoLevelCache->get('foo');
         $this->assertSame($value, 'bar', 'Did not get expected value from two level cache');
     }
+
+    /**
+     * @test
+     */
+    public function canHandleInvalidCacheIdentifierOnSet(): void
+    {
+        $cacheBackendMock = $this->createMock(BackendInterface::class);
+        $cacheBackendMock->expects($this->once())->method('set');
+        $variableFrontend = new VariableFrontend('TwoLevelCacheTest', $cacheBackendMock);
+        $this->inject($this->twoLevelCache, 'secondLevelCache', $variableFrontend);
+
+        $this->twoLevelCache->set('I.Am.An.Invalid.Identifier-#ß%&!', 'dummyValue');
+    }
+
+    /**
+     * @test
+     */
+    public function canHandleInvalidCacheIdentifierOnGet(): void
+    {
+        $cacheBackendMock = $this->createMock(BackendInterface::class);
+        $cacheBackendMock->expects($this->once())->method('get');
+        $variableFrontend = new VariableFrontend('TwoLevelCacheTest', $cacheBackendMock);
+        $this->inject($this->twoLevelCache, 'secondLevelCache', $variableFrontend);
+
+        $this->assertFalse($this->twoLevelCache->get('I.Am.An.Invalid.Identifier-#ß%&!'));
+    }
 }