Merge pull request #821 from Zankaria/refactor-log

Break up the log subsystem and move it into Data

compose.yml

@@ -9,7 +9,7 @@ services:
     depends_on:
       - db
     volumes:
-      - ./local-instances/${LOCAL_INSTANCE_NAME:-0}/www:/var/www/html
+      - ./local-instances/${INSTANCE:-0}/www:/var/www/html
       - ./docker/nginx/vichan.conf:/etc/nginx/conf.d/default.conf
       - ./docker/nginx/nginx.conf:/etc/nginx/nginx.conf
       - ./docker/nginx/proxy.conf:/etc/nginx/conf.d/proxy.conf
@@ -21,7 +21,7 @@ services:
       context: .
       dockerfile: ./docker/php/Dockerfile
     volumes:
-      - ./local-instances/${LOCAL_INSTANCE_NAME:-0}/www:/var/www
+      - ./local-instances/${INSTANCE:-0}/www:/var/www
       - ./docker/php/www.conf:/usr/local/etc/php-fpm.d/www.conf
       - ./docker/php/jit.ini:/usr/local/etc/php/conf.d/jit.ini
 
@@ -37,4 +37,4 @@ services:
       MYSQL_DATABASE: vichan
       MYSQL_ROOT_PASSWORD: password
     volumes:
-      - ./local-instances/${LOCAL_INSTANCE_NAME:-0}/mysql:/var/lib/mysql
+      - ./local-instances/${INSTANCE:-0}/mysql:/var/lib/mysql

composer.json

@@ -38,7 +38,6 @@
             "inc/functions/num.php",
             "inc/functions/format.php",
             "inc/functions/theme.php",
-            "inc/driver/log-driver.php",
             "inc/service/captcha-queries.php",
             "inc/context.php"
         ]

inc/Data/Driver/ErrorLogLogDriver.php

@@ -0,0 +1,28 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+
+/**
+ * Log via the php function error_log.
+ */
+class ErrorLogLogDriver implements LogDriver {
+	use LogTrait;
+
+	private string $name;
+	private int $level;
+
+	public function __construct(string $name, int $level) {
+		$this->name = $name;
+		$this->level = $level;
+	}
+
+	public function log(int $level, string $message): void {
+		if ($level <= $this->level) {
+			$lv = $this->levelToString($level);
+			$line = "{$this->name} $lv: $message";
+			\error_log($line, 0, null, null);
+		}
+	}
+}

inc/Data/Driver/FileLogDriver.php

@@ -0,0 +1,60 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+
+/**
+ * Log to a file.
+ */
+class FileLogDriver implements LogDriver {
+	use LogTrait;
+
+	private string $name;
+	private int $level;
+	private mixed $fd;
+
+	public function __construct(string $name, int $level, string $file_path) {
+		/*
+		 * error_log is slow as hell in it's 3rd mode, so use fopen + file locking instead.
+		 * https://grobmeier.solutions/performance-ofnonblocking-write-to-files-via-php-21082009.html
+		 *
+		 * Whatever file appending is atomic is contentious:
+		 *  - There are no POSIX guarantees: https://stackoverflow.com/a/7237901
+		 *  - But linus suggested they are on linux, on some filesystems: https://web.archive.org/web/20151201111541/http://article.gmane.org/gmane.linux.kernel/43445
+		 *  - But it doesn't seem to be always the case: https://www.notthewizard.com/2014/06/17/are-files-appends-really-atomic/
+		 *
+		 * So we just use file locking to be sure.
+		 */
+
+		$this->fd = \fopen($file_path, 'a');
+		if ($this->fd === false) {
+			throw new \RuntimeException("Unable to open log file at $file_path");
+		}
+
+		$this->name = $name;
+		$this->level = $level;
+
+		// In some cases PHP does not run the destructor.
+		\register_shutdown_function([$this, 'close']);
+	}
+
+	public function __destruct() {
+		$this->close();
+	}
+
+	public function log(int $level, string $message): void {
+		if ($level <= $this->level) {
+			$lv = $this->levelToString($level);
+			$line = "{$this->name} $lv: $message\n";
+			\flock($this->fd, LOCK_EX);
+			\fwrite($this->fd, $line);
+			\flock($this->fd, LOCK_UN);
+		}
+	}
+
+	public function close() {
+		\flock($this->fd, LOCK_UN);
+		\fclose($this->fd);
+	}
+}

