From 2781b63b684775a146e8a6e76de857ea8b135a67 Mon Sep 17 00:00:00 2001
From: Ben Roberts <ben@sihnon.net>
Date: Sat, 20 Aug 2011 11:34:29 +0100
Subject: [PATCH] Bug fixes to reduce logging-related crashes

* Provide a method for plugins to report failure to initialise
* Check plugins initialised properly before adding to list of logging
backends
* Check the logger is available before trying to log a message
(might cause bugs if errors are logged very early in initialisation)
---
 .../lib/SihnonFramework/Exceptions.class.php  |  3 +++
 source/lib/SihnonFramework/Log.class.php      | 18 ++++++++++-----
 .../Log/Plugin/FlatFile.class.php             | 23 +++++++++++++++++++
 source/lib/SihnonFramework/LogEntry.class.php | 10 +++++++-
 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/source/lib/SihnonFramework/Exceptions.class.php b/source/lib/SihnonFramework/Exceptions.class.php
index 429267a..4aa2764 100644
--- a/source/lib/SihnonFramework/Exceptions.class.php
+++ b/source/lib/SihnonFramework/Exceptions.class.php
@@ -30,5 +30,8 @@ class SihnonFramework_Exception_CacheObjectNotFound    extends SihnonFramework_E
 
 class SihnonFramework_Exception_InvalidPluginName      extends SihnonFramework_Exception {};
 
+class SihnonFramework_Exception_LogException           extends SihnonFramework_Exception {};
+class SihnonFramework_Exception_LogFileNotWriteable    extends SihnonFramework_Exception_LogException {};
+class SihnonFramework_Exception_InvalidLog             extends SihnonFramework_Exception_LogException {};
 
 ?>
diff --git a/source/lib/SihnonFramework/Log.class.php b/source/lib/SihnonFramework/Log.class.php
index fe403d4..41e6dd2 100644
--- a/source/lib/SihnonFramework/Log.class.php
+++ b/source/lib/SihnonFramework/Log.class.php
@@ -15,6 +15,8 @@ class SihnonFramework_Log {
     protected $plugins = array();
     
     public function __construct(SihnonFramework_Config $config) {
+        $log = SihnonFramework_Main::instance()->log();
+        
         // Get a list of the logging plugins to be used
         $plugins = $config->get('logging.plugins');
         
@@ -22,12 +24,16 @@ class SihnonFramework_Log {
             // Get a list of all the instances of this plugin to be used
             $instances = $config->get("logging.{$plugin}");
             foreach ($instances as $instance) {
-                $this->plugins[$plugin][] = array(
-                    'name' => $instance,
-                    'backend' => Sihnon_Log_PluginFactory::create($config, $plugin, $instance),
-                    'severity' => $config->get("logging.{$plugin}.{$instance}.severity"),
-                    'category' => $config->get("logging.{$plugin}.{$instance}.category"),
-                );
+                try {
+                    $this->plugins[$plugin][] = array(
+                        'name' => $instance,
+                        'backend' => Sihnon_Log_PluginFactory::create($config, $plugin, $instance),
+                        'severity' => $config->get("logging.{$plugin}.{$instance}.severity"),
+                        'category' => $config->get("logging.{$plugin}.{$instance}.category"),
+                    );
+                } catch(SihnonFramework_Exception_LogException $e) {
+                    SihnonFramework_LogEntry::warning($log, $e->getMessage());
+                }
             }
         }
         
diff --git a/source/lib/SihnonFramework/Log/Plugin/FlatFile.class.php b/source/lib/SihnonFramework/Log/Plugin/FlatFile.class.php
index 2d6afc3..399bf89 100644
--- a/source/lib/SihnonFramework/Log/Plugin/FlatFile.class.php
+++ b/source/lib/SihnonFramework/Log/Plugin/FlatFile.class.php
@@ -18,6 +18,9 @@ class SihnonFramework_Log_Plugin_FlatFile extends SihnonFramework_Log_PluginBase
         $this->filename = $filename;
         $this->format   = $format;
         
+        // Verify the given log file can be written to
+        $this->verifyLogfile();
+        
         $this->fp = fopen($this->filename, 'a');
     }
     
@@ -32,6 +35,26 @@ class SihnonFramework_Log_Plugin_FlatFile extends SihnonFramework_Log_PluginBase
         return new self($instance, $filename, $format);
     }
     
+    protected function verifyLogfile() {
+        // Check that the file exists, or can be created
+        $file_ok = false;
+        
+        if (preg_match('#php://(stderr|stdout)#', $this->filename)) {
+            $file_ok = true;
+        } else if (file_exists($this->filename) && is_writable($this->filename)) {
+            $file_ok = true;
+        } else {
+            $directory = dirname($this->filename);
+            if (file_exists($directory) && is_writeable($directory)) {
+                $file_ok = true;
+            }
+        }
+        
+        if (!$file_ok) {
+            throw new SihnonFramework_Exception_LogFileNotWriteable($this->filename);
+        }
+    }
+    
     /**
      * Records a new entry in the storage backend used by this logging plugin
      * 
diff --git a/source/lib/SihnonFramework/LogEntry.class.php b/source/lib/SihnonFramework/LogEntry.class.php
index 68fe490..7c9ecf4 100644
--- a/source/lib/SihnonFramework/LogEntry.class.php
+++ b/source/lib/SihnonFramework/LogEntry.class.php
@@ -125,13 +125,21 @@ class SihnonFramework_LogEntry {
     }
     
     protected static function log($logger, $severity, $message, $category = SihnonFramework_Log::CATEGORY_DEFAULT) {
+        if (!$logger) {
+            throw new SihnonFramework_Exception_InvalidLog();
+        }
+        
         $backtrace = debug_backtrace(false);
         $entry = new static($severity, $category, time(), static::$localHostname, static::$localProgname, getmypid(), $backtrace[1]['file'], $backtrace[1]['line'], $message);
         
-        $logger->log($entry);
+       $logger->log($entry);
     }
     
     public static function logInternal($logger, $severity, $file, $line, $message, $category = SihnonFramework_Log::CATEGORY_DEFAULT) {
+        if (!$logger) {
+            throw new SihnonFramework_Exception_InvalidLog();
+        }
+        
         $entry = new static($severity, $category, time(), static::$localHostname, static::$localProgname, getmypid(), $file, $line, $message);
         $logger->log($entry);
     }