From 3215c392aa2b6a836ca5e421deb0178cc6eefbec Mon Sep 17 00:00:00 2001 From: "Timothy J. Warren" Date: Fri, 5 Feb 2021 13:26:01 -0500 Subject: [PATCH] Improve validation of cache keys and update dependencies --- build/phpunit.xml | 45 +++++++++++++-------------- composer.json | 7 ++--- src/Driver/AbstractDriver.php | 6 ++++ src/Driver/ApcuDriver.php | 6 ++++ src/Driver/MemcachedDriver.php | 8 +++++ src/Driver/NullDriver.php | 2 ++ src/Driver/RedisDriver.php | 4 ++- src/KeyValidateTrait.php | 55 +++++++++++++++++++++++++++++++++ src/_Driver.php | 38 ++--------------------- tests/Driver/DriverTestBase.php | 18 +++++++++++ 10 files changed, 124 insertions(+), 65 deletions(-) create mode 100644 src/KeyValidateTrait.php diff --git a/build/phpunit.xml b/build/phpunit.xml index 7177f88..68de89e 100644 --- a/build/phpunit.xml +++ b/build/phpunit.xml @@ -1,25 +1,22 @@ - - - - ../src - - - - - ../tests - - - - - - - - - - \ No newline at end of file + + + + ../src + + + + + + + + + + + ../tests + + + + + + diff --git a/composer.json b/composer.json index c183ee0..aa7d3a3 100644 --- a/composer.json +++ b/composer.json @@ -39,12 +39,11 @@ "consolidation/robo": "^2.0.0", "monolog/monolog": "^2.0.1", "pdepend/pdepend": "^2.2", - "phploc/phploc": "^5.0", + "phploc/phploc": "^7.0", "phpmd/phpmd": "^2.4", - "phpunit/phpunit": "^8.5.0", - "sebastian/phpcpd": "^4.1", + "phpunit/phpunit": "^9.5.0", + "sebastian/phpcpd": "^6.0.3", "squizlabs/php_codesniffer": "^3.3.2", - "theseer/phpdox": "^0.12.0", "phpstan/phpstan": "^0.12.2" }, "suggest": { diff --git a/src/Driver/AbstractDriver.php b/src/Driver/AbstractDriver.php index 8977cf3..dd3abb0 100644 --- a/src/Driver/AbstractDriver.php +++ b/src/Driver/AbstractDriver.php @@ -16,6 +16,7 @@ namespace Aviat\Banker\Driver; use Aviat\Banker\LoggerTrait; +use Aviat\Banker\KeyValidateTrait; use Psr\Log\LoggerAwareInterface; /** @@ -23,6 +24,7 @@ use Psr\Log\LoggerAwareInterface; */ abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface { + use KeyValidateTrait; use LoggerTrait; /** @@ -48,6 +50,8 @@ abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface { */ public function getMultiple(array $keys = []): array { + $this->validateKeys($keys); + $output = []; foreach ($keys as $key) @@ -70,6 +74,8 @@ abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface { */ public function setMultiple(array $items, ?int $expires = NULL): bool { + $this->validateKeys($items, TRUE); + $setResults = []; foreach ($items as $k => $v) { diff --git a/src/Driver/ApcuDriver.php b/src/Driver/ApcuDriver.php index 2beedda..cc0587c 100644 --- a/src/Driver/ApcuDriver.php +++ b/src/Driver/ApcuDriver.php @@ -74,6 +74,8 @@ class ApcuDriver extends AbstractDriver { */ public function getMultiple(array $keys = []): array { + $this->validateKeys($keys); + $status = FALSE; return apcu_fetch($keys, $status); } @@ -102,6 +104,8 @@ class ApcuDriver extends AbstractDriver { */ public function setMultiple(array $items, ?int $expires = NULL): bool { + $this->validateKeys($items, TRUE); + $ttl = $this->getTTLFromExpiration((int)$expires); $errorKeys = ($expires === NULL) @@ -130,6 +134,8 @@ class ApcuDriver extends AbstractDriver { */ public function deleteMultiple(array $keys = []): bool { + $this->validateKeys($keys); + $failedToDelete = apcu_delete($keys); return empty($failedToDelete); } diff --git a/src/Driver/MemcachedDriver.php b/src/Driver/MemcachedDriver.php index 1fd40b8..10567a0 100644 --- a/src/Driver/MemcachedDriver.php +++ b/src/Driver/MemcachedDriver.php @@ -109,6 +109,8 @@ class MemcachedDriver extends AbstractDriver { */ public function getMultiple(array $keys = []): array { + $this->validateKeys($keys); + $response = $this->conn->getMulti($keys); return (is_array($response)) ? $response : []; } @@ -123,6 +125,8 @@ class MemcachedDriver extends AbstractDriver { */ public function set(string $key, $value, ?int $expires = NULL): bool { + $this->validateKey($key); + return ($expires === NULL) ? $this->conn->set($key, $value) : $this->conn->set($key, $value, $expires); @@ -137,6 +141,8 @@ class MemcachedDriver extends AbstractDriver { */ public function setMultiple(array $items, ?int $expires = NULL): bool { + $this->validateKeys($items, TRUE); + return ($expires === NULL) ? $this->conn->setMulti($items) : $this->conn->setMulti($items, $expires); @@ -161,6 +167,8 @@ class MemcachedDriver extends AbstractDriver { */ public function deleteMultiple(array $keys = []): bool { + $this->validateKeys($keys); + $deleted = $this->conn->deleteMulti($keys); foreach ($deleted as $key => $status) diff --git a/src/Driver/NullDriver.php b/src/Driver/NullDriver.php index 62b7c69..97890a6 100644 --- a/src/Driver/NullDriver.php +++ b/src/Driver/NullDriver.php @@ -104,6 +104,8 @@ class NullDriver extends AbstractDriver { */ public function deleteMultiple(array $keys = []): bool { + $this->validateKeys($keys); + $res = TRUE; foreach($keys as $key) diff --git a/src/Driver/RedisDriver.php b/src/Driver/RedisDriver.php index 72e76cf..fd002f4 100644 --- a/src/Driver/RedisDriver.php +++ b/src/Driver/RedisDriver.php @@ -119,7 +119,9 @@ class RedisDriver extends AbstractDriver { */ public function deleteMultiple(array $keys = []): bool { - $res = $this->conn->del(...$keys); + $this->validateKeys($keys); + + $res = $this->conn->del(...array_values($keys)); return $res === count($keys); } diff --git a/src/KeyValidateTrait.php b/src/KeyValidateTrait.php new file mode 100644 index 0000000..39433ee --- /dev/null +++ b/src/KeyValidateTrait.php @@ -0,0 +1,55 @@ + + * @copyright 2016 - 2020 Timothy J. Warren + * @license http://www.opensource.org/licenses/mit-license.html MIT License + * @version 3.1.0 + * @link https://git.timshomepage.net/timw4mail/banker + */ +namespace Aviat\Banker; + +use Aviat\Banker\Exception\InvalidArgumentException; + +trait KeyValidateTrait { + /** + * @param $keys + * @param bool $hash + * @throws InvalidArgumentException + */ + protected function validateKeys($keys, bool $hash = FALSE): void + { + // Check type of keys + if ( ! is_iterable($keys)) + { + throw new InvalidArgumentException('Keys must be an array or a traversable object'); + } + + $keys = ($hash) ? array_keys((array)$keys) : (array)$keys; + + // Check each key + array_walk($keys, fn($key) => $this->validateKey($key)); + } + + /** + * @param string $key + * @throws InvalidArgumentException + */ + protected function validateKey($key): void + { + if ( ! is_string($key)) + { + throw new InvalidArgumentException('Cache key must be a string.'); + } + else if (preg_match("`[{}()/@:\\\]`", $key) === 1) + { + throw new InvalidArgumentException('Invalid characters in cache key'); + } + } +} \ No newline at end of file diff --git a/src/_Driver.php b/src/_Driver.php index 88fd747..b554063 100644 --- a/src/_Driver.php +++ b/src/_Driver.php @@ -22,6 +22,8 @@ use Aviat\Banker\Exception\InvalidArgumentException; * Private trait for shared driver-related functionality */ trait _Driver { + use KeyValidateTrait; + /** * Driver class for handling the chosen caching backend * @@ -45,40 +47,4 @@ trait _Driver { return new $class($driverConfig['connection'], $driverConfig['options']); } - - /** - * @param $keys - * @param bool $hash - * @throws InvalidArgumentException - */ - private function validateKeys($keys, bool $hash = FALSE): void - { - // Check type of keys - if ( ! is_iterable($keys)) - { - throw new InvalidArgumentException('Keys must be an array or a traversable object'); - } - - $keys = ($hash) ? array_keys((array)$keys) : (array)$keys; - - // Check each key - array_walk($keys, fn($key) => $this->validateKey($key)); - } - - /** - * @param string $key - * @throws InvalidArgumentException - */ - private function validateKey($key): void - { - if ( ! is_string($key)) - { - throw new InvalidArgumentException('Cache key must be a string.'); - } - - if (is_string($key) && preg_match("`[{}()/@:\\\]`", $key) === 1) - { - throw new InvalidArgumentException('Invalid characters in cache key'); - } - } } \ No newline at end of file diff --git a/tests/Driver/DriverTestBase.php b/tests/Driver/DriverTestBase.php index bb7798b..44e5f2a 100644 --- a/tests/Driver/DriverTestBase.php +++ b/tests/Driver/DriverTestBase.php @@ -16,6 +16,7 @@ namespace Aviat\Banker\Tests\Driver; use Aviat\Banker\Driver\DriverInterface; +use Aviat\Banker\Exception\InvalidArgumentException; use PHPUnit\Framework\TestCase; class DriverTestBase extends TestCase { @@ -90,6 +91,17 @@ class DriverTestBase extends TestCase { $this->assertEquals($data, $this->driver->getMultiple(array_keys($data))); } + public function testSetMultipleInvalidKey(): void + { + $this->expectException(InvalidArgumentException::class); + $data = [ + 128 => 0, + 0x123 => 1, + ]; + + $this->driver->setMultiple($data); + } + public function testSetMultipleExpires(): void { $data = [ @@ -114,6 +126,12 @@ class DriverTestBase extends TestCase { $this->assertEquals('bar', $this->driver->get('foo')); } + public function testSetInvalidKey(): void + { + $this->expectException(\TypeError::class); + $this->driver->set(0x12, 'foo'); + } + public function testDelete(): void { $this->driver->set('a1', 'b2');