Merge pull request #236 from clue-labs/covfefe

Minor code cleanup, remove dead code and improve code coverage to 100%
reactphp · May 21, 2020 · b3de2a0 · b3de2a0
2 parents f5049f0 + 15426bd
commit b3de2a0
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 17 deletions.
diff --git 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
@@ -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,22 +49,15 @@ 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
         $stream->pause();
 
         // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
      */