Improve validation of cache keys and update dependencies
Gitea - aviat/banker/pipeline/head There was a failure building this commit Details

This commit is contained in:
Timothy Warren 2021-02-05 13:26:01 -05:00
parent 624e0efd97
commit 3215c392aa
10 changed files with 124 additions and 65 deletions

View File

@ -1,25 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<phpunit <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" colors="true" stopOnFailure="false" bootstrap="../tests/bootstrap.php" beStrictAboutTestsThatDoNotTestAnything="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
colors="true" <coverage>
stopOnFailure="false" <include>
bootstrap="../tests/bootstrap.php" <directory suffix=".php">../src</directory>
beStrictAboutTestsThatDoNotTestAnything="true" </include>
> <report>
<filter> <clover outputFile="logs/clover.xml"/>
<whitelist> <crap4j outputFile="logs/crap4j.xml"/>
<directory suffix=".php">../src</directory> <html outputDirectory="../coverage"/>
</whitelist> <xml outputDirectory="logs/coverage"/>
</filter> </report>
<testsuites> </coverage>
<testsuite name="Cache"> <testsuites>
<directory>../tests</directory> <testsuite name="Cache">
</testsuite> <directory>../tests</directory>
</testsuites> </testsuite>
<logging> </testsuites>
<log type="coverage-html" target="../coverage"/> <logging>
<log type="coverage-clover" target="logs/clover.xml"/> <junit outputFile="logs/junit.xml"/>
<log type="coverage-crap4j" target="logs/crap4j.xml"/> </logging>
<log type="coverage-xml" target="logs/coverage" /> </phpunit>
<log type="junit" target="logs/junit.xml" />
</logging>
</phpunit>

View File

