From 28395d55e5d3e7a1c7b302a3a5d34459803db12d Mon Sep 17 00:00:00 2001 From: Zankaria Date: Sun, 4 Feb 2024 12:42:21 +0100 Subject: [PATCH 1/4] Refactor the logging system --- inc/config.php | 17 +++- inc/driver/log-driver.php | 189 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 2 deletions(-) create mode 100644 inc/driver/log-driver.php diff --git a/inc/config.php b/inc/config.php index ffd1b544..c34eb034 100644 --- a/inc/config.php +++ b/inc/config.php @@ -65,8 +65,21 @@ // been generated. This keeps the script from querying the database and causing strain when not needed. $config['has_installed'] = '.installed'; - // Use syslog() for logging all error messages and unauthorized login attempts. - $config['syslog'] = false; + // Deprecated, use 'log_system'. + // $config['syslog'] = false; + + $config['log_system'] = []; + // Log all error messages and unauthorized login attempts. + // Can be "syslog", "error_log" (default), "file", "stderr" or "none". + $config['log_system']['type'] = 'error_log'; + // The application name used by the logging system. Defaults to "tinyboard" for backwards compatibility. + $config['log_system']['name'] = 'tinyboard'; + // Only relevant if 'log_system' is set to "syslog". If true, double print the logs also in stderr. + // Defaults to false. + $config['log_system']['syslog_stderr'] = false; + // Only relevant if "log_system" is set to `file`. Sets the file that vichan will log to. + // Defaults to '/var/log/vichan.log'. + $config['log_system']['file_path'] = '/var/log/vichan.log'; // Use `host` via shell_exec() to lookup hostnames, avoiding query timeouts. May not work on your system. // Requires safe_mode to be disabled. diff --git a/inc/driver/log-driver.php b/inc/driver/log-driver.php new file mode 100644 index 00000000..0026f009 --- /dev/null +++ b/inc/driver/log-driver.php @@ -0,0 +1,189 @@ +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; +} From f05c290b67ed5a359a0e8170ac5177eddbb70ad9 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Thu, 4 Apr 2024 09:56:59 +0200 Subject: [PATCH 2/4] log-driver.php: autoload the logging dirver --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index e1951679..b06ff6a1 100644 --- a/composer.json +++ b/composer.json @@ -34,6 +34,7 @@ "inc/queue.php", "inc/functions.php", "inc/driver/http-driver.php", + "inc/driver/log-driver.php", "inc/service/captcha-queries.php" ] }, From 55cb6bc400513ea5ca23ea0aac6cefa7c212f122 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Wed, 3 Apr 2024 23:42:05 +0200 Subject: [PATCH 3/4] context.php: add log driver --- inc/context.php | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/inc/context.php b/inc/context.php index 5e540bab..3e52b569 100644 --- a/inc/context.php +++ b/inc/context.php @@ -1,24 +1,53 @@ config['log_system']['name']; + $level = $this->config['debug'] ? Log::DEBUG : Log::NOTICE; + $backend = $this->config['log_system']['type']; + + // Check 'syslog' for backwards compatibility. + if ((isset($this->config['syslog']) && $this->config['syslog']) || $backend === 'syslog') { + return LogDrivers::syslog($name, $level, $this->config['log_system']['syslog_stderr']); + } elseif ($backend === 'file') { + return LogDrivers::file($name, $level, $this->config['log_system']['file_path']); + } elseif ($backend === 'stderr') { + return LogDrivers::stderr($name, $level); + } elseif ($backend === 'none') { + return LogDrivers::none(); + } else { + return LogDrivers::error_log($name, $level); + } + } + + public function __construct(array $config) { $this->config = $config; } + public function getLog(): Log { + if (is_null($this->log)) { + $this->log = $this->initLogDriver(); + } + return $this->log; + } + public function getHttpDriver(): HttpDriver { if (is_null($this->http)) { $this->http = HttpDrivers::getHttpDriver($this->config['upload_by_url_timeout'], $this->config['max_filesize']); From 710f6aa6c2983dee3295e7586b19782c7f2fa88c Mon Sep 17 00:00:00 2001 From: Zankaria Date: Wed, 3 Apr 2024 23:51:02 +0200 Subject: [PATCH 4/4] post.php: use logger --- post.php | 54 +++++++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/post.php b/post.php index 387b1a97..487cf2dc 100644 --- a/post.php +++ b/post.php @@ -7,6 +7,7 @@ require_once 'inc/bootstrap.php'; use Vichan\AppContext; use Vichan\Driver\HttpDriver; +use Vichan\Driver\Log; use Vichan\Service\{RemoteCaptchaQuery, NativeCaptchaQuery}; /** @@ -250,7 +251,7 @@ if (isset($_GET['Newsgroups']) && $config['nntpchan']['enabled']) { $content = file_get_contents("php://input"); } elseif ($ct == 'multipart/mixed' || $ct == 'multipart/form-data') { - _syslog(LOG_INFO, "MM: Files: ".print_r($GLOBALS, true)); // Debug + $context->getLog()->log(Log::DEBUG, 'MM: Files: ' . print_r($GLOBALS, true)); $content = ''; @@ -415,8 +416,9 @@ if (isset($_POST['delete'])) { modLog("User at $ip deleted their own post #$id"); } - _syslog(LOG_INFO, 'Deleted post: ' . - '/' . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '') + $context->getLog()->log( + Log::INFO, + 'Deleted post: /' . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '') ); } } @@ -491,9 +493,7 @@ if (isset($_POST['delete'])) { error($config['error']['captcha']); } } catch (RuntimeException $e) { - if ($config['syslog']) { - _syslog(LOG_ERR, "Native captcha IO exception: {$e->getMessage()}"); - } + $context->getLog()->log(Log::ERROR, "Native captcha IO exception: {$e->getMessage()}"); error($config['error']['local_io_error']); } } @@ -512,9 +512,7 @@ if (isset($_POST['delete'])) { $post = $query->fetch(PDO::FETCH_ASSOC); if ($post === false) { - if ($config['syslog']) { - _syslog(LOG_INFO, "Failed to report non-existing post #{$id} in {$board['dir']}"); - } + $context->getLog()->log(Log::INFO, "Failed to report non-existing post #{$id} in {$board['dir']}"); error($config['error']['nopost']); } @@ -523,11 +521,12 @@ if (isset($_POST['delete'])) { error($error); } - if ($config['syslog']) - _syslog(LOG_INFO, 'Reported post: ' . - '/' . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '') . - ' for "' . $reason . '"' - ); + $context->getLog()->log( + Log::INFO, + 'Reported post: /' + . $board['dir'] . $config['dir']['res'] . link_for($post) . ($post['thread'] ? '#' . $id : '') + . " for \"$reason\"" + ); $query = prepare("INSERT INTO ``reports`` VALUES (NULL, :time, :ip, :board, :post, :reason)"); $query->bindValue(':time', time(), PDO::PARAM_INT); $query->bindValue(':ip', $_SERVER['REMOTE_ADDR'], PDO::PARAM_STR); @@ -637,14 +636,10 @@ if (isset($_POST['delete'])) { } } } catch (RuntimeException $e) { - if ($config['syslog']) { - _syslog(LOG_ERR, "Captcha IO exception: {$e->getMessage()}"); - } + $context->getLog()->log(Log::ERROR, "Captcha IO exception: {$e->getMessage()}"); error($config['error']['remote_io_error']); } catch (JsonException $e) { - if ($config['syslog']) { - _syslog(LOG_ERR, "Bad JSON reply to captcha: {$e->getMessage()}"); - } + $context->getLog()->log(Log::ERROR, "Bad JSON reply to captcha: {$e->getMessage()}"); error($config['error']['remote_io_error']); } @@ -1128,11 +1123,9 @@ if (isset($_POST['delete'])) { try { $file['size'] = strip_image_metadata($file['tmp_name']); } catch (RuntimeException $e) { - if ($config['syslog']) { - _syslog(LOG_ERR, "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); - } + $context->getLog()->log(Log::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); } } else { $image->to($file['file']); @@ -1168,9 +1161,7 @@ if (isset($_POST['delete'])) { $post['body_nomarkup'] .= "" . htmlspecialchars($value) . ""; } } catch (RuntimeException $e) { - if ($config['syslog']) { - _syslog(LOG_ERR, "Could not OCR image: {$e->getMessage()}"); - } + $context->getLog()->log(Log::ERROR, "Could not OCR image: {$e->getMessage()}"); } } } @@ -1360,9 +1351,10 @@ if (isset($_POST['delete'])) { buildThread($post['op'] ? $id : $post['thread']); - if ($config['syslog']) - _syslog(LOG_INFO, 'New post: /' . $board['dir'] . $config['dir']['res'] . - link_for($post) . (!$post['op'] ? '#' . $id : '')); + $context->getLog()->log( + Log::INFO, + 'New post: /' . $board['dir'] . $config['dir']['res'] . link_for($post) . (!$post['op'] ? '#' . $id : '') + ); if (!$post['mod']) header('X-Associated-Content: "' . $redirect . '"');