[Cache] Fix key encoding issue in Memcached adapter

Author: lstrojny

src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php

@@ -42,11 +42,11 @@ public static function setUpBeforeClass(): void
         }
     }
 
-    public function createCachePool(int $defaultLifetime = 0): CacheItemPoolInterface
+    public function createCachePool(int $defaultLifetime = 0, string $testMethod = null, string $namespace = null): CacheItemPoolInterface
     {
         $client = $defaultLifetime ? AbstractAdapter::createConnection('memcached://'.getenv('MEMCACHED_HOST')) : self::$client;
 
-        return new MemcachedAdapter($client, str_replace('\\', '.', __CLASS__), $defaultLifetime);
+        return new MemcachedAdapter($client, $namespace ?? str_replace('\\', '.', __CLASS__), $defaultLifetime);
     }
 
     public function testOptions()
@@ -248,4 +248,36 @@ public function testMultiServerDsn()
         ];
         $this->assertSame($expected, $client->getServerList());
     }
+
+    public function testKeyEncoding()
+    {
+        $reservedMemcachedCharacters = " \n\r\t\v\f\0";
+
+        $namespace = $reservedMemcachedCharacters.random_int(0, \PHP_INT_MAX);
+        $pool = $this->createCachePool(0, null, $namespace);
+
+        /**
+         * Choose a key that is below {@see \Symfony\Component\Cache\Adapter\MemcachedAdapter::$maxIdLength} so that
+         * {@see \Symfony\Component\Cache\Traits\AbstractTrait::getId()} does not shorten the key but choose special
+         * characters that would be encoded and therefore increase the key length over the Memcached limit.
+         */
+        // 250 is Memcached’s max key length, 7 bytes for prefix seed
+        $key = str_repeat('%', 250 - 7 - \strlen($reservedMemcachedCharacters) - \strlen($namespace)).$reservedMemcachedCharacters;
+
+        self::assertFalse($pool->hasItem($key));
+
+        $item = $pool->getItem($key);
+        self::assertFalse($item->isHit());
+        self::assertSame($key, $item->getKey());
+
+        self::assertTrue($pool->save($item->set('foobar')));
+
+        self::assertTrue($pool->hasItem($key));
+        $item = $pool->getItem($key);
+        self::assertTrue($item->isHit());
+        self::assertSame($key, $item->getKey());
+
+        self::assertTrue($pool->deleteItem($key));
+        self::assertFalse($pool->hasItem($key));
+    }
 }

src/Symfony/Component/Cache/Traits/MemcachedTrait.php

@@ -31,6 +31,14 @@ trait MemcachedTrait
         \Memcached::OPT_SERIALIZER => \Memcached::SERIALIZER_PHP,
     ];
 
+    /**
+     * We are replacing characters that are illegal in Memcached keys with reserved characters from
+     * {@see \Symfony\Contracts\Cache\ItemInterface::RESERVED_CHARACTERS} that are legal in Memcached.
+     * Note: don’t use {@see \Symfony\Component\Cache\Adapter\AbstractAdapter::NS_SEPARATOR}.
+     */
+    private static $RESERVED_MEMCACHED = " \n\r\t\v\f\0";
+    private static $RESERVED_PSR6 = '@()\{}/';
+
     private $marshaller;
     private $client;
     private $lazyClient;
@@ -235,7 +243,7 @@ protected function doSave(array $values, int $lifetime)
 
         $encodedValues = [];
         foreach ($values as $key => $value) {
-            $encodedValues[rawurlencode($key)] = $value;
+            $encodedValues[self::encodeKey($key)] = $value;
         }
 
         return $this->checkResultCode($this->getClient()->setMulti($encodedValues, $lifetime)) ? $failed : false;
@@ -247,13 +255,13 @@ protected function doSave(array $values, int $lifetime)
     protected function doFetch(array $ids)
     {
         try {
-            $encodedIds = array_map('rawurlencode', $ids);
+            $encodedIds = array_map('self::encodeKey', $ids);
 
             $encodedResult = $this->checkResultCode($this->getClient()->getMulti($encodedIds));
 
             $result = [];
             foreach ($encodedResult as $key => $value) {
-                $result[rawurldecode($key)] = $this->marshaller->unmarshall($value);
+                $result[self::decodeKey($key)] = $this->marshaller->unmarshall($value);
             }
 
             return $result;
@@ -267,7 +275,7 @@ protected function doFetch(array $ids)
      */
     protected function doHave($id)
     {
-        return false !== $this->getClient()->get(rawurlencode($id)) || $this->checkResultCode(\Memcached::RES_SUCCESS === $this->client->getResultCode());
+        return false !== $this->getClient()->get(self::encodeKey($id)) || $this->checkResultCode(\Memcached::RES_SUCCESS === $this->client->getResultCode());
     }
 
     /**
@@ -276,7 +284,7 @@ protected function doHave($id)
     protected function doDelete(array $ids)
     {
         $ok = true;
-        $encodedIds = array_map('rawurlencode', $ids);
+        $encodedIds = array_map('self::encodeKey', $ids);
         foreach ($this->checkResultCode($this->getClient()->deleteMulti($encodedIds)) as $result) {
             if (\Memcached::RES_SUCCESS !== $result && \Memcached::RES_NOTFOUND !== $result) {
                 $ok = false;
@@ -322,4 +330,14 @@ private function getClient(): \Memcached
 
         return $this->client = $this->lazyClient;
     }
+
+    private static function encodeKey(string $key): string
+    {
+        return strtr($key, self::$RESERVED_MEMCACHED, self::$RESERVED_PSR6);
+    }
+
+    private static function decodeKey(string $key): string
+    {
+        return strtr($key, self::$RESERVED_PSR6, self::$RESERVED_MEMCACHED);
+    }
 }