@ -39,12 +39,11 @@
"consolidation/robo": "^2.0.0", "consolidation/robo": "^2.0.0",
"monolog/monolog": "^2.0.1", "monolog/monolog": "^2.0.1",
"pdepend/pdepend": "^2.2", "pdepend/pdepend": "^2.2",
"phploc/phploc": "^5.0", "phploc/phploc": "^7.0",
"phpmd/phpmd": "^2.4", "phpmd/phpmd": "^2.4",
"phpunit/phpunit": "^8.5.0", "phpunit/phpunit": "^9.5.0",
"sebastian/phpcpd": "^4.1", "sebastian/phpcpd": "^6.0.3",
"squizlabs/php_codesniffer": "^3.3.2", "squizlabs/php_codesniffer": "^3.3.2",
"theseer/phpdox": "^0.12.0",
"phpstan/phpstan": "^0.12.2" "phpstan/phpstan": "^0.12.2"
}, },
"suggest": { "suggest": {

View File

@ -16,6 +16,7 @@
namespace Aviat\Banker\Driver; namespace Aviat\Banker\Driver;
use Aviat\Banker\LoggerTrait; use Aviat\Banker\LoggerTrait;
use Aviat\Banker\KeyValidateTrait;
use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerAwareInterface;
/** /**
@ -23,6 +24,7 @@ use Psr\Log\LoggerAwareInterface;
*/ */
abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface { abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface {
use KeyValidateTrait;
use LoggerTrait; use LoggerTrait;
/** /**
@ -48,6 +50,8 @@ abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface {
*/ */
public function getMultiple(array $keys = []): array public function getMultiple(array $keys = []): array
{ {
$this->validateKeys($keys);
$output = []; $output = [];
foreach ($keys as $key) foreach ($keys as $key)
@ -70,6 +74,8 @@ abstract class AbstractDriver implements DriverInterface, LoggerAwareInterface {
*/ */
public function setMultiple(array $items, ?int $expires = NULL): bool public function setMultiple(array $items, ?int $expires = NULL): bool
{ {
$this->validateKeys($items, TRUE);
$setResults = []; $setResults = [];
foreach ($items as $k => $v) foreach ($items as $k => $v)
{ {

View File

@ -74,6 +74,8 @@ class ApcuDriver extends AbstractDriver {
*/ */
public function getMultiple(array $keys = []): array public function getMultiple(array $keys = []): array
{ {
$this->validateKeys($keys);
$status = FALSE; $status = FALSE;
return apcu_fetch($keys, $status); return apcu_fetch($keys, $status);
} }
@ -102,6 +104,8 @@ class ApcuDriver extends AbstractDriver {
*/ */
public function setMultiple(array $items, ?int $expires = NULL): bool public function setMultiple(array $items, ?int $expires = NULL): bool
{ {
$this->validateKeys($items, TRUE);
$ttl = $this->getTTLFromExpiration((int)$expires); $ttl = $this->getTTLFromExpiration((int)$expires);
$errorKeys = ($expires === NULL) $errorKeys = ($expires === NULL)
@ -130,6 +134,8 @@ class ApcuDriver extends AbstractDriver {
*/ */
public function deleteMultiple(array $keys = []): bool public function deleteMultiple(array $keys = []): bool
{ {
$this->validateKeys($keys);
$failedToDelete = apcu_delete($keys); $failedToDelete = apcu_delete($keys);
return empty($failedToDelete); return empty($failedToDelete);
} }

View File

@ -109,6 +109,8 @@ class MemcachedDriver extends AbstractDriver {
*/ */
public function getMultiple(array $keys = []): array public function getMultiple(array $keys = []): array
{ {
$this->validateKeys($keys);
$response = $this->conn->getMulti($keys); $response = $this->conn->getMulti($keys);
return (is_array($response)) ? $response : []; return (is_array($response)) ? $response : [];
} }
@ -123,6 +125,8 @@ class MemcachedDriver extends AbstractDriver {
*/ */
public function set(string $key, $value, ?int $expires = NULL): bool public function set(string $key, $value, ?int $expires = NULL): bool
{ {
$this->validateKey($key);
return ($expires === NULL) return ($expires === NULL)
? $this->conn->set($key, $value) ? $this->conn->set($key, $value)
: $this->conn->set($key, $value, $expires); : $this->conn->set($key, $value, $expires);
@ -137,6 +141,8 @@ class MemcachedDriver extends AbstractDriver {
*/ */
public function setMultiple(array $items, ?int $expires = NULL): bool public function setMultiple(array $items, ?int $expires = NULL): bool
{ {
$this->validateKeys($items, TRUE);
return ($expires === NULL) return ($expires === NULL)
? $this->conn->setMulti($items) ? $this->conn->setMulti($items)
: $this->conn->setMulti($items, $expires); : $this->conn->setMulti($items, $expires);
@ -161,6 +167,8 @@ class MemcachedDriver extends AbstractDriver {
*/ */
public function deleteMultiple(array $keys = []): bool public function deleteMultiple(array $keys = []): bool
{ {
$this->validateKeys($keys);
$deleted = $this->conn->deleteMulti($keys); $deleted = $this->conn->deleteMulti($keys);
foreach ($deleted as $key => $status) foreach ($deleted as $key => $status)

View File

@ -104,6 +104,8 @@ class NullDriver extends AbstractDriver {
*/ */
public function deleteMultiple(array $keys = []): bool public function deleteMultiple(array $keys = []): bool
{ {
$this->validateKeys($keys);
$res = TRUE; $res = TRUE;
foreach($keys as $key) foreach($keys as $key)

View File

@ -119,7 +119,9 @@ class RedisDriver extends AbstractDriver {
*/ */
public function deleteMultiple(array $keys = []): bool 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); return $res === count($keys);
} }

55
src/KeyValidateTrait.php Normal file
View File

@ -0,0 +1,55 @@
<?php declare(strict_types=1);
/**
* Banker
*
* A Caching library implementing psr/cache (PSR 6) and psr/simple-cache (PSR 16)
*
* PHP version 7.4
*
* @package Banker
* @author Timothy J. Warren <tim@timshomepage.net>
* @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');
}
}
}

View File

@ -22,6 +22,8 @@ use Aviat\Banker\Exception\InvalidArgumentException;
* Private trait for shared driver-related functionality * Private trait for shared driver-related functionality
*/ */
trait _Driver { trait _Driver {
use KeyValidateTrait;
/** /**
* Driver class for handling the chosen caching backend * Driver class for handling the chosen caching backend
* *
@ -45,40 +47,4 @@ trait _Driver {
return new $class($driverConfig['connection'], $driverConfig['options']); 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');
}
}
} }

View File

@ -16,6 +16,7 @@
namespace Aviat\Banker\Tests\Driver; namespace Aviat\Banker\Tests\Driver;
use Aviat\Banker\Driver\DriverInterface; use Aviat\Banker\Driver\DriverInterface;
use Aviat\Banker\Exception\InvalidArgumentException;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
class DriverTestBase extends TestCase { class DriverTestBase extends TestCase {
@ -90,6 +91,17 @@ class DriverTestBase extends TestCase {
$this->assertEquals($data, $this->driver->getMultiple(array_keys($data))); $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 public function testSetMultipleExpires(): void
{ {
$data = [ $data = [
@ -114,6 +126,12 @@ class DriverTestBase extends TestCase {
$this->assertEquals('bar', $this->driver->get('foo')); $this->assertEquals('bar', $this->driver->get('foo'));
} }
public function testSetInvalidKey(): void
{
$this->expectException(\TypeError::class);
$this->driver->set(0x12, 'foo');
}
public function testDelete(): void public function testDelete(): void
{ {
$this->driver->set('a1', 'b2'); $this->driver->set('a1', 'b2');