Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit afd4027

Browse filesBrowse 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
Copy full SHA for afd4027

File tree

2 files changed

+57
-7
lines changed
Filter options

2 files changed

+57
-7
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Tests/Adapter/MemcachedAdapterTest.php
+34-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Traits/MemcachedTrait.php
+23-5Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.