Skip to content

Commit 86d8608

Browse files
clueclue
committed
Improve cancellation forwarding after DNS lookup
1 parent 97076ba commit 86d8608

File tree

2 files changed

+223
-63
lines changed

2 files changed

+223
-63
lines changed

‎src/DnsConnector.php‎

Lines changed: 56 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -40,73 +40,72 @@ public function connect($uri)
4040
return $connector->connect($uri);
4141
}
4242

43-
return $this
44-
->resolveHostname($host, $uri)
45-
->then(function ($ip) use ($connector, $host, $parts) {
46-
$uri = '';
47-
48-
// prepend original scheme if known
49-
if (isset($parts['scheme'])) {
50-
$uri .= $parts['scheme'] . '://';
51-
}
52-
53-
if (strpos($ip, ':') !== false) {
54-
// enclose IPv6 addresses in square brackets before appending port
55-
$uri .= '[' . $ip . ']';
56-
} else {
57-
$uri .= $ip;
58-
}
59-
60-
// append original port if known
61-
if (isset($parts['port'])) {
62-
$uri .= ':' . $parts['port'];
63-
}
64-
65-
// append orignal path if known
66-
if (isset($parts['path'])) {
67-
$uri .= $parts['path'];
68-
}
69-
70-
// append original query if known
71-
if (isset($parts['query'])) {
72-
$uri .= '?' . $parts['query'];
73-
}
74-
75-
// append original hostname as query if resolved via DNS and if
76-
// destination URI does not contain "hostname" query param already
77-
$args = array();
78-
parse_str(isset($parts['query']) ? $parts['query'] : '', $args);
79-
if ($host !== $ip && !isset($args['hostname'])) {
80-
$uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . rawurlencode($host);
81-
}
82-
83-
// append original fragment if known
84-
if (isset($parts['fragment'])) {
85-
$uri .= '#' . $parts['fragment'];
86-
}
87-
88-
return $connector->connect($uri);
89-
});
90-
}
91-
92-
private function resolveHostname($host, $uri)
93-
{
9443
$promise = $this->resolver->resolve($host);
44+
$resolved = null;
9545

9646
return new Promise\Promise(
97-
function ($resolve, $reject) use ($promise, $uri) {
47+
function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host, $parts) {
9848
// resolve/reject with result of DNS lookup
99-
$promise->then($resolve, function ($e) use ($uri, $reject) {
49+
$promise->then(function ($ip) use (&$promise, &$resolved, $connector, $host, $parts) {
50+
$resolved = $ip;
51+
$uri = '';
52+
53+
// prepend original scheme if known
54+
if (isset($parts['scheme'])) {
55+
$uri .= $parts['scheme'] . '://';
56+
}
57+
58+
if (strpos($ip, ':') !== false) {
59+
// enclose IPv6 addresses in square brackets before appending port
60+
$uri .= '[' . $ip . ']';
61+
} else {
62+
$uri .= $ip;
63+
}
64+
65+
// append original port if known
66+
if (isset($parts['port'])) {
67+
$uri .= ':' . $parts['port'];
68+
}
69+
70+
// append orignal path if known
71+
if (isset($parts['path'])) {
72+
$uri .= $parts['path'];
73+
}
74+
75+
// append original query if known
76+
if (isset($parts['query'])) {
77+
$uri .= '?' . $parts['query'];
78+
}
79+
80+
// append original hostname as query if resolved via DNS and if
81+
// destination URI does not contain "hostname" query param already
82+
$args = array();
83+
parse_str(isset($parts['query']) ? $parts['query'] : '', $args);
84+
if ($host !== $ip && !isset($args['hostname'])) {
85+
$uri .= (isset($parts['query']) ? '&' : '?') . 'hostname=' . rawurlencode($host);
86+
}
87+
88+
// append original fragment if known
89+
if (isset($parts['fragment'])) {
90+
$uri .= '#' . $parts['fragment'];
91+
}
92+
93+
return $promise = $connector->connect($uri);
94+
}, function ($e) use ($uri, $reject) {
10095
$reject(new RuntimeException('Connection to ' . $uri .' failed during DNS lookup: ' . $e->getMessage(), 0, $e));
101-
});
96+
})->then($resolve, $reject);
10297
},
103-
function ($_, $reject) use ($promise, $uri) {
98+
function ($_, $reject) use (&$promise, &$resolved, $uri) {
10499
// cancellation should reject connection attempt
105-
$reject(new RuntimeException('Connection to ' . $uri . ' cancelled during DNS lookup'));
100+
// reject DNS resolution with custom reason, otherwise rely on connection cancellation below
101+
if ($resolved === null) {
102+
$reject(new RuntimeException('Connection to ' . $uri . ' cancelled during DNS lookup'));
103+
}
106104

107-
// (try to) cancel pending DNS lookup
105+
// (try to) cancel pending DNS lookup / connection attempt
108106
if ($promise instanceof CancellablePromiseInterface) {
109107
$promise->cancel();
108+
$promise = null;
110109
}
111110
}
112111
);

‎tests/DnsConnectorTest.php‎

Lines changed: 167 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
namespace React\Tests\Socket;
44

5-
use React\Socket\DnsConnector;
65
use React\Promise;
6+
use React\Promise\Deferred;
7+
use React\Socket\DnsConnector;
78

89
class DnsConnectorTest extends TestCase
910
{
@@ -77,6 +78,38 @@ public function testRejectsImmediatelyIfUriIsInvalid()
7778
$promise->then($this->expectCallableNever(), $this->expectCallableOnce());
7879
}
7980

81+
/**
82+
* @expectedException RuntimeException
83+
* @expectedExceptionMessage Connection failed
84+
*/
85+
public function testRejectsWithTcpConnectorRejectionIfGivenIp()
86+
{
87+
$promise = Promise\reject(new \RuntimeException('Connection failed'));
88+
$this->resolver->expects($this->never())->method('resolve');
89+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80'))->willReturn($promise);
90+
91+
$promise = $this->connector->connect('1.2.3.4:80');
92+
$promise->cancel();
93+
94+
$this->throwRejection($promise);
95+
}
96+
97+
/**
98+
* @expectedException RuntimeException
99+
* @expectedExceptionMessage Connection failed
100+
*/
101+
public function testRejectsWithTcpConnectorRejectionAfterDnsIsResolved()
102+
{
103+
$promise = Promise\reject(new \RuntimeException('Connection failed'));
104+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn(Promise\resolve('1.2.3.4'));
105+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($promise);
106+
107+
$promise = $this->connector->connect('example.com:80');
108+
$promise->cancel();
109+
110+
$this->throwRejection($promise);
111+
}
112+
80113
/**
81114
* @expectedException RuntimeException
82115
* @expectedExceptionMessage Connection to example.invalid:80 failed during DNS lookup: DNS error
@@ -121,16 +154,144 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection()
121154
$this->throwRejection($promise);
122155
}
123156

124-
public function testCancelDuringTcpConnectionCancelsTcpConnection()
157+
public function testCancelDuringTcpConnectionCancelsTcpConnectionIfGivenIp()
125158
{
126-
$pending = new Promise\Promise(function () { }, function () { throw new \Exception(); });
127-
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->will($this->returnValue(Promise\resolve('1.2.3.4')));
128-
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->will($this->returnValue($pending));
159+
$pending = new Promise\Promise(function () { }, $this->expectCallableOnce());
160+
$this->resolver->expects($this->never())->method('resolve');
161+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80'))->willReturn($pending);
162+
163+
$promise = $this->connector->connect('1.2.3.4:80');
164+
$promise->cancel();
165+
}
166+
167+
public function testCancelDuringTcpConnectionCancelsTcpConnectionAfterDnsIsResolved()
168+
{
169+
$pending = new Promise\Promise(function () { }, $this->expectCallableOnce());
170+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn(Promise\resolve('1.2.3.4'));
171+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($pending);
129172

130173
$promise = $this->connector->connect('example.com:80');
131174
$promise->cancel();
175+
}
132176

133-
$promise->then($this->expectCallableNever(), $this->expectCallableOnce());
177+
/**
178+
* @expectedException RuntimeException
179+
* @expectedExceptionMessage Connection cancelled
180+
*/
181+
public function testCancelDuringTcpConnectionCancelsTcpConnectionWithTcpRejectionAfterDnsIsResolved()
182+
{
183+
$first = new Deferred();
184+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($first->promise());
185+
$pending = new Promise\Promise(function () { }, function () {
186+
throw new \RuntimeException('Connection cancelled');
187+
});
188+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($pending);
189+
190+
$promise = $this->connector->connect('example.com:80');
191+
$first->resolve('1.2.3.4');
192+
193+
$promise->cancel();
194+
195+
$this->throwRejection($promise);
196+
}
197+
198+
/**
199+
* @requires PHP 7
200+
*/
201+
public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences()
202+
{
203+
if (class_exists('React\Promise\When')) {
204+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
205+
}
206+
207+
gc_collect_cycles();
208+
209+
$dns = new Deferred();
210+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise());
211+
$this->tcp->expects($this->never())->method('connect');
212+
213+
$promise = $this->connector->connect('example.com:80');
214+
$dns->reject(new \RuntimeException('DNS failed'));
215+
unset($promise, $dns);
216+
217+
$this->assertEquals(0, gc_collect_cycles());
218+
}
219+
220+
/**
221+
* @requires PHP 7
222+
*/
223+
public function testRejectionAfterDnsLookupShouldNotCreateAnyGarbageReferences()
224+
{
225+
if (class_exists('React\Promise\When')) {
226+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
227+
}
228+
229+
gc_collect_cycles();
230+
231+
$dns = new Deferred();
232+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise());
233+
234+
$tcp = new Deferred();
235+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp->promise());
236+
237+
$promise = $this->connector->connect('example.com:80');
238+
$dns->resolve('1.2.3.4');
239+
$tcp->reject(new \RuntimeException('Connection failed'));
240+
unset($promise, $dns, $tcp);
241+
242+
$this->assertEquals(0, gc_collect_cycles());
243+
}
244+
245+
/**
246+
* @requires PHP 7
247+
*/
248+
public function testCancelDuringDnsLookupShouldNotCreateAnyGarbageReferences()
249+
{
250+
if (class_exists('React\Promise\When')) {
251+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
252+
}
253+
254+
gc_collect_cycles();
255+
256+
$dns = new Deferred(function () {
257+
throw new \RuntimeException();
258+
});
259+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise());
260+
$this->tcp->expects($this->never())->method('connect');
261+
262+
$promise = $this->connector->connect('example.com:80');
263+
264+
$promise->cancel();
265+
unset($promise, $dns);
266+
267+
$this->assertEquals(0, gc_collect_cycles());
268+
}
269+
270+
/**
271+
* @requires PHP 7
272+
*/
273+
public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences()
274+
{
275+
if (class_exists('React\Promise\When')) {
276+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
277+
}
278+
279+
gc_collect_cycles();
280+
281+
$dns = new Deferred();
282+
$this->resolver->expects($this->once())->method('resolve')->with($this->equalTo('example.com'))->willReturn($dns->promise());
283+
$tcp = new Promise\Promise(function () { }, function () {
284+
throw new \RuntimeException('Connection cancelled');
285+
});
286+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('1.2.3.4:80?hostname=example.com'))->willReturn($tcp);
287+
288+
$promise = $this->connector->connect('example.com:80');
289+
$dns->resolve('1.2.3.4');
290+
291+
$promise->cancel();
292+
unset($promise, $dns);
293+
294+
$this->assertEquals(0, gc_collect_cycles());
134295
}
135296

136297
private function throwRejection($promise)

0 commit comments

Comments
 (0)