From 299dd93d790ba837e1fd1071fc6eb71c47a33533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 27 Dec 2024 16:34:40 +0100 Subject: [PATCH] Make `RedisClient` constructor throw if given `$uri` is invalid --- README.md | 4 +- src/Io/Factory.php | 14 +---- src/RedisClient.php | 34 +++++++++-- tests/Io/FactoryStreamingClientTest.php | 31 ++-------- tests/RedisClientTest.php | 81 +++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index bc30f7b..b04b423 100644 --- a/README.md +++ b/README.md @@ -319,10 +319,10 @@ will not have to wait for an actual underlying connection. #### __construct() -The `new RedisClient(string $url, ConnectorInterface $connector = null)` constructor can be used to +The `new RedisClient(string $uri, ConnectorInterface $connector = null)` constructor can be used to create a new `RedisClient` instance. -The `$url` can be given in the +The `$uri` can be given in the [standard](https://www.iana.org/assignments/uri-schemes/prov/redis) form `[redis[s]://][:auth@]host[:port][/db]`. You can omit the URI scheme and port if you're connecting to the default port 6379: diff --git a/src/Io/Factory.php b/src/Io/Factory.php index d920fd6..b66e30d 100644 --- a/src/Io/Factory.php +++ b/src/Io/Factory.php @@ -43,23 +43,15 @@ public function __construct(?ConnectorInterface $connector = null, ?ProtocolFact public function createClient(string $uri): PromiseInterface { // support `redis+unix://` scheme for Unix domain socket (UDS) paths - if (preg_match('/^(redis\+unix:\/\/(?:[^:]*:[^@]*@)?)(.+?)?$/', $uri, $match)) { + if (preg_match('/^(redis\+unix:\/\/(?:[^@]*@)?)(.+)$/', $uri, $match)) { $parts = parse_url($match[1] . 'localhost/' . $match[2]); } else { - if (strpos($uri, '://') === false) { - $uri = 'redis://' . $uri; - } - $parts = parse_url($uri); } $uri = preg_replace(['/(:)[^:\/]*(@)/', '/([?&]password=).*?($|&)/'], '$1***$2', $uri); - if ($parts === false || !isset($parts['scheme'], $parts['host']) || !in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix'])) { - return reject(new \InvalidArgumentException( - 'Invalid Redis URI given (EINVAL)', - defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22 - )); - } + assert(is_array($parts) && isset($parts['scheme'], $parts['host'])); + assert(in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix'])); $args = []; parse_str($parts['query'] ?? '', $args); diff --git a/src/RedisClient.php b/src/RedisClient.php index 8f984b0..df4ebf5 100644 --- a/src/RedisClient.php +++ b/src/RedisClient.php @@ -28,7 +28,7 @@ class RedisClient extends EventEmitter { /** @var string */ - private $target; + private $uri; /** @var Factory */ private $factory; @@ -54,15 +54,39 @@ class RedisClient extends EventEmitter /** @var array */ private $psubscribed = []; - public function __construct(string $url, ?ConnectorInterface $connector = null) + /** + * @param string $uri + * @param ?ConnectorInterface $connector + * @throws \InvalidArgumentException if $uri is not a valid Redis URI + */ + public function __construct(string $uri, ?ConnectorInterface $connector = null) { + // support `redis+unix://` scheme for Unix domain socket (UDS) paths + if (preg_match('/^(redis\+unix:\/\/(?:[^@]*@)?)(.+)$/', $uri, $match)) { + $parts = parse_url($match[1] . 'localhost/' . $match[2]); + } else { + if (strpos($uri, '://') === false) { + $uri = 'redis://' . $uri; + } + + $parts = parse_url($uri); + } + + $uri = (string) preg_replace(['/(:)[^:\/]*(@)/', '/([?&]password=).*?($|&)/'], '$1***$2', $uri); + if ($parts === false || !isset($parts['scheme'], $parts['host']) || !in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix'])) { + throw new \InvalidArgumentException( + 'Invalid Redis URI "' . $uri . '" (EINVAL)', + defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22 + ); + } + $args = []; - \parse_str((string) \parse_url($url, \PHP_URL_QUERY), $args); + \parse_str($parts['query'] ?? '', $args); if (isset($args['idle'])) { $this->idlePeriod = (float)$args['idle']; } - $this->target = $url; + $this->uri = $uri; $this->factory = new Factory($connector); } @@ -75,7 +99,7 @@ private function client(): PromiseInterface return $this->promise; } - return $this->promise = $this->factory->createClient($this->target)->then(function (StreamingClient $redis) { + return $this->promise = $this->factory->createClient($this->uri)->then(function (StreamingClient $redis) { // connection completed => remember only until closed $redis->on('close', function () { $this->promise = null; diff --git a/tests/Io/FactoryStreamingClientTest.php b/tests/Io/FactoryStreamingClientTest.php index fbc274a..4a5ee19 100644 --- a/tests/Io/FactoryStreamingClientTest.php +++ b/tests/Io/FactoryStreamingClientTest.php @@ -55,7 +55,7 @@ public function testCtor(): void public function testWillConnectWithDefaultPort(): void { $this->connector->expects($this->once())->method('connect')->with('redis.example.com:6379')->willReturn(reject(new \RuntimeException())); - $promise = $this->factory->createClient('redis.example.com'); + $promise = $this->factory->createClient('redis://redis.example.com'); $promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection } @@ -63,7 +63,7 @@ public function testWillConnectWithDefaultPort(): void public function testWillConnectToLocalhost(): void { $this->connector->expects($this->once())->method('connect')->with('localhost:1337')->willReturn(reject(new \RuntimeException())); - $promise = $this->factory->createClient('localhost:1337'); + $promise = $this->factory->createClient('redis://localhost:1337'); $promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection } @@ -74,7 +74,7 @@ public function testWillResolveIfConnectorResolves(): void $stream->expects($this->never())->method('write'); $this->connector->expects($this->once())->method('connect')->willReturn(resolve($stream)); - $promise = $this->factory->createClient('localhost'); + $promise = $this->factory->createClient('redis://localhost'); $this->expectPromiseResolve($promise); } @@ -483,23 +483,6 @@ public function testWillRejectIfConnectorRejects(): void )); } - public function testWillRejectIfTargetIsInvalid(): void - { - $promise = $this->factory->createClient('http://invalid target'); - - $promise->then(null, $this->expectCallableOnceWith( - $this->logicalAnd( - $this->isInstanceOf(\InvalidArgumentException::class), - $this->callback(function (\InvalidArgumentException $e) { - return $e->getMessage() === 'Invalid Redis URI given (EINVAL)'; - }), - $this->callback(function (\InvalidArgumentException $e) { - return $e->getCode() === (defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22); - }) - ) - )); - } - public function testCancelWillRejectPromise(): void { $promise = new \React\Promise\Promise(function () { }); @@ -516,10 +499,6 @@ public function testCancelWillRejectPromise(): void public function provideUris(): array { return [ - [ - 'localhost', - 'redis://localhost' - ], [ 'redis://localhost', 'redis://localhost' @@ -705,7 +684,7 @@ public function testCreateClientWillCancelTimerWhenConnectionResolves(): void $deferred = new Deferred(); $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise()); - $promise = $this->factory->createClient('127.0.0.1'); + $promise = $this->factory->createClient('redis://127.0.0.1'); $promise->then($this->expectCallableOnce()); $deferred->resolve($this->createMock(ConnectionInterface::class)); @@ -723,7 +702,7 @@ public function testCreateClientWillCancelTimerWhenConnectionRejects(): void $deferred = new Deferred(); $this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise()); - $promise = $this->factory->createClient('127.0.0.1'); + $promise = $this->factory->createClient('redis://127.0.0.1'); $promise->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'))); diff --git a/tests/RedisClientTest.php b/tests/RedisClientTest.php index 6bfdb7e..273c655 100644 --- a/tests/RedisClientTest.php +++ b/tests/RedisClientTest.php @@ -44,6 +44,67 @@ public function setUp(): void $ref->setValue($this->redis, $this->factory); } + public static function provideInvalidUris(): \Generator + { + yield [ + '', + 'redis://' + ]; + yield [ + 'localhost:100000', + 'redis://localhost:100000' + ]; + yield [ + 'tcp://localhost', + 'tcp://localhost' + ]; + yield [ + 'redis://', + 'redis://' + ]; + yield [ + 'redis+unix://', + 'redis+unix://' + ]; + yield [ + 'user@localhost:100000', + 'redis://user@localhost:100000' + ]; + yield [ + ':pass@localhost:100000', + 'redis://:***@localhost:100000' + ]; + yield [ + 'user:@localhost:100000', + 'redis://user:***@localhost:100000' + ]; + yield [ + 'user:pass@localhost:100000', + 'redis://user:***@localhost:100000' + ]; + yield [ + 'localhost:100000?password=secret', + 'redis://localhost:100000?password=***' + ]; + yield [ + 'user@', + 'redis://user@' + ]; + yield [ + 'user:pass@', + 'redis://user:***@' + ]; + } + + /** @dataProvider provideInvalidUris() */ + public function testCtorWithInvalidUriThrows(string $uri, string $message): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid Redis URI "' . $message . '" (EINVAL)'); + $this->expectExceptionCode(defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22); + new RedisClient($uri); + } + public function testPingWillCreateUnderlyingClientAndReturnPendingPromise(): void { $promise = new Promise(function () { }); @@ -59,6 +120,26 @@ public function testPingWillCreateUnderlyingClientAndReturnPendingPromise(): voi $promise->then($this->expectCallableNever()); } + public function testPingWithUnixUriWillCreateUnderlyingClientAndReturnPendingPromise(): void + { + $this->redis = new RedisClient('redis+unix:///tmp/redis.sock'); + $ref = new \ReflectionProperty($this->redis, 'factory'); + $ref->setAccessible(true); + $ref->setValue($this->redis, $this->factory); + + $promise = new Promise(function () { }); + $this->factory->expects($this->once())->method('createClient')->with('redis+unix:///tmp/redis.sock')->willReturn($promise); + + $loop = $this->createMock(LoopInterface::class); + $loop->expects($this->never())->method('addTimer'); + assert($loop instanceof LoopInterface); + Loop::set($loop); + + $promise = $this->redis->ping(); + + $promise->then($this->expectCallableNever()); + } + public function testPingTwiceWillCreateOnceUnderlyingClient(): void { $promise = new Promise(function () { });