From 23bf9be8ce16009baefae55c6fae7b754670fac1 Mon Sep 17 00:00:00 2001
From: Lars Strojny <lars.strojny@internations.org>
Date: Tue, 8 Sep 2020 11:20:58 +0200
Subject: [PATCH] [Cache] Fix key encoding issue in Memcached adapter

---
 .../Tests/Adapter/MemcachedAdapterTest.php    | 36 +++++++++++++++++--
 .../Component/Cache/Traits/MemcachedTrait.php | 28 ++++++++++++---
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php
index 6c9afe04ab..26609ff443 100644
--- a/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php
+++ b/src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php
@@ -42,11 +42,11 @@ class MemcachedAdapterTest extends AdapterTestCase
         }
     }
 
-    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 @@ class MemcachedAdapterTest extends AdapterTestCase
         ];
         $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));
+    }
 }
diff --git a/src/Symfony/Component/Cache/Traits/MemcachedTrait.php b/src/Symfony/Component/Cache/Traits/MemcachedTrait.php
index 4aa7beefd2..468656e333 100644
--- a/src/Symfony/Component/Cache/Traits/MemcachedTrait.php
+++ b/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 @@ trait MemcachedTrait
 
         $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 @@ trait MemcachedTrait
     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 @@ trait MemcachedTrait
      */
     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 @@ trait MemcachedTrait
     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 @@ trait MemcachedTrait
 
         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);
+    }
 }