diff --git a/src/SecureConnector.php b/src/SecureConnector.php index f04183d3..ca8c838c 100644 --- 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 index 302ae938..59562c60 100644 --- 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 index 06a0936a..5e482162 100644 --- 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 index ce32f366..72a79faa 100644 --- 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 index 0a048ce1..ae288026 100644 --- 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 index 0b3a7025..10cfdf37 100644 --- 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; } }