bug #38108 [Cache] Fix key encoding issue in Memcached adapter (lstrojny)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Cache] Fix key encoding issue in Memcached adapter

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | n.A.
| License       | MIT
| Doc PR        | Fix double encoding in memcached which lead to overlong keys being generated

Because the memcached adapter uses `rawurlencode()` to encode each and every key, keys will sometimes be too long and therefore hit the memcached limit of 250 bytes. This happens when the key is small enough to be below 250 when passed to `AbstractAdapterTrait::getId()` and is then blown up over the 250 bytes limit in memcached adapter without validating the length again.

Looking through the code, it seems that the double encoding is wholly unnecessary assuming if one makes sure the namespace is properly encoded. This PR therefore removes the double encoding and instead uses rawurlencode on the namespace (which is in turn properly accounted for when calculating whether or not we are over the ID limit).

Commits
-------

23bf9be8ce [Cache] Fix key encoding issue in Memcached adapter

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));
+    }
 }

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);
+    }
 }