Skip to content

Commit 299dd93

Browse files
clueclue
committed
Make RedisClient constructor throw if given $uri is invalid
1 parent cc0c50a commit 299dd93

File tree

5 files changed

+120
-44
lines changed

5 files changed

+120
-44
lines changed

Diff for: ‎README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,10 @@ will not have to wait for an actual underlying connection.
319319

320320
#### __construct()
321321

322-
The `new RedisClient(string $url, ConnectorInterface $connector = null)` constructor can be used to
322+
The `new RedisClient(string $uri, ConnectorInterface $connector = null)` constructor can be used to
323323
create a new `RedisClient` instance.
324324

325-
The `$url` can be given in the
325+
The `$uri` can be given in the
326326
[standard](https://www.iana.org/assignments/uri-schemes/prov/redis) form
327327
`[redis[s]://][:auth@]host[:port][/db]`.
328328
You can omit the URI scheme and port if you're connecting to the default port 6379:

Diff for: ‎src/Io/Factory.php

+3-11
Original file line numberDiff line numberDiff line change
@@ -43,23 +43,15 @@ public function __construct(?ConnectorInterface $connector = null, ?ProtocolFact
4343
public function createClient(string $uri): PromiseInterface
4444
{
4545
// support `redis+unix://` scheme for Unix domain socket (UDS) paths
46-
if (preg_match('/^(redis\+unix:\/\/(?:[^:]*:[^@]*@)?)(.+?)?$/', $uri, $match)) {
46+
if (preg_match('/^(redis\+unix:\/\/(?:[^@]*@)?)(.+)$/', $uri, $match)) {
4747
$parts = parse_url($match[1] . 'localhost/' . $match[2]);
4848
} else {
49-
if (strpos($uri, '://') === false) {
50-
$uri = 'redis://' . $uri;
51-
}
52-
5349
$parts = parse_url($uri);
5450
}
5551

5652
$uri = preg_replace(['/(:)[^:\/]*(@)/', '/([?&]password=).*?($|&)/'], '$1***$2', $uri);
57-
if ($parts === false || !isset($parts['scheme'], $parts['host']) || !in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix'])) {
58-
return reject(new \InvalidArgumentException(
59-
'Invalid Redis URI given (EINVAL)',
60-
defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22
61-
));
62-
}
53+
assert(is_array($parts) && isset($parts['scheme'], $parts['host']));
54+
assert(in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix']));
6355

6456
$args = [];
6557
parse_str($parts['query'] ?? '', $args);

Diff for: ‎src/RedisClient.php

+29-5
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
class RedisClient extends EventEmitter
2929
{
3030
/** @var string */
31-
private $target;
31+
private $uri;
3232

3333
/** @var Factory */
3434
private $factory;
@@ -54,15 +54,39 @@ class RedisClient extends EventEmitter
5454
/** @var array<string,bool> */
5555
private $psubscribed = [];
5656

57-
public function __construct(string $url, ?ConnectorInterface $connector = null)
57+
/**
58+
* @param string $uri
59+
* @param ?ConnectorInterface $connector
60+
* @throws \InvalidArgumentException if $uri is not a valid Redis URI
61+
*/
62+
public function __construct(string $uri, ?ConnectorInterface $connector = null)
5863
{
64+
// support `redis+unix://` scheme for Unix domain socket (UDS) paths
65+
if (preg_match('/^(redis\+unix:\/\/(?:[^@]*@)?)(.+)$/', $uri, $match)) {
66+
$parts = parse_url($match[1] . 'localhost/' . $match[2]);
67+
} else {
68+
if (strpos($uri, '://') === false) {
69+
$uri = 'redis://' . $uri;
70+
}
71+
72+
$parts = parse_url($uri);
73+
}
74+
75+
$uri = (string) preg_replace(['/(:)[^:\/]*(@)/', '/([?&]password=).*?($|&)/'], '$1***$2', $uri);
76+
if ($parts === false || !isset($parts['scheme'], $parts['host']) || !in_array($parts['scheme'], ['redis', 'rediss', 'redis+unix'])) {
77+
throw new \InvalidArgumentException(
78+
'Invalid Redis URI "' . $uri . '" (EINVAL)',
79+
defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22
80+
);
81+
}
82+
5983
$args = [];
60-
\parse_str((string) \parse_url($url, \PHP_URL_QUERY), $args);
84+
\parse_str($parts['query'] ?? '', $args);
6185
if (isset($args['idle'])) {
6286
$this->idlePeriod = (float)$args['idle'];
6387
}
6488

65-
$this->target = $url;
89+
$this->uri = $uri;
6690
$this->factory = new Factory($connector);
6791
}
6892

@@ -75,7 +99,7 @@ private function client(): PromiseInterface
7599
return $this->promise;
76100
}
77101

78-
return $this->promise = $this->factory->createClient($this->target)->then(function (StreamingClient $redis) {
102+
return $this->promise = $this->factory->createClient($this->uri)->then(function (StreamingClient $redis) {
79103
// connection completed => remember only until closed
80104
$redis->on('close', function () {
81105
$this->promise = null;

Diff for: ‎tests/Io/FactoryStreamingClientTest.php

+5-26
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ public function testCtor(): void
5555
public function testWillConnectWithDefaultPort(): void
5656
{
5757
$this->connector->expects($this->once())->method('connect')->with('redis.example.com:6379')->willReturn(reject(new \RuntimeException()));
58-
$promise = $this->factory->createClient('redis.example.com');
58+
$promise = $this->factory->createClient('redis://redis.example.com');
5959

6060
$promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection
6161
}
6262

6363
public function testWillConnectToLocalhost(): void
6464
{
6565
$this->connector->expects($this->once())->method('connect')->with('localhost:1337')->willReturn(reject(new \RuntimeException()));
66-
$promise = $this->factory->createClient('localhost:1337');
66+
$promise = $this->factory->createClient('redis://localhost:1337');
6767

6868
$promise->then(null, $this->expectCallableOnce()); // avoid reporting unhandled rejection
6969
}
@@ -74,7 +74,7 @@ public function testWillResolveIfConnectorResolves(): void
7474
$stream->expects($this->never())->method('write');
7575

7676
$this->connector->expects($this->once())->method('connect')->willReturn(resolve($stream));
77-
$promise = $this->factory->createClient('localhost');
77+
$promise = $this->factory->createClient('redis://localhost');
7878

7979
$this->expectPromiseResolve($promise);
8080
}
@@ -483,23 +483,6 @@ public function testWillRejectIfConnectorRejects(): void
483483
));
484484
}
485485

486-
public function testWillRejectIfTargetIsInvalid(): void
487-
{
488-
$promise = $this->factory->createClient('http://invalid target');
489-
490-
$promise->then(null, $this->expectCallableOnceWith(
491-
$this->logicalAnd(
492-
$this->isInstanceOf(\InvalidArgumentException::class),
493-
$this->callback(function (\InvalidArgumentException $e) {
494-
return $e->getMessage() === 'Invalid Redis URI given (EINVAL)';
495-
}),
496-
$this->callback(function (\InvalidArgumentException $e) {
497-
return $e->getCode() === (defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22);
498-
})
499-
)
500-
));
501-
}
502-
503486
public function testCancelWillRejectPromise(): void
504487
{
505488
$promise = new \React\Promise\Promise(function () { });
@@ -516,10 +499,6 @@ public function testCancelWillRejectPromise(): void
516499
public function provideUris(): array
517500
{
518501
return [
519-
[
520-
'localhost',
521-
'redis://localhost'
522-
],
523502
[
524503
'redis://localhost',
525504
'redis://localhost'
@@ -705,7 +684,7 @@ public function testCreateClientWillCancelTimerWhenConnectionResolves(): void
705684
$deferred = new Deferred();
706685
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise());
707686

708-
$promise = $this->factory->createClient('127.0.0.1');
687+
$promise = $this->factory->createClient('redis://127.0.0.1');
709688
$promise->then($this->expectCallableOnce());
710689

711690
$deferred->resolve($this->createMock(ConnectionInterface::class));
@@ -723,7 +702,7 @@ public function testCreateClientWillCancelTimerWhenConnectionRejects(): void
723702
$deferred = new Deferred();
724703
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise());
725704

726-
$promise = $this->factory->createClient('127.0.0.1');
705+
$promise = $this->factory->createClient('redis://127.0.0.1');
727706

728707
$promise->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
729708

Diff for: ‎tests/RedisClientTest.php

+81
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,67 @@ public function setUp(): void
4444
$ref->setValue($this->redis, $this->factory);
4545
}
4646

47+
public static function provideInvalidUris(): \Generator
48+
{
49+
yield [
50+
'',
51+
'redis://'
52+
];
53+
yield [
54+
'localhost:100000',
55+
'redis://localhost:100000'
56+
];
57+
yield [
58+
'tcp://localhost',
59+
'tcp://localhost'
60+
];
61+
yield [
62+
'redis://',
63+
'redis://'
64+
];
65+
yield [
66+
'redis+unix://',
67+
'redis+unix://'
68+
];
69+
yield [
70+
'user@localhost:100000',
71+
'redis://user@localhost:100000'
72+
];
73+
yield [
74+
':pass@localhost:100000',
75+
'redis://:***@localhost:100000'
76+
];
77+
yield [
78+
'user:@localhost:100000',
79+
'redis://user:***@localhost:100000'
80+
];
81+
yield [
82+
'user:pass@localhost:100000',
83+
'redis://user:***@localhost:100000'
84+
];
85+
yield [
86+
'localhost:100000?password=secret',
87+
'redis://localhost:100000?password=***'
88+
];
89+
yield [
90+
'user@',
91+
'redis://user@'
92+
];
93+
yield [
94+
'user:pass@',
95+
'redis://user:***@'
96+
];
97+
}
98+
99+
/** @dataProvider provideInvalidUris() */
100+
public function testCtorWithInvalidUriThrows(string $uri, string $message): void
101+
{
102+
$this->expectException(\InvalidArgumentException::class);
103+
$this->expectExceptionMessage('Invalid Redis URI "' . $message . '" (EINVAL)');
104+
$this->expectExceptionCode(defined('SOCKET_EINVAL') ? SOCKET_EINVAL : 22);
105+
new RedisClient($uri);
106+
}
107+
47108
public function testPingWillCreateUnderlyingClientAndReturnPendingPromise(): void
48109
{
49110
$promise = new Promise(function () { });
@@ -59,6 +120,26 @@ public function testPingWillCreateUnderlyingClientAndReturnPendingPromise(): voi
59120
$promise->then($this->expectCallableNever());
60121
}
61122

123+
public function testPingWithUnixUriWillCreateUnderlyingClientAndReturnPendingPromise(): void
124+
{
125+
$this->redis = new RedisClient('redis+unix:///tmp/redis.sock');
126+
$ref = new \ReflectionProperty($this->redis, 'factory');
127+
$ref->setAccessible(true);
128+
$ref->setValue($this->redis, $this->factory);
129+
130+
$promise = new Promise(function () { });
131+
$this->factory->expects($this->once())->method('createClient')->with('redis+unix:///tmp/redis.sock')->willReturn($promise);
132+
133+
$loop = $this->createMock(LoopInterface::class);
134+
$loop->expects($this->never())->method('addTimer');
135+
assert($loop instanceof LoopInterface);
136+
Loop::set($loop);
137+
138+
$promise = $this->redis->ping();
139+
140+
$promise->then($this->expectCallableNever());
141+
}
142+
62143
public function testPingTwiceWillCreateOnceUnderlyingClient(): void
63144
{
64145
$promise = new Promise(function () { });

0 commit comments

Comments
 (0)