reactphp · WyriHaximus · Sep 28, 2018 · Aug 3, 2018 · Aug 3, 2018 · Aug 5, 2018
diff --git a/src/SecureConnector.php b/src/SecureConnector.php
@@ -40,8 +40,10 @@ public function connect($uri)
         $context = $this->context;
 
         $encryption = $this->streamEncryption;
-        return $this->connector->connect($uri)->then(function (ConnectionInterface $connection) use ($context, $encryption) {
+        $connected = false;
+        $promise = $this->connector->connect($uri)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) {
             // (unencrypted) TCP/IP connection succeeded
+            $connected = true;
 
             if (!$connection instanceof Connection) {
                 $connection->close();
@@ -54,11 +56,29 @@ public function connect($uri)
             }
 
             // try to enable encryption
-            return $encryption->enable($connection)->then(null, function ($error) use ($connection) {
+            return $promise = $encryption->enable($connection)->then(null, function ($error) use ($connection, $uri) {
                 // establishing encryption failed => close invalid connection and return error
                 $connection->close();
-                throw $error;
+
+                throw new \RuntimeException(
+                    'Connection to ' . $uri . ' failed during TLS handshake: ' . $error->getMessage(),
+                    $error->getCode()
+                );
             });
         });
+
+        return new \React\Promise\Promise(
+            function ($resolve, $reject) use ($promise) {
+                $promise->then($resolve, $reject);
+            },
+            function ($_, $reject) use (&$promise, $uri, &$connected) {
+                if ($connected) {
+                    $reject(new \RuntimeException('Connection to ' . $uri . ' cancelled during TLS handshake'));
+                }
+
+                $promise->cancel();
+                $promise = null;
+            }
+        );
     }
 }
diff --git a/src/SecureServer.php b/src/SecureServer.php
@@ -184,6 +184,11 @@ function ($conn) use ($that) {
                 $that->emit('connection', array($conn));
             },
             function ($error) use ($that, $connection) {
+                $error = new \RuntimeException(
+                    'Connection from ' . $connection->getRemoteAddress() . ' failed during TLS handshake: ' . $error->getMessage(),
+                    $error->getCode()
+                );
+
                 $that->emit('error', array($error));
                 $connection->end();
             }