inc/Data/Driver/LogDriver.php

@@ -0,0 +1,22 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+
+interface LogDriver {
+	public const EMERG = \LOG_EMERG;
+	public const ERROR = \LOG_ERR;
+	public const WARNING = \LOG_WARNING;
+	public const NOTICE = \LOG_NOTICE;
+	public const INFO = \LOG_INFO;
+	public const DEBUG = \LOG_DEBUG;
+
+	/**
+	 * Log a message if the level of relevancy is at least the minimum.
+	 *
+	 * @param int $level Message level. Use Log interface constants.
+	 * @param string $message The message to log.
+	 */
+	public function log(int $level, string $message): void;
+}

inc/Data/Driver/LogTrait.php

@@ -0,0 +1,26 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+
+trait LogTrait {
+	public static function levelToString(int $level): string {
+		switch ($level) {
+			case LogDriver::EMERG:
+				return 'EMERG';
+			case LogDriver::ERROR:
+				return 'ERROR';
+			case LogDriver::WARNING:
+				return 'WARNING';
+			case LogDriver::NOTICE:
+				return 'NOTICE';
+			case LogDriver::INFO:
+				return 'INFO';
+			case LogDriver::DEBUG:
+				return 'DEBUG';
+			default:
+				throw new \InvalidArgumentException('Not a logging level');
+		}
+	}
+}

inc/Data/Driver/StderrLogDriver.php

@@ -0,0 +1,27 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+
+/**
+ * Log to php's standard error file stream.
+ */
+class StderrLogDriver implements LogDriver {
+	use LogTrait;
+
+	private string $name;
+	private int $level;
+
+	public function __construct(string $name, int $level) {
+		$this->name = $name;
+		$this->level = $level;
+	}
+
+	public function log(int $level, string $message): void {
+		if ($level <= $this->level) {
+			$lv = $this->levelToString($level);
+			\fwrite(\STDERR, "{$this->name} $lv: $message\n");
+		}
+	}
+}

inc/Data/Driver/SyslogLogDriver.php

@@ -0,0 +1,35 @@
+<?php
+namespace Vichan\Data\Driver;
+
+defined('TINYBOARD') or exit;
+
+/**
+ * Log to syslog.
+ */
+class SyslogLogDriver implements LogDriver {
+	private int $level;
+
+	public function __construct(string $name, int $level, bool $print_stderr) {
+		$flags = \LOG_ODELAY;
+		if ($print_stderr) {
+			$flags |= \LOG_PERROR;
+		}
+
+		if (!\openlog($name, $flags, \LOG_USER)) {
+			throw new \RuntimeException('Unable to open syslog');
+		}
+
+		$this->level = $level;
+	}
+
+	public function log(int $level, string $message): void {
+		if ($level <= $this->level) {
+			if (isset($_SERVER['REMOTE_ADDR'], $_SERVER['REQUEST_METHOD'], $_SERVER['REQUEST_URI'])) {
+				// CGI
+				\syslog($level, "$message - client: {$_SERVER['REMOTE_ADDR']}, request: \"{$_SERVER['REQUEST_METHOD']} {$_SERVER['REQUEST_URI']}\"");
+			} else {
+				\syslog($level, $message);
+			}
+		}
+	}
+}

inc/config.php

