Skip to content

Commit

Permalink
Fail only if not the first write in on-writable watcher
Browse files Browse the repository at this point in the history
  • Loading branch information
trowski committed May 1, 2020
1 parent c84e008 commit a55611a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
1 change: 0 additions & 1 deletion .travis.yml
Expand Up @@ -20,7 +20,6 @@ jobs:
- mv composer.phar /usr/local/bin/composer
allow_failures:
- php: nightly
- os: osx
fast_finish: true

env:
Expand Down
23 changes: 15 additions & 8 deletions lib/ResourceOutputStream.php
Expand Up @@ -60,6 +60,15 @@ public function __construct($stream, int $chunkSize = null)
$resource = &$this->resource;

$this->watcher = Loop::onWritable($stream, static function ($watcher, $stream) use ($writes, &$chunkSize, &$writable, &$resource) {
$firstWrite = true;

// Using error handler to verify that a write of zero bytes was not due an error.
// @see https://github.com/reactphp/stream/pull/150
$error = 0;
\set_error_handler(static function (int $errno) use (&$error) {
$error = $errno;
});

try {
while (!$writes->isEmpty()) {
/** @var Deferred $deferred */
Expand All @@ -75,11 +84,6 @@ public function __construct($stream, int $chunkSize = null)
throw new ClosedException("The stream was closed by the peer");
}

$error = null;
\set_error_handler(static function (int $errno) use (&$error) {
$error = $errno;
});

// Error reporting suppressed since fwrite() emits E_WARNING if the pipe is broken or the buffer is full.
// Use conditional, because PHP doesn't like getting null passed
if ($chunkSize) {
Expand All @@ -88,8 +92,6 @@ public function __construct($stream, int $chunkSize = null)
$written = @\fwrite($stream, $data);
}

\restore_error_handler();

\assert(
$written !== false || \PHP_VERSION_ID >= 70400, // PHP 7.4+ returns false on EPIPE.
"Trying to write on a previously fclose()'d resource. Do NOT manually fclose() resources the still referenced in the loop."
Expand All @@ -105,7 +107,8 @@ public function __construct($stream, int $chunkSize = null)
}

// Broken pipes between processes on macOS/FreeBSD do not detect EOF properly.
if (($written === 0 || $written === false) && $error !== null) {
// fwrite() may write zero bytes on subsequent calls due to the buffer filling again.
if (($written === 0 || $written === false) && $error && $firstWrite) {
$message = "Failed to write to stream";
if ($error = \error_get_last()) {
$message .= \sprintf("; %s", $error["message"]);
Expand All @@ -120,6 +123,8 @@ public function __construct($stream, int $chunkSize = null)
}

$deferred->resolve($written + $previous);

$firstWrite = false;
}
} catch (\Throwable $exception) {
$resource = null;
Expand All @@ -137,6 +142,8 @@ public function __construct($stream, int $chunkSize = null)
if ($writes->isEmpty()) {
Loop::disable($watcher);
}

\restore_error_handler();
}
});

Expand Down
2 changes: 2 additions & 0 deletions test/ResourceOutputStreamTest.php
Expand Up @@ -78,6 +78,8 @@ public function testClosedRemoteSocket()
}

/**
* @requires PHPUnit >= 7
*
* @see https://github.com/reactphp/stream/pull/150
*/
public function testUploadBiggerBlockSecure()
Expand Down

0 comments on commit a55611a

Please sign in to comment.