Merge pull request #7763 from nupplaphil/bug/7756_log_directory

Add Fallback in case the logfile isn't accessible.

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,13 +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
-	 *
-	 * @throws \Exception
-	 * @throws InternalServerErrorException
 	 */
-	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();
@@ -84,12 +83,22 @@ class LoggerFactory
 
 				// just add a stream in case it's either writable or not file
 				if (!is_file($stream) || is_writable($stream)) {
-					static::addStreamHandler($logger, $stream, $loglevel);
+					try {
+						static::addStreamHandler($logger, $stream, $loglevel);
+					} catch (\Throwable $e) {
+						// No Logger ..
+						$logger = new VoidLogger();
+					}
 				}
 				break;
 
 			case 'syslog':
-				$logger = new SyslogLogger($this->channel, $introspection, $loglevel);
+				try {
+					$logger = new SyslogLogger($this->channel, $introspection, $loglevel);
+				} catch (\Throwable $e) {
+					// No logger ...
+					$logger = new VoidLogger();
+				}
 				break;
 
 			case 'stream':
@@ -97,7 +106,12 @@ class LoggerFactory
 				$stream = $config->get('system', 'logfile');
 				// just add a stream in case it's either writable or not file
 				if (!is_file($stream) || is_writable($stream)) {
-					$logger = new StreamLogger($this->channel, $stream, $introspection, $loglevel);
+					try {
+						$logger = new StreamLogger($this->channel, $stream, $introspection, $fileSystem, $loglevel);
+					} catch (\Throwable $t) {
+						// No logger ...
+						$logger = new VoidLogger();
+					}
 				} else {
 					$logger = new VoidLogger();
 				}
@@ -125,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');
@@ -171,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;
 		}
 
@@ -211,7 +226,6 @@ class LoggerFactory
 				return LogLevel::INFO;
 			// legacy DATA
 			case "4":
-				return LogLevel::DEBUG;
 			// legacy ALL
 			case "5":
 				return LogLevel::DEBUG;
@@ -230,7 +244,6 @@ class LoggerFactory
 	 *
 	 * @return void
 	 *
-	 * @throws InternalServerErrorException if the logger is incompatible to the logger factory
 	 * @throws \Exception in case of general failures
 	 */
 	public static function addStreamHandler($logger, $stream, $level = LogLevel::NOTICE)
@@ -249,8 +262,6 @@ class LoggerFactory
 			$fileHandler->setFormatter($formatter);
 
 			$logger->pushHandler($fileHandler);
-		} else {
-			throw new InternalServerErrorException('Logger instance incompatible for MonologFactory');
 		}
 	}
 

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) &&
+			/** @var FileSystem $fileSystem */
-			    !is_writeable($stream)) {
+			$fileSystem = self::getClass(FileSystem::class);
-				$warningtext[] = L10n::t('The logfile \'%s\' is not writable. No logging possible', $stream);
+
+			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');

src/Util/FileSystem.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace Friendica\Util;
+
+/**
+ * Util class for filesystem manipulation
+ */
+final class FileSystem
+{
+	/**
+	 * @var string a error message
+	 */
+	private $errorMessage;
+
+	/**
+	 * Creates a directory based on a file, which gets accessed
+	 *
+	 * @param string $file The file
+	 *
+	 * @return string The directory name (empty if no directory is found, like urls)
+	 */
+	public function createDir(string $file)
+	{
+		$dirname = null;
+		$pos = strpos($file, '://');
+
+		if (!$pos) {
+			$dirname = realpath(dirname($file));
+		}
+
+		if (substr($file, 0, 7) === 'file://') {
+			$dirname = realpath(dirname(substr($file, 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));
+			}
+
+			return $dirname;
+		} elseif (isset($dirname) && is_dir($dirname)) {
+			return $dirname;
+		} else {
+			return '';
+		}
+	}
+
+	/**
+	 * Creates a stream based on a URL (could be a local file or a real URL)
+	 *
+	 * @param string $url The file/url
+	 *
+	 * @return false|resource the open stream ressource
+	 */
+	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);
+	}
+}

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;
 
@@ -36,10 +37,9 @@ class StreamLogger extends AbstractLogger
 	private $pid;
 
 	/**
-	 * An error message
+	 * @var FileSystem
-	 * @var string
 	 */
-	private $errorMessage;
+	private $fileSystem;
 
 	/**
 	 * Translates LogLevel log levels to integer values
@@ -63,8 +63,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)) {
@@ -81,6 +83,8 @@ class StreamLogger extends AbstractLogger
 		} else {
 			throw new \InvalidArgumentException(sprintf('The level "%s" is not valid.', $level));
 		}
+
+		$this->checkStream();
 	}
 
 	public function close()
@@ -155,43 +159,6 @@ class StreamLogger extends AbstractLogger
 			throw new \LogicException('Missing stream URL.');
 		}
 
-		$this->createDir();
+		$this->stream = $this->fileSystem->createStream($this->url);
-		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);
 	}
 }

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');
 	}
 }