Filter NULL and zero-length string values for parameters.

Having NULL values or zero-length strings for connection parameters does
not make much sense and actually it proved to be an issue with certain
parameters like "password" where an empty string would trigger an AUTH
command with an empty password (and obviously Redis was not happy with
that). The main offenders were a few libraries and frameworks that kept
passing empty values for parameters such as "database" and "password"
even when users left them unconfigured. This fix should make things more
robust and avoid such occurrences in the future.

Related to PR #436 (rejected).
This commit is contained in:
Daniele Alessandri
2020-08-25 12:20:53 +02:00
parent 78041126fc
commit b014d5de5a
3 changed files with 34 additions and 25 deletions
+9 -7
View File
@@ -20,9 +20,7 @@ namespace Predis\Connection;
*/
class Parameters implements ParametersInterface
{
private $parameters;
private static $defaults = array(
protected static $defaults = array(
'scheme' => 'tcp',
'host' => '127.0.0.1',
'port' => 6379,
@@ -33,17 +31,21 @@ class Parameters implements ParametersInterface
*/
public function __construct(array $parameters = array())
{
$this->parameters = $parameters + $this->getDefaults();
$this->parameters = $this->filter($parameters + static::$defaults);
}
/**
* Returns some default parameters with their values.
* Filters parameters removing entries with NULL or 0-length string values.
*
* @params array $parameters Array of parameters to be filtered
*
* @return array
*/
protected function getDefaults()
protected function filter(array $parameters)
{
return self::$defaults;
return array_filter($parameters, function ($value) {
return $value !== null && $value !== '';
});
}
/**
+22 -10
View File
@@ -39,23 +39,29 @@ class ParametersTest extends PredisTestCase
{
$parameters = new Parameters();
$this->assertTrue(isset($parameters->scheme));
$this->assertFalse(isset($parameters->unknown));
$this->assertTrue(isset($parameters->scheme), 'Parameter `scheme` was expected to be set');
$this->assertFalse(isset($parameters->notset), 'Parameter `notset` was expected to be not set');
}
public function sharedTestsWithArrayParameters(Parameters $parameters)
{
$this->assertTrue(isset($parameters->scheme));
$this->assertSame('tcp', $parameters->scheme);
$this->assertTrue(isset($parameters->scheme), 'Parameter `scheme` was expected to be set');
$this->assertSame('tcp', $parameters->scheme, 'Parameter `scheme` was expected to be set');
$this->assertTrue(isset($parameters->port));
$this->assertSame(7000, $parameters->port);
$this->assertTrue(isset($parameters->port), 'Parameter `port` was expected to be set');
$this->assertSame(7000, $parameters->port, 'Parameter `port` was expected to return 7000');
$this->assertTrue(isset($parameters->custom));
$this->assertSame('foobar', $parameters->custom);
$this->assertTrue(isset($parameters->custom), 'Parameter `custom` was expected to be set');
$this->assertSame('foobar', $parameters->custom, 'Parameter `custom` was expected to return "foobar"');
$this->assertFalse(isset($parameters->unknown));
$this->assertNull($parameters->unknown);
$this->assertFalse(isset($parameters->setnull), 'Parameter `setnull` was expected to be not set');
$this->assertNull($parameters->setnull, 'Parameter `setnull` was expected to return NULL');
$this->assertFalse(isset($parameters->setemptystring), 'Parameter `setemptystring` was expected to be not set');
$this->assertNull($parameters->setemptystring, 'Parameter `setemptystring` was expected to return NULL');
$this->assertFalse(isset($parameters->notset), 'Parameter `notset` was expected to be not set');
$this->assertNull($parameters->notset, 'Parameter `notset` was expected to return NULL');
}
/**
@@ -66,6 +72,8 @@ class ParametersTest extends PredisTestCase
$parameters = new Parameters(array(
'port' => 7000,
'custom' => 'foobar',
'setnull' => null,
'setemptystring' => '',
));
$this->sharedTestsWithArrayParameters($parameters);
@@ -79,6 +87,8 @@ class ParametersTest extends PredisTestCase
$parameters = new Parameters(array(
'port' => 7000,
'custom' => 'foobar',
'setnull' => null,
'setemptystring' => '',
));
$this->sharedTestsWithArrayParameters($parameters);
@@ -93,6 +103,8 @@ class ParametersTest extends PredisTestCase
'port' => 7000,
'database' => 5,
'custom' => 'foobar',
'setnull' => null,
'setemptystring' => '',
);
$uriString = $this->getParametersString($overrides);
@@ -37,7 +37,7 @@ class SentinelReplicationTest extends PredisTestCase
/**
* @group disconnected
*/
public function testParametersForSentinelConnectionShouldNotUseDatabaseAndPassword()
public function testParametersForSentinelConnectionShouldIgnoreDatabaseAndPassword()
{
$replication = $this->getReplicationConnection('svc', array(
'tcp://127.0.0.1:5381?role=sentinel&database=1&password=secret',
@@ -45,13 +45,8 @@ class SentinelReplicationTest extends PredisTestCase
$parameters = $replication->getSentinelConnection()->getParameters()->toArray();
$this->assertIsArray($parameters);
$this->assertArrayHasKey('database', $parameters, 'Missing expected `database` in connection parameters');
$this->assertNull($parameters['database'], 'The `database` parameter is expected to be NULL for sentinels');
$this->assertArrayHasKey('password', $parameters, 'Missing expected `password` in connection parameters');
$this->assertNull($parameters['password'], 'The `password` parameter is expected to be NULL for sentinels');
$this->assertArrayNotHasKey('database', $parameters, 'Parameter `database` was expected to not exist in connection parameters');
$this->assertArrayNotHasKey('password', $parameters, 'Parameter `password` was expected to not exist in connection parameters');
}
/**