@@ -71,7 +71,7 @@
 	$config['log_system'] = [
 		/*
 		 * Log all error messages and unauthorized login attempts.
-		 * Can be "syslog", "error_log" (default), "file", "stderr" or "none".
+		 * Can be "syslog", "error_log" (default), "file", or "stderr".
 		 */
 		'type' => 'error_log',
 		// The application name used by the logging system. Defaults to "tinyboard" for backwards compatibility.

inc/context.php

@@ -2,8 +2,7 @@
 namespace Vichan;
 
 use RuntimeException;
-use Vichan\Driver\{Log, LogDrivers};
+use Vichan\Data\Driver\{HttpDriver, ErrorLogLogDriver, FileLogDriver, LogDriver, StderrLogDriver, SyslogLogDriver};
-use Vichan\Data\Driver\HttpDriver;
 use Vichan\Service\HCaptchaQuery;
 use Vichan\Service\NativeCaptchaQuery;
 use Vichan\Service\ReCaptchaQuery;
@@ -35,24 +34,22 @@ class Context {
 function build_context(array $config): Context {
 	return new Context([
 		'config' => $config,
-		Log::class => function($c) {
+		LogDriver::class => function($c) {
 			$config = $c->get('config');
 
 			$name = $config['log_system']['name'];
-			$level = $config['debug'] ? Log::DEBUG : Log::NOTICE;
+			$level = $config['debug'] ? LogDriver::DEBUG : LogDriver::NOTICE;
 			$backend = $config['log_system']['type'];
 
 			// Check 'syslog' for backwards compatibility.
 			if ((isset($config['syslog']) && $config['syslog']) || $backend === 'syslog') {
-				return LogDrivers::syslog($name, $level, $this->config['log_system']['syslog_stderr']);
+				return new SyslogLogDriver($name, $level, $this->config['log_system']['syslog_stderr']);
 			} elseif ($backend === 'file') {
-				return LogDrivers::file($name, $level, $this->config['log_system']['file_path']);
+				return new FileLogDriver($name, $level, $this->config['log_system']['file_path']);
 			} elseif ($backend === 'stderr') {
-				return LogDrivers::stderr($name, $level);
+				return new StderrLogDriver($name, $level);
-			} elseif ($backend === 'none') {
-				return LogDrivers::none();
 			} else {
-				return LogDrivers::error_log($name, $level);
+				return new ErrorLogLogDriver($name, $level);
 			}
 		},
 		HttpDriver::class => function($c) {

inc/driver/log-driver.php

@@ -1,189 +0,0 @@
-<?php // Logging
-namespace Vichan\Driver;
-
-use InvalidArgumentException;
-use RuntimeException;
-
-defined('TINYBOARD') or exit;
-
-
-class LogDrivers {
-	public static function levelToString(int $level): string {
-		switch ($level) {
-			case Log::EMERG:
-				return 'EMERG';
-			case Log::ERROR:
-				return 'ERROR';
-			case Log::WARNING:
-				return 'WARNING';
-			case Log::NOTICE:
-				return 'NOTICE';
-			case Log::INFO:
-				return 'INFO';
-			case Log::DEBUG:
-				return 'DEBUG';
-			default:
-				throw new InvalidArgumentException('Not a logging level');
-		}
-	}
-
-	/**
-	 * Log to syslog.
-	 */
-	public static function syslog(string $name, int $level, bool $print_stderr): Log {
-		$flags = LOG_ODELAY;
-		if ($print_stderr) {
-			$flags |= LOG_PERROR;
-		}
-
-		if (!openlog($name, $flags, LOG_USER)) {
-			throw new RuntimeException('Unable to open syslog');
-		}
-
-		return new class($level) implements Log {
-			private $level;
-
-			public function __construct(int $level) {
-				$this->level = $level;
-			}
-
-			public function log(int $level, string $message): void {
-				if ($level <= $this->level) {
-					if (isset($_SERVER['REMOTE_ADDR'], $_SERVER['REQUEST_METHOD'], $_SERVER['REQUEST_URI'])) {
-						// CGI
-						syslog($level, "$message - client: {$_SERVER['REMOTE_ADDR']}, request: \"{$_SERVER['REQUEST_METHOD']} {$_SERVER['REQUEST_URI']}\"");
-					} else {
-						syslog($level, $message);
-					}
-				}
-			}
-		};
-	}
-
-	/**
-	 * Log via the php function error_log.
-	 */
-	public static function error_log(string $name, int $level): Log {
-		return new class($name, $level) implements Log {
-			private string $name;
-			private int $level;
-
-			public function __construct(string $name, int $level) {
-				$this->name = $name;
-				$this->level = $level;
-			}
-
-			public function log(int $level, string $message): void {
-				if ($level <= $this->level) {
-					$lv = LogDrivers::levelToString($level);
-					$line = "{$this->name} $lv: $message";
-					error_log($line, 0, null, null);
-				}
-			}
-		};
-	}
-
-	/**
-	 * Log to a file.
-	 */
-	public static function file(string $name, int $level, string $file_path): Log {
-		/*
-		 * error_log is slow as hell in it's 3rd mode, so use fopen + file locking instead.
-		 * https://grobmeier.solutions/performance-ofnonblocking-write-to-files-via-php-21082009.html
-		 *
-		 * Whatever file appending is atomic is contentious:
-		 *  - There are no POSIX guarantees: https://stackoverflow.com/a/7237901
-		 *  - But linus suggested they are on linux, on some filesystems: https://web.archive.org/web/20151201111541/http://article.gmane.org/gmane.linux.kernel/43445
-		 *  - But it doesn't seem to be always the case: https://www.notthewizard.com/2014/06/17/are-files-appends-really-atomic/
-		 *
-		 * So we just use file locking to be sure.
-		 */
-
-		$fd = fopen($file_path, 'a');
-		if ($fd === false) {
-			throw new RuntimeException("Unable to open log file at $file_path");
-		}
-
-		$logger = new class($name, $level, $fd) implements Log {
-			private string $name;
-			private int $level;
-			private mixed $fd;
-
-			public function __construct(string $name, int $level, mixed $fd) {
-				$this->name = $name;
-				$this->level = $level;
-				$this->fd = $fd;
-			}
-
-			public function log(int $level, string $message): void {
-				if ($level <= $this->level) {
-					$lv = LogDrivers::levelToString($level);
-					$line = "{$this->name} $lv: $message\n";
-					flock($this->fd, LOCK_EX);
-					fwrite($this->fd, $line);
-					flock($this->fd, LOCK_UN);
-				}
-			}
-
-			public function close() {
-				fclose($this->fd);
-			}
-		};
-
-		// Close the file on shutdown.
-		register_shutdown_function([$logger, 'close']);
-
-		return $logger;
-	}
-
-	/**
-	 * Log to php's standard error file stream.
-	 */
-	public static function stderr(string $name, int $level): Log {
-		return new class($name, $level) implements Log {
-			private $name;
-			private $level;
-
-			public function __construct(string $name, int $level) {
-				$this->name = $name;
-				$this->level = $level;
-			}
-
-			public function log(int $level, string $message): void {
-				if ($level <= $this->level) {
-					$lv = LogDrivers::levelToString($level);
-					fwrite(STDERR, "{$this->name} $lv: $message\n");
-				}
-			}
-		};
-	}
-
-	/**
-	 * No-op logging system.
-	 */
-	public static function none(): Log {
-		return new class() implements Log {
-			public function log($level, $message): void {
-				// No-op.
-			}
-		};
-	}
-}
-
-interface Log {
-	public const EMERG = LOG_EMERG;
-	public const ERROR = LOG_ERR;
-	public const WARNING = LOG_WARNING;
-	public const NOTICE = LOG_NOTICE;
-	public const INFO = LOG_INFO;
-	public const DEBUG = LOG_DEBUG;
-
-
-	/**
-	 * Log a message if the level of relevancy is at least the minimum.
-	 *
-	 * @param int $level Message level. Use Log interface constants.
-	 * @param string $message The message to log.
-	 */
-	public function log(int $level, string $message): void;
-}

post.php

@@ -6,7 +6,7 @@
 require_once 'inc/bootstrap.php';
 
 use Vichan\{Context, WebDependencyFactory};
-use Vichan\Data\Driver\{HttpDriver, Log};
+use Vichan\Data\Driver\{LogDriver, HttpDriver};
 use Vichan\Service\{RemoteCaptchaQuery, NativeCaptchaQuery};
 use Vichan\Functions\Format;
 
@@ -286,7 +286,7 @@ if (isset($_GET['Newsgroups']) && $config['nntpchan']['enabled']) {
 		$content = file_get_contents("php://input");
 	}
 	elseif ($ct == 'multipart/mixed' || $ct == 'multipart/form-data') {
-		$context->get(Log::class)->log(Log::DEBUG, 'MM: Files: ' . print_r($GLOBALS, true));
+		$context->get(LogDriver::class)->log(LogDriver::DEBUG, 'MM: Files: ' . print_r($GLOBALS, true));
 
 		$content = '';
 
@@ -454,8 +454,8 @@ if (isset($_POST['delete'])) {
 				modLog("User at $ip deleted their own post #$id");
 			}
 
-			$context->get(Log::class)->log(
+			$context->get(LogDriver::class)->log(
-				Log::INFO,
+				LogDriver::INFO,
 				'Deleted post: /' . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '')
 			);
 		}
@@ -531,7 +531,7 @@ if (isset($_POST['delete'])) {
 				error($config['error']['captcha']);
 			}
 		} catch (RuntimeException $e) {
-			$context->get(Log::class)->log(Log::ERROR, "Native captcha IO exception: {$e->getMessage()}");
+			$context->get(LogDriver::class)->log(LogDriver::ERROR, "Native captcha IO exception: {$e->getMessage()}");
 			error($config['error']['local_io_error']);
 		}
 	}
@@ -550,7 +550,7 @@ if (isset($_POST['delete'])) {
 
 		$post = $query->fetch(PDO::FETCH_ASSOC);
 		if ($post === false) {
-			$context->get(Log::class)->log(Log::INFO, "Failed to report non-existing post #{$id} in {$board['dir']}");
+			$context->get(LogDriver::class)->log(LogDriver::INFO, "Failed to report non-existing post #{$id} in {$board['dir']}");
 			error($config['error']['nopost']);
 		}
 
@@ -559,8 +559,8 @@ if (isset($_POST['delete'])) {
 			error($error);
 		}
 
-		$context->get(Log::class)->log(
+		$context->get(LogDriver::class)->log(
-			Log::INFO,
+			LogDriver::INFO,
 			'Reported post: /'
 				 . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '')
 				 . " for \"$reason\""
@@ -673,10 +673,10 @@ if (isset($_POST['delete'])) {
 				}
 			}
 		} catch (RuntimeException $e) {
-			$context->get(Log::class)->log(Log::ERROR, "Captcha IO exception: {$e->getMessage()}");
+			$context->get(LogDriver::class)->log(LogDriver::ERROR, "Captcha IO exception: {$e->getMessage()}");
 			error($config['error']['remote_io_error']);
 		} catch (JsonException $e) {
-			$context->get(Log::class)->log(Log::ERROR, "Bad JSON reply to captcha: {$e->getMessage()}");
+			$context->get(LogDriver::class)->log(LogDriver::ERROR, "Bad JSON reply to captcha: {$e->getMessage()}");
 			error($config['error']['remote_io_error']);
 		}
 
@@ -1162,7 +1162,7 @@ if (isset($_POST['delete'])) {
 					try {
 						$file['size'] = strip_image_metadata($file['tmp_name']);
 					} catch (RuntimeException $e) {
-						$context->get(Log::class)->log(Log::ERROR, "Could not strip image metadata: {$e->getMessage()}");
+						$context->get(LogDriver::class)->log(LogDriver::ERROR, "Could not strip image metadata: {$e->getMessage()}");
 						// Since EXIF metadata can countain sensible info, fail the request.
 						error(_('Could not strip EXIF metadata!'), null, $error);
 					}
@@ -1200,7 +1200,7 @@ if (isset($_POST['delete'])) {
 						$post['body_nomarkup'] .= "<tinyboard ocr image $key>" . htmlspecialchars($txt) . "</tinyboard>";
 					}
 				} catch (RuntimeException $e) {
-					$context->get(Log::class)->log(Log::ERROR, "Could not OCR image: {$e->getMessage()}");
+					$context->get(LogDriver::class)->log(LogDriver::ERROR, "Could not OCR image: {$e->getMessage()}");
 				}
 			}
 		}
@@ -1394,8 +1394,8 @@ if (isset($_POST['delete'])) {
 
 	buildThread($post['op'] ? $id : $post['thread']);
 
-	$context->get(Log::class)->log(
+	$context->get(LogDriver::class)->log(
-		Log::INFO,
+		LogDriver::INFO,
 		'New post: /' . $board['dir'] . $config['dir']['res'] . link_for($post) . (!$post['op'] ? '#' . $id : '')
 	);