Merge pull request #4637 from MGatner/cache-hash

Cache Key Validation

system/Cache/Handlers/BaseHandler.php

@@ -14,12 +14,63 @@
 use Closure;
 use CodeIgniter\Cache\CacheInterface;
 use Exception;
+use InvalidArgumentException;
 
 /**
  * Base class for cache handling
  */
 abstract class BaseHandler implements CacheInterface
 {
+	/**
+	 * Reserved characters that cannot be used in a key or tag.
+	 * From https://github.com/symfony/cache-contracts/blob/c0446463729b89dd4fa62e9aeecc80287323615d/ItemInterface.php#L43
+	 */
+	public const RESERVED_CHARACTERS = '{}()/\@:';
+
+	/**
+	 * Maximum key length.
+	 */
+	public const MAX_KEY_LENGTH = PHP_INT_MAX;
+
+	/**
+	 * Prefix to apply to cache keys.
+	 * May not be used by all handlers.
+	 *
+	 * @var string
+	 */
+	protected $prefix;
+
+	/**
+	 * Validates a cache key according to PSR-6.
+	 * Keys that exceed MAX_KEY_LENGTH are hashed.
+	 * From https://github.com/symfony/cache/blob/7b024c6726af21fd4984ac8d1eae2b9f3d90de88/CacheItem.php#L158
+	 *
+	 * @param string $key    The key to validate
+	 * @param string $prefix Optional prefix to include in length calculations
+	 *
+	 * @throws InvalidArgumentException When $key is not valid
+	 */
+	public static function validateKey($key, $prefix = ''): string
+	{
+		if (! is_string($key))
+		{
+			throw new InvalidArgumentException('Cache key must be a string');
+		}
+		if ($key === '')
+		{
+			throw new InvalidArgumentException('Cache key cannot be empty.');
+		}
+		if (strpbrk($key, self::RESERVED_CHARACTERS) !== false)
+		{
+			throw new InvalidArgumentException('Cache key contains reserved characters ' . self::RESERVED_CHARACTERS);
+		}
+
+		// If the key with prefix exceeds the length then return the hashed version
+		return strlen($prefix . $key) > static::MAX_KEY_LENGTH ? $prefix . md5($key) : $prefix . $key;
+	}
+
+	//--------------------------------------------------------------------
+
 	/**
 	 * Get an item from the cache, or execute the given Closure and store the result.
 	 *

system/Cache/Handlers/FileHandler.php

@@ -21,11 +21,9 @@
 class FileHandler extends BaseHandler
 {
 	/**
-	 * Prefixed to all cache names.
-	 *
-	 * @var string
+	 * Maximum key length.
 	 */
-	protected $prefix;
+	public const MAX_KEY_LENGTH = PHP_MAXPATHLEN;
 
 	/**
 	 * Where to store cached files on the disk.
@@ -94,8 +92,7 @@ public function initialize()
 	 */
 	public function get(string $key)
 	{
-		$key = $this->prefix . $key;
-
+		$key  = static::validateKey($key, $this->prefix);
 		$data = $this->getItem($key);
 
 		return is_array($data) ? $data['data'] : null;
@@ -114,7 +111,7 @@ public function get(string $key)
 	 */
 	public function save(string $key, $value, int $ttl = 60)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		$contents = [
 			'time' => time(),
@@ -152,7 +149,7 @@ public function save(string $key, $value, int $ttl = 60)
 	 */
 	public function delete(string $key)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		return is_file($this->path . $key) && unlink($this->path . $key);
 	}
@@ -170,7 +167,7 @@ public function deleteMatching(string $pattern)
 	{
 		$deleted = 0;
 
-		foreach (glob($this->path . $pattern,  GLOB_NOSORT) as $filename)
+		foreach (glob($this->path . $pattern, GLOB_NOSORT) as $filename)
 		{
 			if (is_file($filename) && @unlink($filename))
 			{
@@ -193,8 +190,7 @@ public function deleteMatching(string $pattern)
 	 */
 	public function increment(string $key, int $offset = 1)
 	{
-		$key = $this->prefix . $key;
-
+		$key  = static::validateKey($key, $this->prefix);
 		$data = $this->getItem($key);
 
 		if ($data === false)
@@ -222,12 +218,11 @@ public function increment(string $key, int $offset = 1)
 	 * @param string  $key    Cache ID
 	 * @param integer $offset Step/value to increase by
 	 *
-	 * @return bool
+	 * @return boolean
 	 */
 	public function decrement(string $key, int $offset = 1)
 	{
-		$key = $this->prefix . $key;
-
+		$key  = static::validateKey($key, $this->prefix);
 		$data = $this->getItem($key);
 
 		if ($data === false)
@@ -288,7 +283,7 @@ public function getCacheInfo()
 	 */
 	public function getMetaData(string $key)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		if (! is_file($this->path . $key))
 		{
@@ -342,26 +337,26 @@ public function isSupported(): bool
 	 * Does the heavy lifting of actually retrieving the file and
 	 * verifying it's age.
 	 *
-	 * @param string $key
+	 * @param string $filename
 	 *
 	 * @return boolean|mixed
 	 */
-	protected function getItem(string $key)
+	protected function getItem(string $filename)
 	{
-		if (! is_file($this->path . $key))
+		if (! is_file($this->path . $filename))
 		{
 			return false;
 		}
 
-		$data = unserialize(file_get_contents($this->path . $key));
+		$data = unserialize(file_get_contents($this->path . $filename));
 
 		// @phpstan-ignore-next-line
 		if ($data['ttl'] > 0 && time() > $data['time'] + $data['ttl'])
 		{
 			// If the file is still there then remove it
-			if (is_file($this->path . $key))
+			if (is_file($this->path . $filename))
 			{
-				unlink($this->path . $key);
+				unlink($this->path . $filename);
 			}
 
 			return false;

system/Cache/Handlers/MemcachedHandler.php

@@ -22,13 +22,6 @@
  */
 class MemcachedHandler extends BaseHandler
 {
-	/**
-	 * Prefixed to all cache names.
-	 *
-	 * @var string
-	 */
-	protected $prefix;
-
 	/**
 	 * The memcached object
 	 *
@@ -166,7 +159,7 @@ public function initialize()
 	 */
 	public function get(string $key)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		if ($this->memcached instanceof Memcached)
 		{
@@ -206,7 +199,7 @@ public function get(string $key)
 	 */
 	public function save(string $key, $value, int $ttl = 60)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		if (! $this->config['raw'])
 		{
@@ -242,7 +235,7 @@ public function save(string $key, $value, int $ttl = 60)
 	 */
 	public function delete(string $key)
 	{
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		return $this->memcached->delete($key);
 	}
@@ -278,7 +271,7 @@ public function increment(string $key, int $offset = 1)
 			return false;
 		}
 
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		// @phpstan-ignore-next-line
 		return $this->memcached->increment($key, $offset, $offset, 60);
@@ -301,7 +294,7 @@ public function decrement(string $key, int $offset = 1)
 			return false;
 		}
 
-		$key = $this->prefix . $key;
+		$key = static::validateKey($key, $this->prefix);
 
 		//FIXME: third parameter isn't other handler actions.
 		// @phpstan-ignore-next-line
@@ -349,8 +342,7 @@ public function getCacheInfo()
 	 */
 	public function getMetaData(string $key)
 	{
-		$key = $this->prefix . $key;
-
+		$key    = static::validateKey($key, $this->prefix);
 		$stored = $this->memcached->get($key);
 
 		// if not an array, don't try to count for PHP7.2

system/Cache/Handlers/PredisHandler.php

@@ -22,13 +22,6 @@
  */
 class PredisHandler extends BaseHandler
 {
-	/**
-	 * Prefixed to all cache names.
-	 *
-	 * @var string
-	 */
-	protected $prefix;
-
 	/**
 	 * Default config
 	 *
@@ -101,8 +94,12 @@ public function initialize()
 	 */
 	public function get(string $key)
 	{
-		$data = array_combine(
-			['__ci_type', '__ci_value'],
+		$key = static::validateKey($key);
+
+		$data = array_combine([
+			'__ci_type',
+			'__ci_value',
+		],
 			$this->redis->hmget($key, ['__ci_type', '__ci_value'])
 		);
 
@@ -141,6 +138,8 @@ public function get(string $key)
 	 */
 	public function save(string $key, $value, int $ttl = 60)
 	{
+		$key = static::validateKey($key);
+
 		switch ($dataType = gettype($value))
 		{
 			case 'array':
@@ -179,6 +178,8 @@ public function save(string $key, $value, int $ttl = 60)
 	 */
 	public function delete(string $key)
 	{
+		$key = static::validateKey($key);
+
 		return $this->redis->del($key) === 1;
 	}
 
@@ -215,6 +216,8 @@ public function deleteMatching(string $pattern)
 	 */
 	public function increment(string $key, int $offset = 1)
 	{
+		$key = static::validateKey($key);
+
 		return $this->redis->hincrby($key, 'data', $offset);
 	}
 
@@ -230,6 +233,8 @@ public function increment(string $key, int $offset = 1)
 	 */
 	public function decrement(string $key, int $offset = 1)
 	{
+		$key = static::validateKey($key);
+
 		return $this->redis->hincrby($key, 'data', -$offset);
 	}
 
@@ -273,6 +278,8 @@ public function getCacheInfo()
 	 */
 	public function getMetaData(string $key)
 	{
+		$key = static::validateKey($key);
+
 		$data = array_combine(['__ci_value'], $this->redis->hmget($key, ['__ci_value']));
 
 		if (isset($data['__ci_value']) && $data['__ci_value'] !== false)