Skip to content

Commit b3de2a0

Browse files
jsorjsor
authored
Merge pull request #236 from clue-labs/covfefe
Minor code cleanup, remove dead code and improve code coverage to 100%
2 parents f5049f0 + 15426bd commit b3de2a0

File tree

8 files changed

+113
-17
lines changed

8 files changed

+113
-17
lines changed

‎src/Connection.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,13 @@ private function parseAddress($address)
156156
// remove trailing colon from address for HHVM < 3.19: https://3v4l.org/5C1lo
157157
// note that technically ":" is a valid address, so keep this in place otherwise
158158
if (\substr($address, -1) === ':' && \defined('HHVM_VERSION_ID') && \HHVM_VERSION_ID < 31900) {
159-
$address = (string)\substr($address, 0, -1);
159+
$address = (string)\substr($address, 0, -1); // @codeCoverageIgnore
160160
}
161161

162162
// work around unknown addresses should return null value: https://3v4l.org/5C1lo and https://bugs.php.net/bug.php?id=74556
163163
// PHP uses "\0" string and HHVM uses empty string (colon removed above)
164164
if ($address === '' || $address[0] === "\x00" ) {
165-
return null;
165+
return null; // @codeCoverageIgnore
166166
}
167167

168168
return 'unix://' . $address;
@@ -171,8 +171,7 @@ private function parseAddress($address)
171171
// check if this is an IPv6 address which includes multiple colons but no square brackets
172172
$pos = \strrpos($address, ':');
173173
if ($pos !== false && \strpos($address, ':') < $pos && \substr($address, 0, 1) !== '[') {
174-
$port = \substr($address, $pos + 1);
175-
$address = '[' . \substr($address, 0, $pos) . ']:' . $port;
174+
$address = '[' . \substr($address, 0, $pos) . ']:' . \substr($address, $pos + 1); // @codeCoverageIgnore
176175
}
177176

178177
return ($this->encryptionEnabled ? 'tls' : 'tcp') . '://' . $address;

‎src/StreamEncryption.php‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ public function __construct(LoopInterface $loop, $server = true)
3333
$this->method = \STREAM_CRYPTO_METHOD_TLS_SERVER;
3434

3535
if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
36-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER;
36+
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER; // @codeCoverageIgnore
3737
}
3838
} else {
3939
$this->method = \STREAM_CRYPTO_METHOD_TLS_CLIENT;
4040

4141
if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
42-
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT;
42+
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT; // @codeCoverageIgnore
4343
}
4444
}
4545
}
@@ -49,22 +49,15 @@ public function enable(Connection $stream)
4949
return $this->toggle($stream, true);
5050
}
5151

52-
public function disable(Connection $stream)
53-
{
54-
return $this->toggle($stream, false);
55-
}
56-
5752
public function toggle(Connection $stream, $toggle)
5853
{
5954
// pause actual stream instance to continue operation on raw stream socket
6055
$stream->pause();
6156

6257
// TODO: add write() event to make sure we're not sending any excessive data
6358

64-
$deferred = new Deferred(function ($_, $reject) use ($toggle) {
65-
// cancelling this leaves this stream in an inconsistent state…
66-
$reject(new \RuntimeException('Cancelled toggling encryption ' . $toggle ? 'on' : 'off'));
67-
});
59+
// cancelling this leaves this stream in an inconsistent state…
60+
$deferred = new Deferred();
6861

6962
// get actual stream socket from stream instance
7063
$socket = $stream->stream;

‎src/TcpConnector.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ public function connect($uri)
5959
// Legacy PHP < 5.6 ignores peer_name and requires legacy context options instead.
6060
// The SNI_server_name context option has to be set here during construction,
6161
// as legacy PHP ignores any values set later.
62+
// @codeCoverageIgnoreStart
6263
if (\PHP_VERSION_ID < 50600) {
6364
$context['ssl'] += array(
6465
'SNI_server_name' => $args['hostname'],
6566
'CN_match' => $args['hostname']
6667
);
6768
}
69+
// @codeCoverageIgnoreEnd
6870
}
6971

7072
// latest versions of PHP no longer accept any other URI components and

‎src/TcpServer.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,7 @@ public function getAddress()
179179
// check if this is an IPv6 address which includes multiple colons but no square brackets
180180
$pos = \strrpos($address, ':');
181181
if ($pos !== false && \strpos($address, ':') < $pos && \substr($address, 0, 1) !== '[') {
182-
$port = \substr($address, $pos + 1);
183-
$address = '[' . \substr($address, 0, $pos) . ']:' . $port;
182+
$address = '[' . \substr($address, 0, $pos) . ']:' . \substr($address, $pos + 1); // @codeCoverageIgnore
184183
}
185184

