diff --git a/README.md b/README.md index 6beab00a..3b444dfd 100644 --- a/README.md +++ b/README.md @@ -1259,6 +1259,13 @@ IRC over TLS, but should not affect HTTP over TLS (HTTPS). Further investigation of this issue is needed. For more insights, this issue is also covered by our test suite. +PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big +chunks of data over TLS streams at once. +We try to work around this by limiting the write chunk size to 8192 +bytes for older PHP versions only. +This is only a work-around and has a noticable performance penalty on +affected versions. + This project also supports running on HHVM. Note that really old HHVM < 3.8 does not support secure TLS connections, as it lacks the required `stream_socket_enable_crypto()` function. diff --git a/composer.json b/composer.json index 20d9db7e..48a9073e 100644 --- a/composer.json +++ b/composer.json @@ -8,14 +8,13 @@ "evenement/evenement": "^3.0 || ^2.0 || ^1.0", "react/dns": "^0.4.11", "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5", - "react/stream": "^1.0 || ^0.7 || ^0.6 || ^0.5 || ^0.4.5", + "react/stream": "^1.0 || ^0.7.1", "react/promise": "^2.1 || ^1.2", "react/promise-timer": "~1.0" }, "require-dev": { "clue/block-react": "^1.1", - "phpunit/phpunit": "~4.8", - "react/stream": "^1.0 || ^0.7 || ^0.6" + "phpunit/phpunit": "~4.8" }, "autoload": { "psr-4": { diff --git a/src/Connection.php b/src/Connection.php index 39d945b8..ae762d87 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -5,9 +5,9 @@ use Evenement\EventEmitter; use React\EventLoop\LoopInterface; use React\Stream\DuplexResourceStream; -use React\Stream\Stream; use React\Stream\Util; use React\Stream\WritableStreamInterface; +use React\Stream\WritableResourceStream; /** * The actual connection implementation for ConnectionInterface @@ -50,20 +50,24 @@ public function __construct($resource, LoopInterface $loop) // See https://bugs.php.net/bug.php?id=65137 // https://bugs.php.net/bug.php?id=41631 // https://github.com/reactphp/socket-client/issues/24 - $clearCompleteBuffer = (version_compare(PHP_VERSION, '5.6.8', '<')); - - // @codeCoverageIgnoreStart - if (class_exists('React\Stream\Stream')) { - // legacy react/stream < 0.7 requires additional buffer property - $this->input = new Stream($resource, $loop); - if ($clearCompleteBuffer) { - $this->input->bufferSize = null; - } - } else { - // preferred react/stream >= 0.7 accepts buffer parameter - $this->input = new DuplexResourceStream($resource, $loop, $clearCompleteBuffer ? -1 : null); - } - // @codeCoverageIgnoreEnd + $clearCompleteBuffer = PHP_VERSION_ID < 50608; + + // PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big + // chunks of data over TLS streams at once. + // We try to work around this by limiting the write chunk size to 8192 + // bytes for older PHP versions only. + // This is only a work-around and has a noticable performance penalty on + // affected versions. Please update your PHP version. + // This applies to all streams because TLS may be enabled later on. + // See https://github.com/reactphp/socket/issues/105 + $limitWriteChunks = (PHP_VERSION_ID < 70018 || (PHP_VERSION_ID >= 70100 && PHP_VERSION_ID < 70104)); + + $this->input = new DuplexResourceStream( + $resource, + $loop, + $clearCompleteBuffer ? -1 : null, + new WritableResourceStream($resource, $loop, null, $limitWriteChunks ? 8192 : null) + ); $this->stream = $resource; diff --git a/tests/FunctionalSecureServerTest.php b/tests/FunctionalSecureServerTest.php index f9e719dd..063a87e0 100644 --- a/tests/FunctionalSecureServerTest.php +++ b/tests/FunctionalSecureServerTest.php @@ -98,6 +98,38 @@ public function testWritesDataInMultipleChunksToConnection() $this->assertEquals(400000, $received); } + public function testWritesMoreDataInMultipleChunksToConnection() + { + $loop = Factory::create(); + + $server = new TcpServer(0, $loop); + $server = new SecureServer($server, $loop, array( + 'local_cert' => __DIR__ . '/../examples/localhost.pem' + )); + $server->on('connection', $this->expectCallableOnce()); + + $server->on('connection', function (ConnectionInterface $conn) { + $conn->write(str_repeat('*', 2000000)); + }); + + $connector = new SecureConnector(new TcpConnector($loop), $loop, array( + 'verify_peer' => false + )); + $promise = $connector->connect($server->getAddress()); + + $local = Block\await($promise, $loop, self::TIMEOUT); + /* @var $local React\Stream\Stream */ + + $received = 0; + $local->on('data', function ($chunk) use (&$received) { + $received += strlen($chunk); + }); + + Block\sleep(self::TIMEOUT, $loop); + + $this->assertEquals(2000000, $received); + } + public function testEmitsDataFromConnection() { $loop = Factory::create();