diff --git a/src/DnsConnector.php b/src/DnsConnector.php index 0dfd6585..fa728227 100644 --- a/src/DnsConnector.php +++ b/src/DnsConnector.php @@ -40,71 +40,76 @@ public function connect($uri) return $connector->connect($uri); } - return $this - ->resolveHostname($host) - ->then(function ($ip) use ($connector, $host, $parts) { - $uri = ''; - - // prepend original scheme if known - if (isset($parts['scheme'])) { - $uri .= $parts['scheme'] . '://'; - } - - if (strpos($ip, ':') !== false) { - // enclose IPv6 addresses in square brackets before appending port - $uri .= '[' . $ip . ']'; - } else { - $uri .= $ip; - } - - // append original port if known - if (isset($parts['port'])) { - $uri .= ':' . $parts['port']; - } - - // append orignal path if known - if (isset($parts['path'])) { - $uri .= $parts['path']; - } - - // append original query if known - if (isset($parts['query'])) { - $uri .= '?' . $parts['query']; - } - - // append original hostname as query if resolved via DNS and if - // destination URI does not contain "hostname" query param already - $args = array(); - parse_str(isset($parts['query']) ? $parts['query'] : '', $args); - if ($host !== $ip && !isset($args['hostname'])) { - $uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . rawurlencode($host); - } - - // append original fragment if known - if (isset($parts['fragment'])) { - $uri .= '#' . $parts['fragment']; - } - - return $connector->connect($uri); - }); - } - - private function resolveHostname($host) - { $promise = $this->resolver->resolve($host); + $resolved = null; return new Promise\Promise( - function ($resolve, $reject) use ($promise) { + function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host, $parts) { // resolve/reject with result of DNS lookup - $promise->then($resolve, $reject); + $promise->then(function ($ip) use (&$promise, &$resolved, $connector, $host, $parts) { + $resolved = $ip; + $uri = ''; + + // prepend original scheme if known + if (isset($parts['scheme'])) { + $uri .= $parts['scheme'] . '://'; + } + + if (strpos($ip, ':') !== false) { + // enclose IPv6 addresses in square brackets before appending port + $uri .= '[' . $ip . ']'; + } else { + $uri .= $ip; + } + + // append original port if known + if (isset($parts['port'])) { + $uri .= ':' . $parts['port']; + } + + // append orignal path if known + if (isset($parts['path'])) { + $uri .= $parts['path']; + } + + // append original query if known + if (isset($parts['query'])) { + $uri .= '?' . $parts['query']; + } + + // append original hostname as query if resolved via DNS and if + // destination URI does not contain "hostname" query param already + $args = array(); + parse_str(isset($parts['query']) ? $parts['query'] : '', $args); + if ($host !== $ip && !isset($args['hostname'])) { + $uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . rawurlencode($host); + } + + // append original fragment if known + if (isset($parts['fragment'])) { + $uri .= '#' . $parts['fragment']; + } + + return $promise = $connector->connect($uri); + }, function ($e) use ($uri, $reject) { + $reject(new RuntimeException('Connection to ' . $uri .' failed during DNS lookup: ' . $e->getMessage(), 0, $e)); + })->then($resolve, $reject); }, - function ($_, $reject) use ($promise) { + function ($_, $reject) use (&$promise, &$resolved, $uri) { // cancellation should reject connection attempt - $reject(new RuntimeException('Connection attempt cancelled during DNS lookup')); + // reject DNS resolution with custom reason, otherwise rely on connection cancellation below + if ($resolved === null) { + $reject(new RuntimeException('Connection to ' . $uri . ' cancelled during DNS lookup')); + } - // (try to) cancel pending DNS lookup + // (try to) cancel pending DNS lookup / connection attempt if ($promise instanceof CancellablePromiseInterface) { + // overwrite callback arguments for PHP7+ only, so they do not show + // up in the Exception trace and do not cause a possible cyclic reference. + $_ = $reject = null; + $promise->cancel(); + $promise = null; } } ); diff --git a/tests/DnsConnectorTest.php b/tests/DnsConnectorTest.php index 3c94c390..e0fb0ea6 100644 --- a/tests/DnsConnectorTest.php +++ b/tests/DnsConnectorTest.php @@ -2,8 +2,9 @@ namespace React\Tests\Socket; -use React\Socket\DnsConnector; use React\Promise; +use React\Promise\Deferred; +use React\Socket\DnsConnector; class DnsConnectorTest extends TestCase { @@ -77,14 +78,70 @@ public function testRejectsImmediatelyIfUriIsInvalid() $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); } + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Connection failed + */ + public function testRejectsWithTcpConnectorRejectionIfGivenIp() + { + $promise = Promise\reject(new \RuntimeException('Connection failed')); + $this->resolver->expects($this->never())->method('resolve'); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80'))->willReturn($promise); + + $promise = $this->connector->connect('1.2.3.4:80'); + $promise->cancel(); + + $this->throwRejection($promise); + } + + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Connection failed + */ + public function testRejectsWithTcpConnectorRejectionAfterDnsIsResolved() + { + $promise = Promise\reject(new \RuntimeException('Connection failed')); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn(Promise\resolve('1.2.3.4')); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($promise); + + $promise = $this->connector->connect('example.com:80'); + $promise->cancel(); + + $this->throwRejection($promise); + } + + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Connection to example.invalid:80 failed during DNS lookup: DNS error + */ public function testSkipConnectionIfDnsFails() { - $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.invalid'))->will($this->returnValue(Promise\reject())); + $promise = Promise\reject(new \RuntimeException('DNS error')); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.invalid'))->willReturn($promise); $this->tcp->expects($this->never())->method('connect'); - $this->connector->connect('example.invalid:80'); + $promise = $this->connector->connect('example.invalid:80'); + + $this->throwRejection($promise); + } + + public function testRejectionExceptionUsesPreviousExceptionIfDnsFails() + { + $exception = new \RuntimeException(); + + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.invalid'))->willReturn(Promise\reject($exception)); + + $promise = $this->connector->connect('example.invalid:80'); + + $promise->then(null, function ($e) { + throw $e->getPrevious(); + })->then(null, $this->expectCallableOnceWith($this->identicalTo($exception))); } + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Connection to example.com:80 cancelled during DNS lookup + */ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection() { $pending = new Promise\Promise(function () { }, $this->expectCallableOnce()); @@ -94,18 +151,184 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection() $promise = $this->connector->connect('example.com:80'); $promise->cancel(); - $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); + $this->throwRejection($promise); } - public function testCancelDuringTcpConnectionCancelsTcpConnection() + public function testCancelDuringTcpConnectionCancelsTcpConnectionIfGivenIp() { - $pending = new Promise\Promise(function () { }, function () { throw new \Exception(); }); - $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->will($this->returnValue(Promise\resolve('1.2.3.4'))); - $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->will($this->returnValue($pending)); + $pending = new Promise\Promise(function () { }, $this->expectCallableOnce()); + $this->resolver->expects($this->never())->method('resolve'); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80'))->willReturn($pending); + + $promise = $this->connector->connect('1.2.3.4:80'); + $promise->cancel(); + } + + public function testCancelDuringTcpConnectionCancelsTcpConnectionAfterDnsIsResolved() + { + $pending = new Promise\Promise(function () { }, $this->expectCallableOnce()); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn(Promise\resolve('1.2.3.4')); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($pending); $promise = $this->connector->connect('example.com:80'); $promise->cancel(); + } - $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); + /** + * @expectedException RuntimeException + * @expectedExceptionMessage Connection cancelled + */ + public function testCancelDuringTcpConnectionCancelsTcpConnectionWithTcpRejectionAfterDnsIsResolved() + { + $first = new Deferred(); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($first->promise()); + $pending = new Promise\Promise(function () { }, function () { + throw new \RuntimeException('Connection cancelled'); + }); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($pending); + + $promise = $this->connector->connect('example.com:80'); + $first->resolve('1.2.3.4'); + + $promise->cancel(); + + $this->throwRejection($promise); + } + + /** + * @requires PHP 7 + */ + public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $dns = new Deferred(); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise()); + $this->tcp->expects($this->never())->method('connect'); + + $promise = $this->connector->connect('example.com:80'); + $dns->reject(new \RuntimeException('DNS failed')); + unset($promise, $dns); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @requires PHP 7 + */ + public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferences() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $dns = new Deferred(); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise()); + + $tcp = new Deferred(); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp->promise()); + + $promise = $this->connector->connect('example.com:80'); + $dns->resolve('1.2.3.4'); + $tcp->reject(new \RuntimeException('Connection failed')); + unset($promise, $dns, $tcp); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @requires PHP 7 + */ + public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferencesAgain() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $dns = new Deferred(); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise()); + + $tcp = new Deferred(); + $dns->promise()->then(function () use ($tcp) { + $tcp->reject(new \RuntimeException('Connection failed')); + }); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp->promise()); + + $promise = $this->connector->connect('example.com:80'); + $dns->resolve('1.2.3.4'); + + unset($promise, $dns, $tcp); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @requires PHP 7 + */ + public function testCancelDuringDnsLookupShouldNotCreateAnyGarbageReferences() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $dns = new Deferred(function () { + throw new \RuntimeException(); + }); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise()); + $this->tcp->expects($this->never())->method('connect'); + + $promise = $this->connector->connect('example.com:80'); + + $promise->cancel(); + unset($promise, $dns); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @requires PHP 7 + */ + public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $dns = new Deferred(); + $this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise()); + $tcp = new Promise\Promise(function () { }, function () { + throw new \RuntimeException('Connection cancelled'); + }); + $this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp); + + $promise = $this->connector->connect('example.com:80'); + $dns->resolve('1.2.3.4'); + + $promise->cancel(); + unset($promise, $dns, $tcp); + + $this->assertEquals(0, gc_collect_cycles()); + } + + private function throwRejection($promise) + { + $ex = null; + $promise->then(null, function ($e) use (&$ex) { + $ex = $e; + }); + + throw $ex; } } diff --git a/tests/IntegrationTest.php b/tests/IntegrationTest.php index 59dff4f1..7a3b45fa 100644 --- a/tests/IntegrationTest.php +++ b/tests/IntegrationTest.php @@ -184,7 +184,7 @@ function ($e) use (&$wait) { /** * @requires PHP 7 */ - public function testWaitingForConnectionTimeoutShouldNotCreateAnyGarbageReferences() + public function testWaitingForConnectionTimeoutDuringDnsLookupShouldNotCreateAnyGarbageReferences() { if (class_exists('React\Promise\When')) { $this->markTestSkipped('Not supported on legacy Promise v1 API'); @@ -217,6 +217,42 @@ function ($e) use (&$wait) { $this->assertEquals(0, gc_collect_cycles()); } + /** + * @requires PHP 7 + */ + public function testWaitingForConnectionTimeoutDuringTcpConnectionShouldNotCreateAnyGarbageReferences() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + $loop = Factory::create(); + $connector = new Connector($loop, array('timeout' => 0.000001)); + + gc_collect_cycles(); + + $wait = true; + $promise = $connector->connect('8.8.8.8:53')->then( + null, + function ($e) use (&$wait) { + $wait = false; + throw $e; + } + ); + + // run loop for short period to ensure we detect connection timeout error + Block\sleep(0.01, $loop); + if ($wait) { + Block\sleep(0.2, $loop); + if ($wait) { + $this->fail('Connection attempt did not fail'); + } + } + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + public function testWaitingForInvalidDnsConnectionShouldNotCreateAnyGarbageReferences() { if (class_exists('React\Promise\When')) {