8000 bug #38108 [Cache] Fix key encoding issue in Memcached adapter (lstro… · symfony/symfony@afd4027 · GitHub
[go: up one dir, main page]

Skip to content

Commit afd4027

Browse files
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 ------- 23bf9be [Cache] Fix key encoding issue in Memcached adapter
2 parents 9bb8084 + 23bf9be commit afd4027

File tree

2 files changed

+57
-7
lines changed

2 files changed

+57
-7
lines changed

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ public static function setUpBeforeClass(): void
4242
}
4343
}
4444

45-
public function createCachePool(int $defaultLifetime = 0): CacheItemPoolInterface
45+
public function createCachePool(int $defaultLifetime = 0, string $testMethod = null, string $namespace = null): CacheItemPoolInterface
4646
{
4747
$client = $defaultLifetime ? AbstractAdapter::createConnection('memcached://'.getenv('MEMCACHED_HOST')) : self::$client;
4848

49-
return new MemcachedAdapter($client, str_replace('\\', '.', __CLASS__), $defaultLifetime);
49+
return new MemcachedAdapter($client, $namespace ?? str_replace('\\', '.', __CLASS__), $defaultLifetime);
5050
}
5151

5252
public function testOptions()
@@ -248,4 +248,36 @@ public function testMultiServerDsn()
248248
];
249249
$this->assertSame($expected, $client->getServerList());
250250
}
251+
252+
public function testKeyEncoding()
253+
{
254+
$reservedMemcachedCharacters = " \n\r\t\v\f\0";
255+
256+
$namespace = $reservedMemcachedCharacters.random_int(0, \PHP_INT_MAX);
257+
$pool = $this->createCachePool(0, null, $namespace);
258+
259+
/**
260+
* Choose a key that is below {@see \Symfony\Component\Cache\Adapter\MemcachedAdapter::$maxIdLength} so that
261+
* {@see \Symfony\Component\Cache\Traits\AbstractTrait::getId()} does not shorten the key but choose special
262+
* characters that would be encoded and therefore increase the key length over the Memcached limit.
263+
*/
264+
// 250 is Memcached’s max key length, 7 bytes for prefix seed
265+
$key = str_repeat('%', 250 - 7 - \strlen($reservedMemcachedCharacters) - \strlen($namespace)).$reservedMemcachedCharacters;
266+
267+
self::assertFalse($pool->hasItem($key));
268+
269+
$item = $pool->getItem($key);
270+
self::assertFalse($item->isHit());
271+
self::assertSame($key, $item->getKey());
272+
273+
self::assertTrue($pool->save($item->set('foobar')));
274+
275+
self::assertTrue($pool->hasItem($key));
276+
$item = $pool->getItem($key);
277+
self::assertTrue($item->isHit());
278+
self::assertSame($key, $item->getKey());
279+
280+
self::assertTrue($pool->deleteItem($key));
281+
self::assertFalse($pool->hasItem($key));
282+
}
251283
}

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ trait MemcachedTrait
3131
\Memcached::OPT_SERIALIZER => \Memcached::SERIALIZER_PHP,
3232
];
3333

34+
/**
35+
* We are replacing characters that are illegal in Memcached keys with reserved characters from
36+
* {@see \Symfony\Contracts\Cache\ItemInterface::RESERVED_CHARACTERS} that are legal in Memcached.
37+
* Note: don’t use {@see \Symfony\Component\Cache\Adapter\AbstractAdapter::NS_SEPARATOR}.
38+
*/
39+
private static $RESERVED_MEMCACHED = " \n\r\t\v\f\0";
40+
private static $RESERVED_PSR6 = '@()\{}/';
41+
3442
private $marshaller;
3543
private $client;
3644
private $lazyClient;
@@ -235,7 +243,7 @@ protected function doSave(array $values, int $lifetime)
235243

236244
$encodedValues = [];
237245
foreach ($values as $key => $value) {
238-
$encodedValues[rawurlencode($key)] = $value;
246+
$encodedValues[self::encodeKey($key)] = $value;
239247
}
240248

241249
return $this->checkResultCode($this->getClient()->setMulti($encodedValues, $lifetime)) ? $failed : false;
@@ -247,13 +255,13 @@ protected function doSave(array $values, int $lifetime)
247255
protected function doFetch(array $ids)
248256
{
249257
try {
250-
$encodedIds = array_map('rawurlencode', $ids);
258+
$encodedIds = array_map('self::encodeKey', $ids);
251259

252260
$encodedResult = $this->checkResultCode($this->getClient()->getMulti($encodedIds));
253261

254262
$result = [];
255263
foreach ($encodedResult as $key => $value) {
256-
$result[rawurldecode($key)] = $this->marshaller->unmarshall($value);
264+
$result[self::decodeKey($key)] = $this->marshaller->unmarshall($value);
257265
}
258266

259267
return $result;
@@ -267,7 +275,7 @@ protected function doFetch(array $ids)
267275
*/
268276
protected function doHave($id)
269277
{
270-
return false !== $this->getClient()->get(rawurlencode($id)) || $this->checkResultCode(\Memcached::RES_SUCCESS === $this->client->getResultCode());
278+
return false !== $this->getClient()->get(self::encodeKey($id)) || $this->checkResultCode(\Memcached::RES_SUCCESS === $this->client->getResultCode());
271279
}
272280

273281
/**
@@ -276,7 +284,7 @@ protected function doHave($id)
276284
protected function doDelete(array $ids)
277285
{
278286
$ok = true;
279-
$encodedIds = array_map('rawurlencode', $ids);
287+
$encodedIds = array_map('self::encodeKey', $ids);
280288
foreach ($this->checkResultCode($this->getClient()->deleteMulti($encodedIds)) as $result) {
281289
if (\Memcached::RES_SUCCESS !== $result && \Memcached::RES_NOTFOUND !== $result) {
282290
$ok = false;
@@ -322,4 +330,14 @@ private function getClient(): \Memcached
322330

323331
return $this->client = $this->lazyClient;
324332
}
333+
334+
private static function encodeKey(string $key): string
335+
{
336+
return strtr($key, self::$RESERVED_MEMCACHED, self::$RESERVED_PSR6);
337+
}
338+
339+
private static function decodeKey(string $key): string
340+
{
341+
return strtr($key, self::$RESERVED_PSR6, self::$RESERVED_MEMCACHED);
342+
}
325343
}

0 commit comments

Comments
 (0)
0