Skip to content

Commit

Permalink
Ensure LogStreamHandler only accepts absolute $path arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
@clue
clue committed Feb 28, 2024
1 parent 0068933 commit 4ac1c54
Show file tree
Hide file tree
Showing 2 changed files with 258 additions and 21 deletions.
17 changes: 16 additions & 1 deletion src/Io/LogStreamHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@ class LogStreamHandler
/** @var resource */
private $stream;

/** @throws \RuntimeException if given `$path` can not be opened in append mode */
/**
* @param string $path absolute log file path
* @throws \InvalidArgumentException if given `$path` is not an absolute file path
* @throws \RuntimeException if given `$path` can not be opened in append mode
*/
public function __construct(string $path)
{
if (\strpos($path, "\0") !== false || (\stripos($path, 'php://') !== 0 && !$this->isAbsolutePath($path))) {
throw new \InvalidArgumentException(
'Unable to open log file "' . \addslashes($path) . '": Invalid path given'
);
}

$errstr = '';
\set_error_handler(function (int $_, string $error) use (&$errstr): bool {
// Match errstr from PHP's warning message.
Expand Down Expand Up @@ -42,4 +52,9 @@ public function log(string $message): void
$ret = \fwrite($this->stream, $prefix . $message . \PHP_EOL);
assert(\is_int($ret));
}

private function isAbsolutePath(string $path): bool
{
return \DIRECTORY_SEPARATOR !== '\\' ? \substr($path, 0, 1) === '/' : (bool) \preg_match('#^[A-Z]:[/\\\\]#i', $path);
}
}
262 changes: 242 additions & 20 deletions tests/Io/LogStreamHandlerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,192 @@

class LogStreamHandlerTest extends TestCase
{
public static function provideFilesystemPaths(): \Generator
{
yield [
__FILE__,
true
];
yield [
__FILE__ . "\0",
false
];
yield [
str_replace(DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR . DIRECTORY_SEPARATOR, __FILE__),
true
];
yield [
str_replace(DIRECTORY_SEPARATOR, DIRECTORY_SEPARATOR === '\\' ? '/' : '\\', __FILE__),
DIRECTORY_SEPARATOR === '\\'
];

yield [
'access.log',
false
];
yield [
'./access.log',
false
];
yield [
'../access.log',
false
];
yield [
'.\\access.log',
false
];
yield [
'..\\access.log',
false
];
yield [
'\\\\access.log',
false
];
if (DIRECTORY_SEPARATOR === '\\') {
// invalid paths on Windows, technically valid on Unix but unlikely to be writable here
yield [
'/access.log',
false
];
yield [
'//access.log',
false
];
}

yield [
'',
false
];
yield [
'.',
false
];
yield [
'..',
false
];
yield [
__DIR__ . DIRECTORY_SEPARATOR . "\0",
false
];

yield [
'/dev/null',
DIRECTORY_SEPARATOR !== '\\'
];
yield [
'nul',
false
];
yield [
'\\\\.\\nul',
false
];
if (DIRECTORY_SEPARATOR === '\\') {
// valid path on Windows, but we don't want to write here on Unix
yield [
__DIR__ . DIRECTORY_SEPARATOR . 'nul',
true
];
yield [
__DIR__ . DIRECTORY_SEPARATOR . 'NUL',
true
];
}

yield [
'php://stdout',
true
];
yield [
'PHP://STDOUT',
true
];
yield [
'php:stdout',
false
];

yield [
'php://stderr',
true
];
yield [
'PHP://STDERR',
true
];
yield [
'php:stderr',
false
];
}

public static function provideValidPaths(): \Generator
{
foreach (self::provideFilesystemPaths() as [$path, $valid]) {
if ($valid) {
yield [$path];
}
}
}

/**
* @dataProvider providevalidPaths
* @doesNotPerformAssertions
*/
public function testCtorWithValidPathWorks(string $path): void
{
new LogStreamHandler($path);
}

public static function provideInvalidPaths(): \Generator
{
foreach (self::provideFilesystemPaths() as [$path, $valid]) {
if (!$valid) {
yield [$path];
}
}
}

/**
* @dataProvider provideInvalidPaths
*/
public function testCtorWithInvalidPathThrows(string $path): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Unable to open log file "' . addslashes($path) . '": Invalid path given');
new LogStreamHandler($path);
}

public function testCtorWithDirectoryInsteadOfFileThrowsWithoutCallingGlobalErrorHandler(): void
{
$called = 0;
set_error_handler($new = function () use (&$called): bool {
++$called;
return false;
});

try {
try {
new LogStreamHandler(__DIR__);
} finally {
$previous = set_error_handler(function (): bool { return false; });
restore_error_handler();
restore_error_handler();
}
$this->fail();
} catch (\RuntimeException $e) {
$errstr = DIRECTORY_SEPARATOR === '\\' ? 'Permission denied' : 'Is a directory';
$this->assertEquals('Unable to open log file "' . __DIR__ . '": ' . $errstr, $e->getMessage());

$this->assertEquals(0, $called);
$this->assertSame($new, $previous ?? null);
}
}

public function testLogWithMemoryStreamWritesMessageWithCurrentDateAndTime(): void
{
$logger = new LogStreamHandler('php://memory');
Expand Down Expand Up @@ -40,29 +226,65 @@ public function testLogWithOutputStreamPrintsMessageWithCurrentDateAndTime(): vo
$logger->log('Hello');
}

public function testCtorWithDirectoryInsteadOfFileThrowsWithoutCallingGlobalErrorHandler(): void
public function testLogWithPathToNewFileWillCreateNewFileWithLogMessageAndCurrentDateAndTime(): void
{
$called = 0;
set_error_handler($new = function () use (&$called): bool {
++$called;
return false;
});
$path = tempnam(sys_get_temp_dir(), 'log');
assert(is_string($path));
unlink($path);

try {
try {
new LogStreamHandler(__DIR__);
} finally {
$previous = set_error_handler(function (): bool { return false; });
restore_error_handler();
restore_error_handler();
}
$this->fail();
} catch (\RuntimeException $e) {
$errstr = DIRECTORY_SEPARATOR === '\\' ? 'Permission denied' : 'Is a directory';
$this->assertEquals('Unable to open log file "' . __DIR__ . '": ' . $errstr, $e->getMessage());
$logger = new LogStreamHandler($path);

$this->assertEquals(0, $called);
$this->assertSame($new, $previous ?? null);
$logger->log('Hello');

$output = file_get_contents($path);
assert(is_string($output));

// 2021-01-29 12:22:01.717 Hello\n
if (method_exists($this, 'assertMatchesRegularExpression')) {
$this->assertMatchesRegularExpression("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); // @phpstan-ignore-line
} else {
// legacy PHPUnit < 9.1
$this->assertRegExp("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output);
}

unset($logger);
unlink($path);
}

public function testLogWithPathToExistingFileWillAppendLogMessageWithCurrentDateAndTime(): void
{
$stream = tmpfile();
assert(is_resource($stream));
fwrite($stream, 'First' . PHP_EOL);

$meta = stream_get_meta_data($stream);
assert(is_string($meta['uri']));

$logger = new LogStreamHandler($meta['uri']);

$logger->log('Hello');

rewind($stream);
$output = stream_get_contents($stream);
assert(is_string($output));

// First\n
// 2021-01-29 12:22:01.717 Hello\n
if (method_exists($this, 'assertMatchesRegularExpression')) {
$this->assertMatchesRegularExpression("/^First" . PHP_EOL . "\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output); // @phpstan-ignore-line
} else {
// legacy PHPUnit < 9.1
$this->assertRegExp("/^First" . PHP_EOL . "\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} Hello" . PHP_EOL . "$/", $output);
}
}

/**
* @doesNotPerformAssertions
*/
public function testLogWithDevNullWritesNothing(): void
{
$logger = new LogStreamHandler(DIRECTORY_SEPARATOR !== '\\' ? '/dev/null' : __DIR__ . '\\nul');

$logger->log('Hello');
}
}

0 comments on commit 4ac1c54

Please sign in to comment.