Skip to content

Fix StreamHandler truncating writes on non-blocking streamsFix partial writes on non-blocking streams #2016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rushik2204 wants to merge 1 commit into
base: main
Choose a base branch
from
+95 −145
Open

Fix StreamHandler truncating writes on non-blocking streamsFix partial writes on non-blocking streams #2016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
220 changes: 75 additions & 145 deletions src/Monolog/Handler/StreamHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,55 +19,49 @@
* Stores to any stream resource
*
* Can be used to store into php://stderr, remote and local files, etc.
*
* @author Jordi Boggiano <[email protected]>
*/
class StreamHandler extends AbstractProcessingHandler
{
protected const MAX_CHUNK_SIZE = 2147483647;
/** 10MB */
protected const DEFAULT_CHUNK_SIZE = 10 * 1024 * 1024;

protected int $streamChunkSize;
/** @var resource|null */
protected $stream;
protected string|null $url = null;
private string|null $errorMessage = null;
protected int|null $filePermission;
protected ?string $url = null;
private ?string $errorMessage = null;
protected ?int $filePermission;
protected bool $useLocking;
protected string $fileOpenMode;
/** @var true|null */
private bool|null $dirCreated = null;
private ?bool $dirCreated = null;
private bool $retrying = false;
private int|null $inodeUrl = null;

/**
* @param resource|string $stream If a missing path can't be created, an UnexpectedValueException will be thrown on first write
* @param int|null $filePermission Optional file permissions (default (0644) are only for owner read/write)
* @param bool $useLocking Try to lock log file before doing any writes
* @param string $fileOpenMode The fopen() mode used when opening a file, if $stream is a file path
*
* @throws \InvalidArgumentException If stream is not a resource or string
*/
public function __construct($stream, int|string|Level $level = Level::Debug, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false, string $fileOpenMode = 'a')
{
private ?int $inodeUrl = null;

public function __construct(
$stream,
int|string|Level $level = Level::Debug,
bool $bubble = true,
?int $filePermission = null,
bool $useLocking = false,
string $fileOpenMode = 'a'
) {
parent::__construct($level, $bubble);

if (($phpMemoryLimit = Utils::expandIniShorthandBytes(\ini_get('memory_limit'))) !== false) {
if ($phpMemoryLimit > 0) {
// use max 10% of allowed memory for the chunk size, and at least 100KB
$this->streamChunkSize = min(static::MAX_CHUNK_SIZE, max((int) ($phpMemoryLimit / 10), 100 * 1024));
$this->streamChunkSize = min(
self::MAX_CHUNK_SIZE,
max((int) ($phpMemoryLimit / 10), 100 * 1024)
);
} else {
// memory is unlimited, set to the default 10MB
$this->streamChunkSize = static::DEFAULT_CHUNK_SIZE;
$this->streamChunkSize = self::DEFAULT_CHUNK_SIZE;
}
} else {
// no memory limit information, set to the default 10MB
$this->streamChunkSize = static::DEFAULT_CHUNK_SIZE;
$this->streamChunkSize = self::DEFAULT_CHUNK_SIZE;
}

if (\is_resource($stream)) {
$this->stream = $stream;

stream_set_chunk_size($this->stream, $this->streamChunkSize);
} elseif (\is_string($stream)) {
$this->url = Utils::canonicalizePath($stream);
Expand All @@ -80,143 +74,100 @@ public function __construct($stream, int|string|Level $level = Level::Debug, boo
$this->useLocking = $useLocking;
}

/**
* @inheritDoc
*/
public function reset(): void
{
parent::reset();

// auto-close on reset to make sure we periodically close the file in long running processes
// as long as they correctly call reset() between jobs
if ($this->url !== null && $this->url !== 'php://memory') {
$this->close();
}
}

/**
* @inheritDoc
*/
public function close(): void
{
if (null !== $this->url && \is_resource($this->stream)) {
fclose($this->stream);
}
$this->stream = null;
$this->dirCreated = null;
}

/**
* Return the currently active stream if it is open
*
* @return resource|null
*/
public function getStream()
{
return $this->stream;
}

/**
* Return the stream URL if it was configured with a URL and not an active resource
*/
public function getUrl(): ?string
{
return $this->url;
}

public function getStreamChunkSize(): int
{
return $this->streamChunkSize;
}

/**
* @inheritDoc
*/
protected function write(LogRecord $record): void
{
if ($this->hasUrlInodeWasChanged()) {
$this->close();
$this->write($record);

return;
}

if (!\is_resource($this->stream)) {
$url = $this->url;
if (null === $url || '' === $url) {
throw new \LogicException('Missing stream url, the stream can not be opened. This may be caused by a premature call to close().' . Utils::getRecordMessageForException($record));

if ($url === null || $url === '') {
throw new \LogicException(
'Missing stream url, the stream can not be opened.'
. Utils::getRecordMessageForException($record)
);
}

$this->createDir($url);
$this->errorMessage = null;
set_error_handler($this->customErrorHandler(...));

set_error_handler($this->customErrorHandler(...));
try {
$stream = fopen($url, $this->fileOpenMode);
$this->stream = fopen($url, $this->fileOpenMode);
if ($this->filePermission !== null) {
@chmod($url, $this->filePermission);
}
} finally {
restore_error_handler();
}
if (!\is_resource($stream)) {
$this->stream = null;

throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened in append mode: '.$this->errorMessage, $url) . Utils::getRecordMessageForException($record));
if (!\is_resource($this->stream)) {
throw new \UnexpectedValueException(
sprintf(
'The stream or file "%s" could not be opened in append mode: %s',
$url,
$this->errorMessage
)
. Utils::getRecordMessageForException($record)
);
}
stream_set_chunk_size($stream, $this->streamChunkSize);
$this->stream = $stream;
$this->inodeUrl = $this->getInodeFromUrl();

stream_set_chunk_size($this->stream, $this->streamChunkSize);
}

$stream = $this->stream;

if ($this->useLocking) {
// ignoring errors here, there's not much we can do about them
flock($stream, LOCK_EX);
}

$this->errorMessage = null;
set_error_handler($this->customErrorHandler(...));

try {
$this->streamWrite($stream, $record);
// 🔥 FIX: loop on fwrite for non-blocking streams
$formatted = (string) $record->formatted;

$length = strlen($formatted);
$written = 0;

while ($written < $length) {
$result = @fwrite($stream, substr($formatted, $written));

if ($result === false) {
break;
}

$written += $result;
}
} finally {
restore_error_handler();
}
if ($this->errorMessage !== null) {
$error = $this->errorMessage;
// close the resource if possible to reopen it, and retry the failed write
if (!$this->retrying && $this->url !== null && $this->url !== 'php://memory') {
$this->retrying = true;
$this->close();
$this->write($record);

return;
}

throw new \UnexpectedValueException('Writing to the log file failed: '.$error . Utils::getRecordMessageForException($record));
if ($this->errorMessage !== null) {
throw new \UnexpectedValueException(
'Writing to the log file failed: '
. $this->errorMessage
. Utils::getRecordMessageForException($record)
);
}

$this->retrying = false;
if ($this->useLocking) {
flock($stream, LOCK_UN);
}
}

/**
* Write to stream
* Write to stream (kept for BC / overrides)
*
* @param resource $stream
*/
protected function streamWrite($stream, LogRecord $record): void
{
fwrite($stream, (string) $record->formatted);
}

/**
* @return true
*/
private function customErrorHandler(int $code, string $msg): bool
{
$this->errorMessage = preg_replace('{^(fopen|mkdir|fwrite)\(.*?\): }', '', $msg);

return true;
}

Expand All @@ -236,45 +187,24 @@ private function getDirFromStream(string $stream): ?string

private function createDir(string $url): void
{
// Do not try to create dir if it has already been tried.
if (true === $this->dirCreated) {
if ($this->dirCreated === true) {
return;
}

$dir = $this->getDirFromStream($url);
if (null !== $dir && !is_dir($dir)) {
if ($dir !== null && !is_dir($dir)) {
$this->errorMessage = null;
set_error_handler(function (...$args) {
return $this->customErrorHandler(...$args);
});
set_error_handler($this->customErrorHandler(...));
$status = mkdir($dir, 0777, true);
restore_error_handler();
if (false === $status && !is_dir($dir) && strpos((string) $this->errorMessage, 'File exists') === false) {
throw new \UnexpectedValueException(sprintf('There is no existing directory at "%s" and it could not be created: '.$this->errorMessage, $dir));
}
}
$this->dirCreated = true;
}

private function getInodeFromUrl(): ?int
{
if ($this->url === null || str_starts_with($this->url, 'php://')) {
return null;
}

$inode = @fileinode($this->url);

return $inode === false ? null : $inode;
}

private function hasUrlInodeWasChanged(): bool
{
if ($this->inodeUrl === null || $this->retrying || $this->inodeUrl === $this->getInodeFromUrl()) {
return false;
if ($status === false && !is_dir($dir)) {
throw new \UnexpectedValueException(
sprintf('There is no existing directory at "%s" and it could not be created: %s', $dir, $this->errorMessage)
);
}
}

$this->retrying = true;

return true;
$this->dirCreated = true;
}
}
20 changes: 20 additions & 0 deletions tests/Monolog/Handler/StreamHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,4 +377,24 @@ public function testReopensFileIfInodeChanges()
$data = @file_get_contents($filename);
$this->assertEquals('test2', $data);
}


public function testNonBlockingStreamDoesNotTruncateWrites(): void
{
$stream = fopen('php://memory', 'w+');
stream_set_blocking($stream, false);

$handler = new StreamHandler($stream);
$handler->setFormatter($this->getIdentityFormatter());

$message = str_repeat('1234567890', 100000);
$handler->handle($this->getRecord(Level::Info, $message));

rewind($stream);
$written = stream_get_contents($stream);

$this->assertSame($message, $written);
}


}