From 6b2c28e2d72d618bd46b9264f9e348725d7b5f5f Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Tue, 22 Oct 2019 22:47:37 +0200 Subject: [PATCH] Add checks & realpath() usage - New util class "FileSystem" - Add check in admin summary too --- src/Factory/LoggerFactory.php | 13 +++-- src/Module/Admin/Summary.php | 19 +++++-- src/Util/FileSystem.php | 64 ++++++++++++++++++++++ src/Util/Logger/StreamLogger.php | 50 +++-------------- tests/src/Util/Logger/StreamLoggerTest.php | 43 ++++++++++++--- 5 files changed, 129 insertions(+), 60 deletions(-) create mode 100644 src/Util/FileSystem.php diff --git a/src/Factory/LoggerFactory.php b/src/Factory/LoggerFactory.php index b25d358387..d177faf7a9 100644 --- a/src/Factory/LoggerFactory.php +++ b/src/Factory/LoggerFactory.php @@ -6,6 +6,7 @@ use Friendica\Core\Config\Configuration; use Friendica\Core\Logger; use Friendica\Database\Database; use Friendica\Network\HTTPException\InternalServerErrorException; +use Friendica\Util\FileSystem; use Friendica\Util\Introspection; use Friendica\Util\Logger\Monolog\DevelopHandler; use Friendica\Util\Logger\Monolog\IntrospectionProcessor; @@ -51,10 +52,11 @@ class LoggerFactory * @param Database $database The Friendica Database instance * @param Configuration $config The config * @param Profiler $profiler The profiler of the app + * @param FileSystem $fileSystem FileSystem utils * * @return LoggerInterface The PSR-3 compliant logger instance */ - public function create( Database $database, Configuration $config, Profiler $profiler) + public function create(Database $database, Configuration $config, Profiler $profiler, FileSystem $fileSystem) { if (empty($config->get('system', 'debugging', false))) { $logger = new VoidLogger(); @@ -105,7 +107,7 @@ class LoggerFactory // just add a stream in case it's either writable or not file if (!is_file($stream) || is_writable($stream)) { try { - $logger = new StreamLogger($this->channel, $stream, $introspection, $loglevel); + $logger = new StreamLogger($this->channel, $stream, $introspection, $fileSystem, $loglevel); } catch (\Throwable $t) { // No logger ... $logger = new VoidLogger(); @@ -137,13 +139,14 @@ class LoggerFactory * * @param Configuration $config The config * @param Profiler $profiler The profiler of the app + * @param FileSystem $fileSystem FileSystem utils * * @return LoggerInterface The PSR-3 compliant logger instance * * @throws InternalServerErrorException * @throws \Exception */ - public static function createDev(Configuration $config, Profiler $profiler) + public static function createDev(Configuration $config, Profiler $profiler, FileSystem $fileSystem) { $debugging = $config->get('system', 'debugging'); $stream = $config->get('system', 'dlogfile'); @@ -183,7 +186,7 @@ class LoggerFactory case 'stream': default: - $logger = new StreamLogger(self::DEV_CHANNEL, $stream, $introspection, LogLevel::DEBUG); + $logger = new StreamLogger(self::DEV_CHANNEL, $stream, $introspection, $fileSystem, LogLevel::DEBUG); break; } @@ -225,7 +228,7 @@ class LoggerFactory case "5": // legacy ALL case "4": - return LogLevel::DEBUG; + return LogLevel::DEBUG; // default if nothing set default: return $level; diff --git a/src/Module/Admin/Summary.php b/src/Module/Admin/Summary.php index d0bb4347a1..e1952f294b 100644 --- a/src/Module/Admin/Summary.php +++ b/src/Module/Admin/Summary.php @@ -14,6 +14,7 @@ use Friendica\Model\Register; use Friendica\Module\BaseAdminModule; use Friendica\Util\ConfigFileLoader; use Friendica\Util\DateTimeFormat; +use Friendica\Util\FileSystem; use Friendica\Util\Network; class Summary extends BaseAdminModule @@ -76,11 +77,21 @@ class Summary extends BaseAdminModule // Check logfile permission if (Config::get('system', 'debugging')) { - $stream = Config::get('system', 'logfile'); + $file = Config::get('system', 'logfile'); - if (is_file($stream) && - !is_writeable($stream)) { - $warningtext[] = L10n::t('The logfile \'%s\' is not writable. No logging possible', $stream); + /** @var FileSystem $fileSystem */ + $fileSystem = self::getClass(FileSystem::class); + + try { + $stream = $fileSystem->createStream($file); + + if (is_file($stream) && + !is_writeable($stream)) { + $warningtext[] = L10n::t('The logfile \'%s\' is not writable. No logging possible', $stream); + } + + } catch (\Throwable $exception) { + $warningtext[] = L10n::t('The logfile \'%s\' is not usable. No logging possible (error: \'%s\')', $file, $exception->getMessage()); } $stream = Config::get('system', 'dlogfile'); diff --git a/src/Util/FileSystem.php b/src/Util/FileSystem.php new file mode 100644 index 0000000000..33e04788e6 --- /dev/null +++ b/src/Util/FileSystem.php @@ -0,0 +1,64 @@ +errorMessage, $dirname)); + } + + return $dirname; + } elseif (isset($dirname) && is_dir($dirname)) { + return $dirname; + } else { + return ''; + } + } + + public function createStream(string $url) + { + $directory = $this->createDir($url); + set_error_handler([$this, 'customErrorHandler']); + if (!empty($directory)) { + $url = $directory . DIRECTORY_SEPARATOR . pathinfo($url, PATHINFO_BASENAME); + } + + $stream = fopen($url, 'ab'); + restore_error_handler(); + + if (!is_resource($stream)) { + throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened: ' . $this->errorMessage, $url)); + } + + return $stream; + } + + private function customErrorHandler($code, $msg) + { + $this->errorMessage = preg_replace('{^(fopen|mkdir)\(.*?\): }', '', $msg); + } +} diff --git a/src/Util/Logger/StreamLogger.php b/src/Util/Logger/StreamLogger.php index c9c245d63b..600da14fc2 100644 --- a/src/Util/Logger/StreamLogger.php +++ b/src/Util/Logger/StreamLogger.php @@ -3,6 +3,7 @@ namespace Friendica\Util\Logger; use Friendica\Util\DateTimeFormat; +use Friendica\Util\FileSystem; use Friendica\Util\Introspection; use Psr\Log\LogLevel; @@ -35,11 +36,11 @@ class StreamLogger extends AbstractLogger */ private $pid; + /** - * An error message - * @var string + * @var FileSystem */ - private $errorMessage; + private $fileSystem; /** * Translates LogLevel log levels to integer values @@ -63,8 +64,10 @@ class StreamLogger extends AbstractLogger * * @throws \Exception */ - public function __construct($channel, $stream, Introspection $introspection, $level = LogLevel::DEBUG) + public function __construct($channel, $stream, Introspection $introspection, FileSystem $fileSystem, $level = LogLevel::DEBUG) { + $this->fileSystem = $fileSystem; + parent::__construct($channel, $introspection); if (is_resource($stream)) { @@ -157,43 +160,6 @@ class StreamLogger extends AbstractLogger throw new \LogicException('Missing stream URL.'); } - $this->createDir(); - set_error_handler([$this, 'customErrorHandler']); - $this->stream = fopen($this->url, 'ab'); - restore_error_handler(); - - if (!is_resource($this->stream)) { - $this->stream = null; - - throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened: ' . $this->errorMessage, $this->url)); - } - } - - private function createDir() - { - $dirname = null; - $pos = strpos($this->url, '://'); - if (!$pos) { - $dirname = dirname($this->url); - } - - if (substr($this->url, 0, 7) === 'file://') { - $dirname = dirname(substr($this->url, 7)); - } - - if (isset($dirname) && !is_dir($dirname)) { - set_error_handler([$this, 'customErrorHandler']); - $status = mkdir($dirname, 0777, true); - restore_error_handler(); - - if (!$status && !is_dir($dirname)) { - throw new \UnexpectedValueException(sprintf('Directory "%s" cannot get created: ' . $this->errorMessage, $dirname)); - } - } - } - - private function customErrorHandler($code, $msg) - { - $this->errorMessage = preg_replace('{^(fopen|mkdir)\(.*?\): }', '', $msg); + $this->stream = $this->fileSystem->createStream($this->url); } } diff --git a/tests/src/Util/Logger/StreamLoggerTest.php b/tests/src/Util/Logger/StreamLoggerTest.php index d42ba1d914..7dcb08ba6d 100644 --- a/tests/src/Util/Logger/StreamLoggerTest.php +++ b/tests/src/Util/Logger/StreamLoggerTest.php @@ -2,6 +2,7 @@ namespace Friendica\Test\src\Util\Logger; +use Friendica\Util\FileSystem; use Friendica\Test\Util\VFSTrait; use Friendica\Util\Logger\StreamLogger; use org\bovigo\vfs\vfsStream; @@ -22,11 +23,18 @@ class StreamLoggerTest extends AbstractLoggerTest */ private $logfile; + /** + * @var Filesystem + */ + private $fileSystem; + protected function setUp() { parent::setUp(); $this->setUpVfsDir(); + + $this->fileSystem = new Filesystem(); } /** @@ -37,7 +45,7 @@ class StreamLoggerTest extends AbstractLoggerTest $this->logfile = vfsStream::newFile('friendica.log') ->at($this->root); - $this->logger = new StreamLogger('test', $this->logfile->url(), $this->introspection, $level); + $this->logger = new StreamLogger('test', $this->logfile->url(), $this->introspection, $this->fileSystem, $level); return $this->logger; } @@ -60,7 +68,7 @@ class StreamLoggerTest extends AbstractLoggerTest $filehandler = fopen($logfile->url(), 'ab'); - $logger = new StreamLogger('test', $filehandler, $this->introspection); + $logger = new StreamLogger('test', $filehandler, $this->introspection, $this->fileSystem); $logger->emergency('working'); $text = $logfile->getContent(); @@ -76,7 +84,7 @@ class StreamLoggerTest extends AbstractLoggerTest $logfile = vfsStream::newFile('friendica.log') ->at($this->root); - $logger = new StreamLogger('test', $logfile->url(), $this->introspection); + $logger = new StreamLogger('test', $logfile->url(), $this->introspection, $this->fileSystem); $logger->emergency('working'); $logger->close(); // close doesn't affect @@ -94,7 +102,7 @@ class StreamLoggerTest extends AbstractLoggerTest */ public function testNoUrl() { - $logger = new StreamLogger('test', '', $this->introspection); + $logger = new StreamLogger('test', '', $this->introspection, $this->fileSystem); $logger->emergency('not working'); } @@ -109,7 +117,7 @@ class StreamLoggerTest extends AbstractLoggerTest $logfile = vfsStream::newFile('friendica.log') ->at($this->root)->chmod(0); - $logger = new StreamLogger('test', $logfile->url(), $this->introspection); + $logger = new StreamLogger('test', $logfile->url(), $this->introspection, $this->fileSystem); $logger->emergency('not working'); } @@ -123,7 +131,7 @@ class StreamLoggerTest extends AbstractLoggerTest { $this->markTestIncomplete('We need a platform independent way to set directory to readonly'); - $logger = new StreamLogger('test', '/$%/wrong/directory/file.txt', $this->introspection); + $logger = new StreamLogger('test', '/$%/wrong/directory/file.txt', $this->introspection, $this->fileSystem); $logger->emergency('not working'); } @@ -135,7 +143,7 @@ class StreamLoggerTest extends AbstractLoggerTest */ public function testWrongMinimumLevel() { - $logger = new StreamLogger('test', 'file.text', $this->introspection, 'NOPE'); + $logger = new StreamLogger('test', 'file.text', $this->introspection, $this->fileSystem, 'NOPE'); } /** @@ -148,7 +156,7 @@ class StreamLoggerTest extends AbstractLoggerTest $logfile = vfsStream::newFile('friendica.log') ->at($this->root); - $logger = new StreamLogger('test', $logfile->url(), $this->introspection); + $logger = new StreamLogger('test', $logfile->url(), $this->introspection, $this->fileSystem); $logger->log('NOPE', 'a test'); } @@ -160,6 +168,23 @@ class StreamLoggerTest extends AbstractLoggerTest */ public function testWrongFile() { - $logger = new StreamLogger('test', null, $this->introspection); + $logger = new StreamLogger('test', null, $this->introspection, $this->fileSystem); + } + + /** + * Test a relative path + */ + public function testRealPath() + { + $this->markTestSkipped('vfsStream isn\'t compatible with chdir, so not testable.'); + + $logfile = vfsStream::newFile('friendica.log') + ->at($this->root); + + chdir($this->root->getChild('logs')->url()); + + $logger = new StreamLogger('test', '../friendica.log' , $this->introspection, $this->fileSystem); + + $logger->info('Test'); } } -- 2.39.5