From c9af5e56a43c8ad19d73830f2bc2fe97c34f4b9d Mon Sep 17 00:00:00 2001 From: Scott Jackson Date: Wed, 21 Sep 2016 09:14:12 -0700 Subject: [PATCH 1/4] Fix redis:// being rejected and username:password passing invalid auth to Redis --- src/Factory.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Factory.php b/src/Factory.php index af4c199..4ac9515 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -98,7 +98,8 @@ private function parseUrl($target) } $parts = parse_url($target); - if ($parts === false || !isset($parts['host']) || $parts['scheme'] !== 'tcp') { + $validSchemes = array('redis', 'tcp'); + if ($parts === false || !isset($parts['host']) || !in_array($parts['scheme'], $validSchemes)) { throw new InvalidArgumentException('Given URL can not be parsed'); } @@ -110,16 +111,14 @@ private function parseUrl($target) $parts['host'] = '127.0.0.1'; } - $auth = null; - if (isset($parts['user'])) { - $auth = $parts['user']; - } + // username:password@ (Redis doesn't support usernames) if (isset($parts['pass'])) { - $auth .= ':' . $parts['pass']; - } - if ($auth !== null) { - $parts['auth'] = $auth; - } + $parts['auth'] = $parts['pass']; + } + // password@ + else if (isset($parts['user'])) { + $parts['auth'] = $parts['user']; + } if (isset($parts['path']) && $parts['path'] !== '') { // skip first slash From 4b2db02e6d1d75648bc0327bec48fa9b4de904bf Mon Sep 17 00:00:00 2001 From: Scott Jackson Date: Wed, 21 Sep 2016 09:23:49 -0700 Subject: [PATCH 2/4] Update tests to reflect valid Redis schemas --- tests/FactoryTest.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/FactoryTest.php b/tests/FactoryTest.php index a1c029c..495eccf 100644 --- a/tests/FactoryTest.php +++ b/tests/FactoryTest.php @@ -64,11 +64,20 @@ public function testWillWriteSelectCommandIfTargetContainsPath() public function testWillWriteAuthCommandIfTargetContainsUserInfo() { $stream = $this->getMockBuilder('React\Stream\Stream')->disableOriginalConstructor()->getMock(); - $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$11\r\nhello:world\r\n"); + $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$11\r\nworld\r\n"); $this->connector->expects($this->once())->method('create')->willReturn(Promise\resolve($stream)); $this->factory->createClient('tcp://hello:world@127.0.0.1'); } + + public function testWillWriteAuthCommandIfTargetContainsPasswordAsUser() + { + $stream = $this->getMockBuilder('React\Stream\Stream')->disableOriginalConstructor()->getMock(); + $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$11\r\nworld\r\n"); + + $this->connector->expects($this->once())->method('create')->willReturn(Promise\resolve($stream)); + $this->factory->createClient('tcp://world@127.0.0.1'); + } public function testWillRejectIfConnectorRejects() { From 3777aed4a934dcd7ddb639dcf789e2210cf51cc4 Mon Sep 17 00:00:00 2001 From: Scott Jackson Date: Wed, 21 Sep 2016 09:32:06 -0700 Subject: [PATCH 3/4] Fix expected $5 in two tests Fix last commit not changing the length of the string in expected response --- tests/FactoryTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/FactoryTest.php b/tests/FactoryTest.php index 495eccf..a24cdd5 100644 --- a/tests/FactoryTest.php +++ b/tests/FactoryTest.php @@ -64,7 +64,7 @@ public function testWillWriteSelectCommandIfTargetContainsPath() public function testWillWriteAuthCommandIfTargetContainsUserInfo() { $stream = $this->getMockBuilder('React\Stream\Stream')->disableOriginalConstructor()->getMock(); - $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$11\r\nworld\r\n"); + $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n"); $this->connector->expects($this->once())->method('create')->willReturn(Promise\resolve($stream)); $this->factory->createClient('tcp://hello:world@127.0.0.1'); @@ -73,7 +73,7 @@ public function testWillWriteAuthCommandIfTargetContainsUserInfo() public function testWillWriteAuthCommandIfTargetContainsPasswordAsUser() { $stream = $this->getMockBuilder('React\Stream\Stream')->disableOriginalConstructor()->getMock(); - $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$11\r\nworld\r\n"); + $stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n"); $this->connector->expects($this->once())->method('create')->willReturn(Promise\resolve($stream)); $this->factory->createClient('tcp://world@127.0.0.1'); From 841d8ce21528fe7cf4fa059519d5a9c7556ccc40 Mon Sep 17 00:00:00 2001 From: Scott Jackson Date: Mon, 17 Apr 2017 09:20:52 -0700 Subject: [PATCH 4/4] Fix code style --- src/Factory.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Factory.php b/src/Factory.php index 4ac9515..e6d595e 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -111,14 +111,13 @@ private function parseUrl($target) $parts['host'] = '127.0.0.1'; } - // username:password@ (Redis doesn't support usernames) if (isset($parts['pass'])) { - $parts['auth'] = $parts['pass']; - } - // password@ - else if (isset($parts['user'])) { - $parts['auth'] = $parts['user']; - } + // username:password@ (Redis doesn't support usernames) + $parts['auth'] = $parts['pass']; + } elseif (isset($parts['user'])) { + // password@ + $parts['auth'] = $parts['user']; + } if (isset($parts['path']) && $parts['path'] !== '') { // skip first slash