diff --git a/src/StreamEncryption.php b/src/StreamEncryption.php
@@ -136,15 +136,20 @@ public function toggleCrypto($socket, Deferred $deferred, $toggle, $method)
         if (true === $result) {
             $deferred->resolve();
         } else if (false === $result) {
+            // overwrite callback arguments for PHP7+ only, so they do not show
+            // up in the Exception trace and do not cause a possible cyclic reference.
+            $d = $deferred;
+            $deferred = null;
+
             if (\feof($socket) || $error === null) {
                 // EOF or failed without error => connection closed during handshake
-                $deferred->reject(new UnexpectedValueException(
+                $d->reject(new UnexpectedValueException(
                     'Connection lost during TLS handshake',
                     \defined('SOCKET_ECONNRESET') ? \SOCKET_ECONNRESET : 0
                 ));
             } else {
                 // handshake failed with error message
-                $deferred->reject(new UnexpectedValueException(
+                $d->reject(new UnexpectedValueException(
                     'Unable to complete TLS handshake: ' . $error
                 ));
             }

diff --git a/tests/FunctionalSecureServerTest.php b/tests/FunctionalSecureServerTest.php
@@ -420,9 +420,12 @@ public function testEmitsErrorIfConnectionIsClosedBeforeHandshake()
 
         $error = Block\await($errorEvent, $loop, self::TIMEOUT);
 
+        // Connection from tcp://127.0.0.1:39528 failed during TLS handshake: Connection lost during TLS handshak
         $this->assertTrue($error instanceof \RuntimeException);
-        $this->assertEquals('Connection lost during TLS handshake', $error->getMessage());
+        $this->assertStringStartsWith('Connection from tcp://', $error->getMessage());
+        $this->assertStringEndsWith('failed during TLS handshake: Connection lost during TLS handshake', $error->getMessage());
         $this->assertEquals(defined('SOCKET_ECONNRESET') ? SOCKET_ECONNRESET : 0, $error->getCode());
+        $this->assertNull($error->getPrevious());
     }
 
     public function testEmitsErrorIfConnectionIsClosedWithIncompleteHandshake()
@@ -445,9 +448,12 @@ public function testEmitsErrorIfConnectionIsClosedWithIncompleteHandshake()
 
         $error = Block\await($errorEvent, $loop, self::TIMEOUT);
 
+        // Connection from tcp://127.0.0.1:39528 failed during TLS handshake: Connection lost during TLS handshak
         $this->assertTrue($error instanceof \RuntimeException);
-        $this->assertEquals('Connection lost during TLS handshake', $error->getMessage());
+        $this->assertStringStartsWith('Connection from tcp://', $error->getMessage());
+        $this->assertStringEndsWith('failed during TLS handshake: Connection lost during TLS handshake', $error->getMessage());
         $this->assertEquals(defined('SOCKET_ECONNRESET') ? SOCKET_ECONNRESET : 0, $error->getCode());
+        $this->assertNull($error->getPrevious());
     }
 
     public function testEmitsNothingIfConnectionIsIdle()

diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php
@@ -280,6 +280,46 @@ function ($e) use (&$wait) {
         $this->assertEquals(0, gc_collect_cycles());
     }
 
+    /**
+     * @requires PHP 7
+     */
+    public function testWaitingForInvalidTlsConnectionShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        $loop = Factory::create();
+        $connector = new Connector($loop, array(
+            'tls' => array(
+                'verify_peer' => true
+            )
+        ));
+
+        gc_collect_cycles();
+
+        $wait = true;
+        $promise = $connector->connect('tls://self-signed.badssl.com:443')->then(
+            null,
+            function ($e) use (&$wait) {
+                $wait = false;
+                throw $e;
+            }
+        );
+
+        // run loop for short period to ensure we detect DNS error
+        Block\sleep(0.1, $loop);
+        if ($wait) {
+            Block\sleep(0.4, $loop);
+            if ($wait) {
+                $this->fail('Connection attempt did not fail');
+            }
+        }
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
     public function testWaitingForSuccessfullyClosedConnectionShouldNotCreateAnyGarbageReferences()
     {
         if (class_exists('React\Promise\When')) {
@@ -303,10 +343,6 @@ function ($conn) {
 
     public function testConnectingFailsIfTimeoutIsTooSmall()
     {
-        if (!function_exists('stream_socket_enable_crypto')) {
-            $this->markTestSkipped('Not supported on your platform (outdated HHVM?)');
-        }
-
         $loop = Factory::create();
 
         $connector = new Connector($loop, array(

diff --git a/tests/SecureConnectorTest.php b/tests/SecureConnectorTest.php
@@ -3,6 +3,7 @@
 namespace React\Tests\Socket;
 
 use React\Promise;
+use React\Promise\Deferred;
 use React\Socket\SecureConnector;
 
 class SecureConnectorTest extends TestCase
@@ -51,16 +52,33 @@ public function testConnectionToInvalidSchemeWillReject()
 
     public function testCancelDuringTcpConnectionCancelsTcpConnection()
     {
-        $pending = new Promise\Promise(function () { }, function () { throw new \Exception(); });
+        $pending = new Promise\Promise(function () { }, $this->expectCallableOnce());
+        $this->tcp->expects($this->once())->method('connect')->with('example.com:80')->willReturn($pending);
+
+        $promise = $this->connector->connect('example.com:80');
+        $promise->cancel();
+    }
+
+    /**
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Connection cancelled
+     */
+    public function testCancelDuringTcpConnectionCancelsTcpConnectionAndRejectsWithTcpRejection()
+    {
+        $pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
         $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->will($this->returnValue($pending));
 
         $promise = $this->connector->connect('example.com:80');
         $promise->cancel();
 
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnce());
+        $this->throwRejection($promise);
     }
 
-    public function testConnectionWillBeClosedAndRejectedIfConnectioIsNoStream()
+    /**
+     * @expectedException UnexpectedValueException
+     * @expectedExceptionMessage Base connector does not use internal Connection class exposing stream resource
+     */
+    public function testConnectionWillBeClosedAndRejectedIfConnectionIsNoStream()
     {
         $connection = $this->getMockBuilder('React\Socket\ConnectionInterface')->getMock();
         $connection->expects($this->once())->method('close');
@@ -69,6 +87,133 @@ public function testConnectionWillBeClosedAndRejectedIfConnectioIsNoStream()
 
         $promise = $this->connector->connect('example.com:80');
 
-        $promise->then($this->expectCallableNever(), $this->expectCallableOnce());
+        $this->throwRejection($promise);
+    }
+
+    public function testStreamEncryptionWillBeEnabledAfterConnecting()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
+
+        $encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
+        $encryption->expects($this->once())->method('enable')->with($connection)->willReturn(new \React\Promise\Promise(function () { }));
+
+        $ref = new \ReflectionProperty($this->connector, 'streamEncryption');
+        $ref->setAccessible(true);
+        $ref->setValue($this->connector, $encryption);
+
+        $pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
+        $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
+
+        $promise = $this->connector->connect('example.com:80');
+    }
+
+    public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
+        $connection->expects($this->once())->method('close');
+
+        $encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
+        $encryption->expects($this->once())->method('enable')->willReturn(Promise\reject(new \RuntimeException('TLS error', 123)));
+
+        $ref = new \ReflectionProperty($this->connector, 'streamEncryption');
+        $ref->setAccessible(true);
+        $ref->setValue($this->connector, $encryption);
+
+        $pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
+        $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
+
+        $promise = $this->connector->connect('example.com:80');
+
+        try {
+            $this->throwRejection($promise);
+        } catch (\RuntimeException $e) {
+            $this->assertEquals('Connection to example.com:80 failed during TLS handshake: TLS error', $e->getMessage());
+            $this->assertEquals(123, $e->getCode());
+            $this->assertNull($e->getPrevious());
+        }
+    }
+
+    /**
+     * @expectedException RuntimeException
+     * @expectedExceptionMessage Connection to example.com:80 cancelled during TLS handshake
+     */
+    public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnection()
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
+        $connection->expects($this->once())->method('close');
+
+        $pending = new Promise\Promise(function () { }, function () {
+            throw new \Exception('Ignored');
+        });
+        $encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
+        $encryption->expects($this->once())->method('enable')->willReturn($pending);
+
+        $ref = new \ReflectionProperty($this->connector, 'streamEncryption');
+        $ref->setAccessible(true);
+        $ref->setValue($this->connector, $encryption);
+
+        $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
+
+        $promise = $this->connector->connect('example.com:80');
+        $promise->cancel();
+
+        $this->throwRejection($promise);
+    }
+
+    public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $tcp = new Deferred();
+        $this->tcp->expects($this->once())->method('connect')->willReturn($tcp->promise());
+
+        $promise = $this->connector->connect('example.com:80');
+        $tcp->reject(new \RuntimeException());
+        unset($promise, $tcp);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testRejectionDuringTlsHandshakeShouldNotCreateAnyGarbageReferences()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
+
+        $tcp = new Deferred();
+        $this->tcp->expects($this->once())->method('connect')->willReturn($tcp->promise());
+
+        $tls = new Deferred();
+        $encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
+        $encryption->expects($this->once())->method('enable')->willReturn($tls->promise());
+
+        $ref = new \ReflectionProperty($this->connector, 'streamEncryption');
+        $ref->setAccessible(true);
+        $ref->setValue($this->connector, $encryption);
+
+        $promise = $this->connector->connect('example.com:80');
+        $tcp->resolve($connection);
+        $tls->reject(new \RuntimeException());
+        unset($promise, $tcp, $tls);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    private function throwRejection($promise)
+    {
+        $ex = null;
+        $promise->then(null, function ($e) use (&$ex) {
+            $ex = $e;
+        });
+
+        throw $ex;
     }
 }