Skip to content

Commit 9f493fd

Browse files
clueclue
committed
Improve cancellation forwarding for TLS handshake after connecting
1 parent d874bac commit 9f493fd

File tree

4 files changed

+149
-8
lines changed

4 files changed

+149
-8
lines changed

‎src/SecureConnector.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ public function connect($uri)
4040
$context = $this->context;
4141

4242
$encryption = $this->streamEncryption;
43-
return $this->connector->connect($uri)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri) {
43+
$connected = false;
44+
$promise = $this->connector->connect($uri)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) {
4445
// (unencrypted) TCP/IP connection succeeded
46+
$connected = true;
4547

4648
if (!$connection instanceof Connection) {
4749
$connection->close();
@@ -54,7 +56,7 @@ public function connect($uri)
5456
}
5557

5658
// try to enable encryption
57-
return $encryption->enable($connection)->then(null, function ($error) use ($connection, $uri) {
59+
return $promise = $encryption->enable($connection)->then(null, function ($error) use ($connection, $uri) {
5860
// establishing encryption failed => close invalid connection and return error
5961
$connection->close();
6062

@@ -65,5 +67,19 @@ public function connect($uri)
6567
);
6668
});
6769
});
70+
71+
return new \React\Promise\Promise(
72+
function ($resolve, $reject) use ($promise) {
73+
$promise->then($resolve, $reject);
74+
},
75+
function ($_, $reject) use (&$promise, $uri, &$connected) {
76+
if ($connected) {
77+
$reject(new \RuntimeException('Connection to ' . $uri . ' cancelled during TLS handshake'));
78+
}
79+
80+
$promise->cancel();
81+
$promise = null;
82+
}
83+
);
6884
}
6985
}

