Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor code cleanup, remove dead code and improve code coverage to 100% #236

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ private function parseAddress($address)
// remove trailing colon from address for HHVM < 3.19: https://3v4l.org/5C1lo
// note that technically ":" is a valid address, so keep this in place otherwise
if (\substr($address, -1) === ':' && \defined('HHVM_VERSION_ID') && \HHVM_VERSION_ID < 31900) {
$address = (string)\substr($address, 0, -1);
$address = (string)\substr($address, 0, -1); // @codeCoverageIgnore
}

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

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

return ($this->encryptionEnabled ? 'tls' : 'tcp') . '://' . $address;
Expand Down
15 changes: 4 additions & 11 deletions src/StreamEncryption.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ public function __construct(LoopInterface $loop, $server = true)
$this->method = \STREAM_CRYPTO_METHOD_TLS_SERVER;

if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER;
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_1_SERVER | \STREAM_CRYPTO_METHOD_TLSv1_2_SERVER; // @codeCoverageIgnore
}
} else {
$this->method = \STREAM_CRYPTO_METHOD_TLS_CLIENT;

if (\PHP_VERSION_ID < 70200 && \PHP_VERSION_ID >= 50600) {
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT;
$this->method |= \STREAM_CRYPTO_METHOD_TLSv1_0_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_1_CLIENT | \STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT; // @codeCoverageIgnore
}
}
}
Expand All @@ -49,22 +49,15 @@ public function enable(Connection $stream)
return $this->toggle($stream, true);
}

public function disable(Connection $stream)
{
return $this->toggle($stream, false);
}

public function toggle(Connection $stream, $toggle)
{
// pause actual stream instance to continue operation on raw stream socket
$stream->pause();

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

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

// get actual stream socket from stream instance
$socket = $stream->stream;
Expand Down
2 changes: 2 additions & 0 deletions src/TcpConnector.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ public function connect($uri)
// Legacy PHP < 5.6 ignores peer_name and requires legacy context options instead.
// The SNI_server_name context option has to be set here during construction,
// as legacy PHP ignores any values set later.
// @codeCoverageIgnoreStart
if (\PHP_VERSION_ID < 50600) {
$context['ssl'] += array(
'SNI_server_name' => $args['hostname'],
'CN_match' => $args['hostname']
);
}
// @codeCoverageIgnoreEnd
}

// latest versions of PHP no longer accept any other URI components and
Expand Down
3 changes: 1 addition & 2 deletions src/TcpServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ public function getAddress()
// check if this is an IPv6 address which includes multiple colons but no square brackets
$pos = \strrpos($address, ':');
if ($pos !== false && \strpos($address, ':') < $pos && \substr($address, 0, 1) !== '[') {
$port = \substr($address, $pos + 1);
$address = '[' . \substr($address, 0, $pos) . ']:' . $port;
$address = '[' . \substr($address, 0, $pos) . ']:' . \substr($address, $pos + 1); // @codeCoverageIgnore
}

return 'tcp://' . $address;
Expand Down
17 changes: 17 additions & 0 deletions tests/ServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,23 @@ public function testConstructorThrowsForExistingUnixPath()
}
}

public function testEmitsErrorWhenUnderlyingTcpServerEmitsError()
{
$loop = Factory::create();

$server = new Server(0, $loop);

$ref = new \ReflectionProperty($server, 'server');
$ref->setAccessible(true);
$tcp = $ref->getvalue($server);

$error = new \RuntimeException();
$server->on('error', $this->expectCallableOnceWith($error));
$tcp->emit('error', array($error));

$server->close();
}

public function testEmitsConnectionForNewConnection()
{
$loop = Factory::create();
Expand Down
32 changes: 32 additions & 0 deletions tests/TcpConnectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use React\Socket\ConnectionInterface;
use React\Socket\TcpConnector;
use React\Socket\TcpServer;
use React\Promise\Promise;

class TcpConnectorTest extends TestCase
{
Expand Down Expand Up @@ -63,6 +64,37 @@ public function connectionToTcpServerShouldSucceed()
$connection->close();
}

/**
* @test
* @expectedException RuntimeException
*/
public function connectionToTcpServerShouldFailIfFileDescriptorsAreExceeded()
{
$loop = Factory::create();

$connector = new TcpConnector($loop);

$ulimit = exec('ulimit -n 2>&1');
if ($ulimit < 1) {
$this->markTestSkipped('Unable to determine limit of open files (ulimit not available?)');
}

// dummy rejected promise to make sure autoloader has initialized all classes
$foo = new Promise(function () { throw new \RuntimeException('dummy'); });

// keep creating dummy file handles until all file descriptors are exhausted
$fds = array();
for ($i = 0; $i < $ulimit; ++$i) {
$fd = @fopen('/dev/null', 'r');
if ($fd === false) {
break;
}
$fds[] = $fd;
}

Block\await($connector->connect('127.0.0.1:9999'), $loop, self::TIMEOUT);
}

/** @test */
public function connectionToTcpServerShouldSucceedWithRemoteAdressSameAsTarget()
{
Expand Down
17 changes: 17 additions & 0 deletions tests/TcpServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,23 @@ public function testCloseRemovesResourceFromLoop()
$server->close();
}

public function testEmitsErrorWhenAcceptListenerFails()
{
$listener = null;
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
$loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) {
$listener = $cb;
return true;
}));

$server = new TcpServer(0, $loop);

$server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

$this->assertNotNull($listener);
$listener(false);
}

/**
* @expectedException RuntimeException
*/
Expand Down
37 changes: 37 additions & 0 deletions tests/UnixServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,26 @@ public function testCtorAddsResourceToLoop()
$server = new UnixServer($this->getRandomSocketUri(), $loop);
}

/**
* @expectedException InvalidArgumentException
*/
public function testCtorThrowsForInvalidAddressScheme()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$server = new UnixServer('tcp://localhost:0', $loop);
}

/**
* @expectedException RuntimeException
*/
public function testCtorThrowsWhenPathIsNotWritable()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$server = new UnixServer('/dev/null', $loop);
}

public function testResumeWithoutPauseIsNoOp()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand Down Expand Up @@ -253,6 +273,23 @@ public function testCloseRemovesResourceFromLoop()
$server->close();
}

public function testEmitsErrorWhenAcceptListenerFails()
{
$listener = null;
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
$loop->expects($this->once())->method('addReadStream')->with($this->anything(), $this->callback(function ($cb) use (&$listener) {
$listener = $cb;
return true;
}));

$server = new UnixServer($this->getRandomSocketUri(), $loop);

$server->on('error', $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

$this->assertNotNull($listener);
$listener(false);
}

/**
* @expectedException RuntimeException
*/
Expand Down