Skip to content

Commit 4102012

Browse files
committed
Move legacy buffer work-around to Connection constructor
1 parent 0d205e9 commit 4102012

File tree

3 files changed

+23
-19
lines changed

3 files changed

+23
-19
lines changed

README.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,8 +1237,11 @@ options as described above.
12371237
All versions of PHP prior to 5.6.8 suffered from a buffering issue where reading
12381238
from a streaming TLS connection could be one `data` event behind.
12391239
This library implements a work-around to try to flush the complete incoming
1240-
data buffers on these versions, but we have seen reports of people saying this
1241-
could still affect some older versions (`5.5.23`, `5.6.7`, and `5.6.8`).
1240+
data buffers on these legacy PHP versions, which has a penalty of around 10% of
1241+
throughput on all connections.
1242+
With this work-around, we have not been able to reproduce this issue anymore,
1243+
but we have seen reports of people saying this could still affect some of the
1244+
older PHP versions (`5.5.23`, `5.6.7`, and `5.6.8`).
12421245
Note that this only affects *some* higher-level streaming protocols, such as
12431246
IRC over TLS, but should not affect HTTP over TLS (HTTPS).
12441247
Further investigation of this issue is needed.

src/Connection.php

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

55
use React\Stream\Stream;
6+
use React\EventLoop\LoopInterface;
67

78
/**
89
* The actual connection implementation for ConnectionInterface
@@ -24,6 +25,22 @@ class Connection extends Stream implements ConnectionInterface
2425
*/
2526
public $encryptionEnabled = false;
2627

28+
public function __construct($stream, LoopInterface $loop)
29+
{
30+
parent::__construct($stream, $loop);
31+
32+
// PHP < 5.6.8 suffers from a buffer indicator bug on secure TLS connections
33+
// as a work-around we always read the complete buffer until its end.
34+
// The buffer size is limited due to TCP/IP buffers anyway, so this
35+
// should not affect usage otherwise.
36+
// See https://bugs.php.net/bug.php?id=65137
37+
// https://bugs.php.net/bug.php?id=41631
38+
// https://github.com/reactphp/socket-client/issues/24
39+
if (version_compare(PHP_VERSION, '5.6.8', '<')) {
40+
$this->bufferSize = null;
41+
}
42+
}
43+
2744
public function handleClose()
2845
{
2946
if (!is_resource($this->stream)) {

src/StreamEncryption.php

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,22 +21,11 @@ class StreamEncryption
2121
private $errstr;
2222
private $errno;
2323

24-
private $wrapSecure = false;
25-
2624
public function __construct(LoopInterface $loop, $server = true)
2725
{
2826
$this->loop = $loop;
2927
$this->server = $server;
3028

31-
// See https://bugs.php.net/bug.php?id=65137
32-
// https://bugs.php.net/bug.php?id=41631
33-
// https://github.com/reactphp/socket-client/issues/24
34-
// On versions affected by this bug we need to fread the stream until we
35-
// get an empty string back because the buffer indicator could be wrong
36-
if (version_compare(PHP_VERSION, '5.6.8', '<')) {
37-
$this->wrapSecure = true;
38-
}
39-
4029
if ($server) {
4130
$this->method = STREAM_CRYPTO_METHOD_TLS_SERVER;
4231

@@ -100,16 +89,11 @@ public function toggle(Connection $stream, $toggle)
10089
$toggleCrypto();
10190
}
10291

103-
$wrap = $this->wrapSecure && $toggle;
10492
$loop = $this->loop;
10593

106-
return $deferred->promise()->then(function () use ($stream, $socket, $wrap, $loop, $toggle) {
94+
return $deferred->promise()->then(function () use ($stream, $socket, $loop, $toggle) {
10795
$loop->removeReadStream($socket);
10896

109-
if ($wrap) {
110-
$stream->bufferSize = null;
111-
}
112-
11397
$stream->encryptionEnabled = $toggle;
11498
$stream->resume();
11599

0 commit comments

Comments
 (0)