Skip to content

Commit

Permalink
Avoid uneeded fragmented TLS work around for PHP 7.3.3+
Browse files Browse the repository at this point in the history
The work around was introduced in order to bring TLS 1.3 support to all
supported versions, but at the same time it may cause very large data
chunks for high throughput scenarios. The underlying bug has been fixed
in PHP 7.3.3 and PHP 7.2.15, so we can avoid this now uneeded work
around on said version.
  • Loading branch information
clue committed May 31, 2019
1 parent 1f92698 commit aa71f9c
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,15 @@ This library does not take responsibility over these context options, so it's
up to consumers of this library to take care of setting appropriate context
options as described above.

PHP < 7.3.3 (and PHP < 7.2.15) suffers from a bug where feof() might
block with 100% CPU usage on fragmented TLS records.
We try to work around this by always consuming the complete receive
buffer at once to avoid stale data in TLS buffers. This is known to
work around high CPU usage for well-behaving peers, but this may
cause very large data chunks for high throughput scenarios. The buggy
behavior can still be triggered due to network I/O buffers or
malicious peers on affected versions, upgrading is highly recommended.

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
Expand Down
17 changes: 12 additions & 5 deletions src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ class Connection extends EventEmitter implements ConnectionInterface

public function __construct($resource, LoopInterface $loop)
{
// PHP < 7.3.3 (and PHP < 7.2.15) suffers from a bug where feof() might
// block with 100% CPU usage on fragmented TLS records.
// We try to work around this by always consuming the complete receive
// buffer at once to avoid stale data in TLS buffers. This is known to
// work around high CPU usage for well-behaving peers, but this may
// cause very large data chunks for high throughput scenarios. The buggy
// behavior can still be triggered due to network I/O buffers or
// malicious peers on affected versions, upgrading is highly recommended.
// @link https://bugs.php.net/bug.php?id=77390
$clearCompleteBuffer = \PHP_VERSION_ID < 70303 || (\PHP_VERSION_ID >= 70200 && \PHP_VERSION_ID < 70215);

// 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
Expand All @@ -53,14 +64,10 @@ public function __construct($resource, LoopInterface $loop)
// See https://github.com/reactphp/socket/issues/105
$limitWriteChunks = (\PHP_VERSION_ID < 70018 || (\PHP_VERSION_ID >= 70100 && \PHP_VERSION_ID < 70104));

// Construct underlying stream to always consume complete receive buffer.
// This avoids stale data in TLS buffers and also works around possible
// buffering issues in legacy PHP versions. The buffer size is limited
// due to TCP/IP buffers anyway, so this should not affect usage otherwise.
$this->input = new DuplexResourceStream(
$resource,
$loop,
-1,
$clearCompleteBuffer ? -1 : null,
new WritableResourceStream($resource, $loop, null, $limitWriteChunks ? 8192 : null)
);

Expand Down

0 comments on commit aa71f9c

Please sign in to comment.