186185
return 'tcp://' . $address;

‎tests/ServerTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,23 @@ public function testConstructorThrowsForExistingUnixPath()
9191
}
9292
}
9393

94+
public function testEmitsErrorWhenUnderlyingTcpServerEmitsError()
95+
{
96+
$loop = Factory::create();
97+
98+
$server = new Server(0, $loop);
99+
100+
$ref = new \ReflectionProperty($server, 'server');
101+
$ref->setAccessible(true);
102+
$tcp = $ref->getvalue($server);
103+
104+
$error = new \RuntimeException();
105+
$server->on('error', $this->expectCallableOnceWith($error));
106+
$tcp->emit('error', array($error));
107+
108+
$server->close();
109+
}
110+
94111
public function testEmitsConnectionForNewConnection()
95112
{
96113
$loop = Factory::create();

‎tests/TcpConnectorTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use React\Socket\ConnectionInterface;
88
use React\Socket\TcpConnector;
99
use React\Socket\TcpServer;
10+
use React\Promise\Promise;
1011

1112
class TcpConnectorTest extends TestCase
1213
{
@@ -63,6 +64,37 @@ public function connectionToTcpServerShouldSucceed()
6364
$connection->close();
6465
}
6566

67+
/**
68+
* @test
69+
* @expectedException RuntimeException
70+
*/
71+
public function connectionToTcpServerShouldFailIfFileDescriptorsAreExceeded()
72+
{
73+
$loop = Factory::create();
74+
75+
$connector = new TcpConnector($loop);
76+
77+
$ulimit = exec('ulimit -n 2>&1');
78+
if ($ulimit < 1) {
79+
$this->markTestSkipped('Unable to determine limit of open files (ulimit not available?)');
80+
}
81+
82+
// dummy rejected promise to make sure autoloader has initialized all classes
83+
$foo = new Promise(function () { throw new \RuntimeException('dummy'); });
84+
85+
// keep creating dummy file handles until all file descriptors are exhausted
86+
$fds = array();
87+
for ($i = 0; $i < $ulimit; ++$i) {
88+
$fd = @fopen('/dev/null', 'r');
89+
if ($fd === false) {
90+
break;
91+
}
92+
$fds[] = $fd;
93+
}
94+
95+
Block\await($connector->connect('127.0.0.1:9999'), $loop, self::TIMEOUT);
96+
}
97+
6698
/** @test */
6799
public function connectionToTcpServerShouldSucceedWithRemoteAdressSameAsTarget()
68100
{

‎tests/TcpServerTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,23 @@ public function testCloseRemovesResourceFromLoop()
256256
$server->close();
257257
}
258258

259+
public function testEmitsErrorWhenAcceptListenerFails()
260+
{
261+
$listener = null;
262+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
263+
$loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) {
264+
$listener = $cb;
265+
return true;
266+
}));
267+
268+
$server = new TcpServer(0, $loop);
269+
270+
$server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
271+
272+
$this->assertNotNull($listener);
273+
$listener(false);
274+
}
275+
259276
/**
260277
* @expectedException RuntimeException
261278
*/

‎tests/UnixServerTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,26 @@ public function testCtorAddsResourceToLoop()
216216
$server = new UnixServer($this->getRandomSocketUri(), $loop);
217217
}
218218

219+
/**
220+
* @expectedException InvalidArgumentException
221+
*/
222+
public function testCtorThrowsForInvalidAddressScheme()
223+
{
224+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
225+
226+
$server = new UnixServer('tcp://localhost:0', $loop);
227+
}
228+
229+
/**
230+
* @expectedException RuntimeException
231+
*/
232+
public function testCtorThrowsWhenPathIsNotWritable()
233+
{
234+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
235+
236+
$server = new UnixServer('/dev/null', $loop);
237+
}
238+
219239
public function testResumeWithoutPauseIsNoOp()
220240
{
221241
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
@@ -253,6 +273,23 @@ public function testCloseRemovesResourceFromLoop()
253273
$server->close();
254274
}
255275

276+
public function testEmitsErrorWhenAcceptListenerFails()
277+
{
278+
$listener = null;
279+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
280+
$loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) {
281+
$listener = $cb;
282+
return true;
283+
}));
284+
285+
$server = new UnixServer($this->getRandomSocketUri(), $loop);
286+
287+
$server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));
288+
289+
$this->assertNotNull($listener);
290+
$listener(false);
291+
}
292+
256293
/**
257294
* @expectedException RuntimeException
258295
*/

0 commit comments

Comments
 (0)