Skip to content

Commit afc8f28

Browse files
committed
Work around write chunk size for TLS streams for PHP < 7.1.4
1 parent 8c9e1ca commit afc8f28

File tree

4 files changed

+60
-18
lines changed

4 files changed

+60
-18
lines changed

README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,13 @@ IRC over TLS, but should not affect HTTP over TLS (HTTPS).
12591259
Further investigation of this issue is needed.
12601260
For more insights, this issue is also covered by our test suite.
12611261

1262+
PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big
1263+
chunks of data over TLS streams at once.
1264+
We try to work around this by limiting the write chunk size to 8192
1265+
bytes for older PHP versions only.
1266+
This is only a work-around and has a noticable performance penalty on
1267+
affected versions.
1268+
12621269
This project also supports running on HHVM.
12631270
Note that really old HHVM < 3.8 does not support secure TLS connections, as it
12641271
lacks the required `stream_socket_enable_crypto()` function.

composer.json

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@
88
"evenement/evenement": "^3.0 || ^2.0 || ^1.0",
99
"react/dns": "^0.4.11",
1010
"react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
11-
"react/stream": "^1.0 || ^0.7 || ^0.6 || ^0.5 || ^0.4.5",
11+
"react/stream": "^1.0 || ^0.7.1",
1212
"react/promise": "^2.1 || ^1.2",
1313
"react/promise-timer": "~1.0"
1414
},
1515
"require-dev": {
1616
"clue/block-react": "^1.1",
17-
"phpunit/phpunit": "~4.8",
18-
"react/stream": "^1.0 || ^0.7 || ^0.6"
17+
"phpunit/phpunit": "~4.8"
1918
},
2019
"autoload": {
2120
"psr-4": {

src/Connection.php

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
use Evenement\EventEmitter;
66
use React\EventLoop\LoopInterface;
77
use React\Stream\DuplexResourceStream;
8-
use React\Stream\Stream;
98
use React\Stream\Util;
109
use React\Stream\WritableStreamInterface;
10+
use React\Stream\WritableResourceStream;
1111

1212
/**
1313
* The actual connection implementation for ConnectionInterface
@@ -50,20 +50,24 @@ public function __construct($resource, LoopInterface $loop)
5050
// See https://bugs.php.net/bug.php?id=65137
5151
// https://bugs.php.net/bug.php?id=41631
5252
// https://github.com/reactphp/socket-client/issues/24
53-
$clearCompleteBuffer = (version_compare(PHP_VERSION, '5.6.8', '<'));
54-
55-
// @codeCoverageIgnoreStart
56-
if (class_exists('React\Stream\Stream')) {
57-
// legacy react/stream < 0.7 requires additional buffer property
58-
$this->input = new Stream($resource, $loop);
59-
if ($clearCompleteBuffer) {
60-
$this->input->bufferSize = null;
61-
}
62-
} else {
63-
// preferred react/stream >= 0.7 accepts buffer parameter
64-
$this->input = new DuplexResourceStream($resource, $loop, $clearCompleteBuffer ? -1 : null);
65-
}
66-
// @codeCoverageIgnoreEnd
53+
$clearCompleteBuffer = PHP_VERSION_ID < 50608;
54+
55+
// PHP < 7.1.4 (and PHP < 7.0.18) suffers from a bug when writing big
56+
// chunks of data over TLS streams at once.
57+
// We try to work around this by limiting the write chunk size to 8192
58+
// bytes for older PHP versions only.
59+
// This is only a work-around and has a noticable performance penalty on
60+
// affected versions. Please update your PHP version.
61+
// This applies to all streams because TLS may be enabled later on.
62+
// See https://github.com/reactphp/socket/issues/105
63+
$limitWriteChunks = (PHP_VERSION_ID < 70018 || (PHP_VERSION_ID >= 70100 && PHP_VERSION_ID < 70104));
64+
65+
$this->input = new DuplexResourceStream(
66+
$resource,
67+
$loop,
68+
$clearCompleteBuffer ? -1 : null,
69+
new WritableResourceStream($resource, $loop, null, $limitWriteChunks ? 8192 : null)
70+
);
6771

6872
$this->stream = $resource;
6973

tests/FunctionalSecureServerTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,38 @@ public function testWritesDataInMultipleChunksToConnection()
9898
$this->assertEquals(400000, $received);
9999
}
100100

101+
public function testWritesMoreDataInMultipleChunksToConnection()
102+
{
103+
$loop = Factory::create();
104+
105+
$server = new TcpServer(0, $loop);
106+
$server = new SecureServer($server, $loop, array(
107+
'local_cert' => __DIR__ . '/../examples/localhost.pem'
108+
));
109+
$server->on('connection', $this->expectCallableOnce());
110+
111+
$server->on('connection', function (ConnectionInterface $conn) {
112+
$conn->write(str_repeat('*', 2000000));
113+
});
114+
115+
$connector = new SecureConnector(new TcpConnector($loop), $loop, array(
116+
'verify_peer' => false
117+
));
118+
$promise = $connector->connect($server->getAddress());
119+
120+
$local = Block\await($promise, $loop, self::TIMEOUT);
121+
/* @var $local React\Stream\Stream */
122+
123+
$received = 0;
124+
$local->on('data', function ($chunk) use (&$received) {
125+
$received += strlen($chunk);
126+
});
127+
128+
Block\sleep(self::TIMEOUT, $loop);
129+
130+
$this->assertEquals(2000000, $received);
131+
}
132+
101133
public function testEmitsDataFromConnection()
102134
{
103135
$loop = Factory::create();

0 commit comments

Comments
 (0)