From 15426bdcb905511915ec2784448e32f08375d45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Mon, 18 May 2020 17:15:50 +0200 Subject: [PATCH] Minor code cleanup, remove dead code and improve code coverage to 100% --- src/Connection.php | 7 +++---- src/StreamEncryption.php | 15 ++++----------- src/TcpConnector.php | 2 ++ src/TcpServer.php | 3 +-- tests/ServerTest.php | 17 +++++++++++++++++ tests/TcpConnectorTest.php | 32 ++++++++++++++++++++++++++++++++ tests/TcpServerTest.php | 17 +++++++++++++++++ tests/UnixServerTest.php | 37 +++++++++++++++++++++++++++++++++++++ 8 files changed, 113 insertions(+), 17 deletions(-) diff --git a/src/Connection.php b/src/Connection.php index 5da20c95..60febcba 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -156,13 +156,13 @@ private function parseAddress($address) // remove trailing colon from address for HHVM < 3.19: https://3v4l.org/5C1lo // note that technically ":" is a valid address, so keep this in place otherwise if (\substr($address, -1) === ':' && \defined('HHVM_VERSION_ID') && \HHVM_VERSION_ID < 31900) { - $address = (string)\substr($address, 0, -1); + $address = (string)\substr($address, 0, -1); // @codeCoverageIgnore } // work around unknown addresses should return null value: https://3v4l.org/5C1lo and https://bugs.php.net/bug.php?id=74556 // PHP uses "\0" string and HHVM uses empty string (colon removed above) if ($address === '' || $address[0] === "\x00" ) { - return null; + return null; // @codeCoverageIgnore } return 'unix://' . $address; @@ -171,8 +171,7 @@ private function parseAddress($address) // check if this is an IPv6 address which includes multiple colons but no square brackets $pos = \strrpos($address, ':'); if ($pos !== false && \strpos($address, ':') < $pos && \substr($address, 0, 1) !== '[') { - $port = \substr($address, $pos + 1); - $address = '[' . \substr($address, 0, $pos) . ']:' . $port; + $address = '[' . \substr($address, 0, $pos) . ']:' . \substr($address, $pos + 1); // @codeCoverageIgnore } return ($this->encryptionEnabled ? 'tls' : 'tcp') . '://' . $address; diff --git a/src/StreamEncryption.php b/src/StreamEncryption.php index 0e0f3d6e..61777640 100644 --- a/src/StreamEncryption.php +++ b/src/StreamEncryption.php @@ -33,13 +33,13 @@ public function __construct(LoopInterface $loop, $server = true) $this->method = \STREAM_CRYPTO_METHOD_TLS_SERVER; if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) { - $this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER; + $this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER; // @codeCoverageIgnore } } else { $this->method = \STREAM_CRYPTO_METHOD_TLS_CLIENT; if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) { - $this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT; + $this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT; // @codeCoverageIgnore } } } @@ -49,11 +49,6 @@ public function enable(Connection $stream) return $this->toggle($stream, true); } - public function disable(Connection $stream) - { - return $this->toggle($stream, false); - } - public function toggle(Connection $stream, $toggle) { // pause actual stream instance to continue operation on raw stream socket @@ -61,10 +56,8 @@ public function toggle(Connection $stream, $toggle) // TODO: add write() event to make sure we're not sending any excessive data - $deferred = new Deferred(function ($_, $reject) use ($toggle) { - // cancelling this leaves this stream in an inconsistent state… - $reject(new \RuntimeException('Cancelled toggling encryption ' . $toggle ? 'on' : 'off')); - }); + // cancelling this leaves this stream in an inconsistent state… + $deferred = new Deferred(); // get actual stream socket from stream instance $socket = $stream->stream; diff --git a/src/TcpConnector.php b/src/TcpConnector.php index 961ae264..d09e9901 100644 --- a/src/TcpConnector.php +++ b/src/TcpConnector.php @@ -59,12 +59,14 @@ public function connect($uri) // Legacy PHP < 5.6 ignores peer_name and requires legacy context options instead. // The SNI_server_name context option has to be set here during construction, // as legacy PHP ignores any values set later. + // @codeCoverageIgnoreStart if (\PHP_VERSION_ID < 50600) { $context['ssl'] += array( 'SNI_server_name' => $args['hostname'], 'CN_match' => $args['hostname'] ); } + // @codeCoverageIgnoreEnd } // latest versions of PHP no longer accept any other URI components and diff --git a/src/TcpServer.php b/src/TcpServer.php index 4b54a815..8f074323 100644 --- a/src/TcpServer.php +++ b/src/TcpServer.php @@ -179,8 +179,7 @@ public function getAddress() // check if this is an IPv6 address which includes multiple colons but no square brackets $pos = \strrpos($address, ':'); if ($pos !== false && \strpos($address, ':') < $pos && \substr($address, 0, 1) !== '[') { - $port = \substr($address, $pos + 1); - $address = '[' . \substr($address, 0, $pos) . ']:' . $port; + $address = '[' . \substr($address, 0, $pos) . ']:' . \substr($address, $pos + 1); // @codeCoverageIgnore } return 'tcp://' . $address; diff --git a/tests/ServerTest.php b/tests/ServerTest.php index c1d3bc92..be96d075 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -91,6 +91,23 @@ public function testConstructorThrowsForExistingUnixPath() } } + public function testEmitsErrorWhenUnderlyingTcpServerEmitsError() + { + $loop = Factory::create(); + + $server = new Server(0, $loop); + + $ref = new \ReflectionProperty($server, 'server'); + $ref->setAccessible(true); + $tcp = $ref->getvalue($server); + + $error = new \RuntimeException(); + $server->on('error', $this->expectCallableOnceWith($error)); + $tcp->emit('error', array($error)); + + $server->close(); + } + public function testEmitsConnectionForNewConnection() { $loop = Factory::create(); diff --git a/tests/TcpConnectorTest.php b/tests/TcpConnectorTest.php index dd10f9af..60b97536 100644 --- a/tests/TcpConnectorTest.php +++ b/tests/TcpConnectorTest.php @@ -7,6 +7,7 @@ use React\Socket\ConnectionInterface; use React\Socket\TcpConnector; use React\Socket\TcpServer; +use React\Promise\Promise; class TcpConnectorTest extends TestCase { @@ -63,6 +64,37 @@ public function connectionToTcpServerShouldSucceed() $connection->close(); } + /** + * @test + * @expectedException RuntimeException + */ + public function connectionToTcpServerShouldFailIfFileDescriptorsAreExceeded() + { + $loop = Factory::create(); + + $connector = new TcpConnector($loop); + + $ulimit = exec('ulimit -n 2>&1'); + if ($ulimit < 1) { + $this->markTestSkipped('Unable to determine limit of open files (ulimit not available?)'); + } + + // dummy rejected promise to make sure autoloader has initialized all classes + $foo = new Promise(function () { throw new \RuntimeException('dummy'); }); + + // keep creating dummy file handles until all file descriptors are exhausted + $fds = array(); + for ($i = 0; $i < $ulimit; ++$i) { + $fd = @fopen('/dev/null', 'r'); + if ($fd === false) { + break; + } + $fds[] = $fd; + } + + Block\await($connector->connect('127.0.0.1:9999'), $loop, self::TIMEOUT); + } + /** @test */ public function connectionToTcpServerShouldSucceedWithRemoteAdressSameAsTarget() { diff --git a/tests/TcpServerTest.php b/tests/TcpServerTest.php index 72b3c28d..ed07f218 100644 --- a/tests/TcpServerTest.php +++ b/tests/TcpServerTest.php @@ -256,6 +256,23 @@ public function testCloseRemovesResourceFromLoop() $server->close(); } + public function testEmitsErrorWhenAcceptListenerFails() + { + $listener = null; + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) { + $listener = $cb; + return true; + })); + + $server = new TcpServer(0, $loop); + + $server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'))); + + $this->assertNotNull($listener); + $listener(false); + } + /** * @expectedException RuntimeException */ diff --git a/tests/UnixServerTest.php b/tests/UnixServerTest.php index 70186fa3..e0eee93b 100644 --- a/tests/UnixServerTest.php +++ b/tests/UnixServerTest.php @@ -216,6 +216,26 @@ public function testCtorAddsResourceToLoop() $server = new UnixServer($this->getRandomSocketUri(), $loop); } + /** + * @expectedException InvalidArgumentException + */ + public function testCtorThrowsForInvalidAddressScheme() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $server = new UnixServer('tcp://localhost:0', $loop); + } + + /** + * @expectedException RuntimeException + */ + public function testCtorThrowsWhenPathIsNotWritable() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + + $server = new UnixServer('/dev/null', $loop); + } + public function testResumeWithoutPauseIsNoOp() { $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); @@ -253,6 +273,23 @@ public function testCloseRemovesResourceFromLoop() $server->close(); } + public function testEmitsErrorWhenAcceptListenerFails() + { + $listener = null; + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) { + $listener = $cb; + return true; + })); + + $server = new UnixServer($this->getRandomSocketUri(), $loop); + + $server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'))); + + $this->assertNotNull($listener); + $listener(false); + } + /** * @expectedException RuntimeException */