From 585d900e696a28eceb5bd4aa1bef5d62216f0cab Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Mon, 18 May 2020 12:29:36 -0500 Subject: [PATCH] Reduce max empty writes to 1 The root cause of the write failures on some platforms seems to be EAGAIN, which should not occur consecutively in the watcher callback. --- .gitignore | 1 + .travis.yml | 11 +++++++-- lib/ResourceOutputStream.php | 4 ++-- test/ResourceOutputStreamTest.php | 40 +++++++++++++++++++++++++++---- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index b93909d..84996f1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .php_cs.cache +.phpunit.result.cache build coverage composer.lock diff --git a/.travis.yml b/.travis.yml index 5bc6d1c..9901bd9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,4 @@ -sudo: false +os: linux language: php @@ -10,7 +10,14 @@ php: - 7.4 - nightly -matrix: +jobs: + include: + - name: macOS + os: osx + language: generic + before_install: + - curl -s http://getcomposer.org/installer | php + - mv composer.phar /usr/local/bin/composer allow_failures: - php: nightly fast_finish: true diff --git a/lib/ResourceOutputStream.php b/lib/ResourceOutputStream.php index 8bf3fdc..f8ebc2c 100644 --- a/lib/ResourceOutputStream.php +++ b/lib/ResourceOutputStream.php @@ -13,7 +13,7 @@ */ final class ResourceOutputStream implements OutputStream { - const MAX_CONSECUTIVE_EMPTY_WRITES = 3; + const MAX_CONSECUTIVE_EMPTY_WRITES = 1; // EAGAIN should not occur on next watcher invocation. const LARGE_CHUNK_SIZE = 128 * 1024; /** @var resource|null */ @@ -100,7 +100,7 @@ public function __construct($stream, int $chunkSize = null) // Broken pipes between processes on macOS/FreeBSD do not detect EOF properly. if ($written === 0 || $written === false) { if ($emptyWrites++ > self::MAX_CONSECUTIVE_EMPTY_WRITES) { - $message = "Failed to write to stream after multiple attempts"; + $message = "Failed to write to stream"; if ($error = \error_get_last()) { $message .= \sprintf("; %s", $error["message"]); } diff --git a/test/ResourceOutputStreamTest.php b/test/ResourceOutputStreamTest.php index a8bf7b1..054c270 100644 --- a/test/ResourceOutputStreamTest.php +++ b/test/ResourceOutputStreamTest.php @@ -2,8 +2,10 @@ namespace Amp\ByteStream\Test; +use Amp\ByteStream\ResourceInputStream; use Amp\ByteStream\ResourceOutputStream; use Amp\ByteStream\StreamException; +use Amp\Delayed; use Amp\PHPUnit\AsyncTestCase; class ResourceOutputStreamTest extends AsyncTestCase @@ -34,10 +36,10 @@ public function testNotWritable() public function testBrokenPipe() { if (($sockets = @\stream_socket_pair( - \stripos(PHP_OS, "win") === 0 ? STREAM_PF_INET : STREAM_PF_UNIX, - STREAM_SOCK_STREAM, - STREAM_IPPROTO_IP - )) === false) { + \stripos(PHP_OS, "win") === 0 ? STREAM_PF_INET : STREAM_PF_UNIX, + STREAM_SOCK_STREAM, + STREAM_IPPROTO_IP + )) === false) { $this->fail("Failed to create socket pair."); } @@ -68,6 +70,36 @@ public function testClosedRemoteSocket() // The first write still succeeds somehow... yield $stream->write("foobar"); + + // A delay seems required for the OS to realize the socket is indeed closed. + yield new Delayed(10); + yield $stream->write("foobar"); } + + /** + * @requires PHPUnit >= 7 + * + * @see https://github.com/reactphp/stream/pull/150 + */ + public function testUploadBiggerBlockSecure() + { + $size = 2 ** 18; // 256kb + + $resource = \stream_socket_client('tls://httpbin.org:443'); + + $output = new ResourceOutputStream($resource); + + $body = \str_repeat('.', $size); + + yield $output->write("POST /post HTTP/1.0\r\nHost: httpbin.org\r\nContent-Length: $size\r\n\r\n" . $body); + + $input = new ResourceInputStream($resource); + $buffer = ''; + while (null !== ($chunk = yield $input->read())) { + $buffer .= $chunk; + } + + $this->assertStringContainsString($body, $buffer); + } }