‎src/StreamEncryption.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,20 @@ public function toggleCrypto($socket, Deferred $deferred, $toggle, $method)
136136
if (true === $result) {
137137
$deferred->resolve();
138138
} else if (false === $result) {
139+
// overwrite callback arguments for PHP7+ only, so they do not show
140+
// up in the Exception trace and do not cause a possible cyclic reference.
141+
$d = $deferred;
142+
$deferred = null;
143+
139144
if (\feof($socket) || $error === null) {
140145
// EOF or failed without error => connection closed during handshake
141-
$deferred->reject(new UnexpectedValueException(
146+
$d->reject(new UnexpectedValueException(
142147
'Connection lost during TLS handshake',
143148
\defined('SOCKET_ECONNRESET') ? \SOCKET_ECONNRESET : 0
144149
));
145150
} else {
146151
// handshake failed with error message
147-
$deferred->reject(new UnexpectedValueException(
152+
$d->reject(new UnexpectedValueException(
148153
'Unable to complete TLS handshake: ' . $error
149154
));
150155
}

‎tests/IntegrationTest.php‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,46 @@ function ($e) use (&$wait) {
280280
$this->assertEquals(0, gc_collect_cycles());
281281
}
282282

283+
/**
284+
* @requires PHP 7
285+
*/
286+
public function testWaitingForInvalidTlsConnectionShouldNotCreateAnyGarbageReferences()
287+
{
288+
if (class_exists('React\Promise\When')) {
289+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
290+
}
291+
292+
$loop = Factory::create();
293+
$connector = new Connector($loop, array(
294+
'tls' => array(
295+
'verify_peer' => true
296+
)
297+
));
298+
299+
gc_collect_cycles();
300+
301+
$wait = true;
302+
$promise = $connector->connect('tls://self-signed.badssl.com:443')->then(
303+
null,
304+
function ($e) use (&$wait) {
305+
$wait = false;
306+
throw $e;
307+
}
308+
);
309+
310+
// run loop for short period to ensure we detect DNS error
311+
Block\sleep(0.1, $loop);
312+
if ($wait) {
313+
Block\sleep(0.4, $loop);
314+
if ($wait) {
315+
$this->fail('Connection attempt did not fail');
316+
}
317+
}
318+
unset($promise);
319+
320+
$this->assertEquals(0, gc_collect_cycles());
321+
}
322+
283323
public function testWaitingForSuccessfullyClosedConnectionShouldNotCreateAnyGarbageReferences()
284324
{
285325
if (class_exists('React\Promise\When')) {
@@ -303,10 +343,6 @@ function ($conn) {
303343

304344
public function testConnectingFailsIfTimeoutIsTooSmall()
305345
{
306-
if (!function_exists('stream_socket_enable_crypto')) {
307-
$this->markTestSkipped('Not supported on your platform (outdated HHVM?)');
308-
}
309-
310346
$loop = Factory::create();
311347

312348
$connector = new Connector($loop, array(

‎tests/SecureConnectorTest.php‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace React\Tests\Socket;
44

55
use React\Promise;
6+
use React\Promise\Deferred;
67
use React\Socket\SecureConnector;
78

89
class SecureConnectorTest extends TestCase
@@ -49,6 +50,15 @@ public function testConnectionToInvalidSchemeWillReject()
4950
$promise->then(null, $this->expectCallableOnce());
5051
}
5152

53+
public function testCancelDuringTcpConnectionCancelsTcpConnection()
54+
{
55+
$pending = new Promise\Promise(function () { }, $this->expectCallableOnce());
56+
$this->tcp->expects($this->once())->method('connect')->with('example.com:80')->willReturn($pending);
57+
58+
$promise = $this->connector->connect('example.com:80');
59+
$promise->cancel();
60+
}
61+
5262
/**
5363
* @expectedException RuntimeException
5464
* @expectedExceptionMessage Connection cancelled
@@ -121,6 +131,80 @@ public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConn
121131
$this->throwRejection($promise);
122132
}
123133

134+
/**
135+
* @expectedException RuntimeException
136+
* @expectedExceptionMessage Connection to example.com:80 cancelled during TLS handshake
137+
*/
138+
public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnection()
139+
{
140+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
141+
$connection->expects($this->once())->method('close');
142+
143+
$pending = new Promise\Promise(function () { }, function () {
144+
throw new \Exception('Ignored');
145+
});
146+
$encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
147+
$encryption->expects($this->once())->method('enable')->willReturn($pending);
148+
149+
$ref = new \ReflectionProperty($this->connector, 'streamEncryption');
150+
$ref->setAccessible(true);
151+
$ref->setValue($this->connector, $encryption);
152+
153+
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
154+
155+
$promise = $this->connector->connect('example.com:80');
156+
$promise->cancel();
157+
158+
$this->throwRejection($promise);
159+
}
160+
161+
public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()
162+
{
163+
if (class_exists('React\Promise\When')) {
164+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
165+
}
166+
167+
gc_collect_cycles();
168+
169+
$tcp = new Deferred();
170+
$this->tcp->expects($this->once())->method('connect')->willReturn($tcp->promise());
171+
172+
$promise = $this->connector->connect('example.com:80');
173+
$tcp->reject(new \RuntimeException());
174+
unset($promise, $tcp);
175+
176+
$this->assertEquals(0, gc_collect_cycles());
177+
}
178+
179+
public function testRejectionDuringTlsHandshakeShouldNotCreateAnyGarbageReferences()
180+
{
181+
if (class_exists('React\Promise\When')) {
182+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
183+
}
184+
185+
gc_collect_cycles();
186+
187+
$connection = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
188+
189+
$tcp = new Deferred();
190+
$this->tcp->expects($this->once())->method('connect')->willReturn($tcp->promise());
191+
192+
$tls = new Deferred();
193+
$encryption = $this->getMockBuilder('React\Socket\StreamEncryption')->disableOriginalConstructor()->getMock();
194+
$encryption->expects($this->once())->method('enable')->willReturn($tls->promise());
195+
196+
$ref = new \ReflectionProperty($this->connector, 'streamEncryption');
197+
$ref->setAccessible(true);
198+
$ref->setValue($this->connector, $encryption);
199+
200+
$promise = $this->connector->connect('example.com:80');
201+
$tcp->resolve($connection);
202+
$tls->reject(new \RuntimeException());
203+
unset($promise, $tcp, $tls);
204+
205+
$this->assertEquals(0, gc_collect_cycles());
206+
}
207+
124208
private function throwRejection($promise)
125209
{
126210
$ex = null;

0 commit comments

Comments
 (0)