Reformat the logging classes. Extract the LoggingHandler to its own file. Add proper documentation. Fix typos.

5 files changed, 429 insertions, 311 deletions

diff --git a/lib/SimpleSAML/Logger.php b/lib/SimpleSAML/Logger.php
index 9ea5982..71a175a 100644
--- a/lib/SimpleSAML/Logger.php
+++ b/lib/SimpleSAML/Logger.php
@@ -1,7 +1,7 @@
 <?php
 
 /**
- * A class for logging
+ * The main logger class for SimpleSAMLphp.
  *
  * @author Lasse Birnbaum Jensen, SDU.
  * @author Andreas Åkre Solberg, UNINETT AS. <andreas.solberg@uninett.no>
@@ -9,40 +9,32 @@
  * @version $ID$
  */
 
-interface SimpleSAML_Logger_LoggingHandler {
-    function log_internal($level,$string);
+class SimpleSAML_Logger
+{
+    private static $loggingHandler = NULL;
+    private static $logLevel = NULL;
+    private static $captureLog = FALSE;
+    private static $capturedLog = array();
 
-    function setLogFormat($format);
-}
+    /**
+     * Array with messages logged before the logging handler was initialized.
+     *
+     * @var array
+     */
+    private static $earlyLog = array();
 
-class SimpleSAML_Logger {
-	private static $loggingHandler = null;
-	private static $logLevel = null;
-	
-	private static $captureLog = FALSE;
-	private static $capturedLog = array();
-
-	/**
-	 * Array with log messages from before we
-	 * initialized the logging handler.
-	 *
-	 * @var array
-	 */
-	private static $earlyLog = array();
-
-
-	/**
-	 * This constant defines the string we set the trackid to while we are fetching the
-	 * trackid from the session class. This is used to prevent infinite recursion.
-	 */
-	private static $TRACKID_FETCHING = '_NOTRACKIDYET_';
-
-	/**
-	 * This variable holds the trackid we have retrieved from the session class.
-	 * It can also hold NULL, in which case we haven't fetched the trackid yet, or
-	 * TRACKID_FETCHING, which means that we are fetching the trackid now.
-	 */
-	private static $trackid = null;
+
+    /**
+     * This constant defines the string we set the track ID to while we are fetching the track ID from the session
+     * class. This is used to prevent infinite recursion.
+     */
+    private static $TRACKID_FETCHING = '_NOTRACKIDYET_';
+
+    /**
+     * This variable holds the track ID we have retrieved from the session class. It can also be NULL, in which case
+     * we haven't fetched the track ID yet, or TRACKID_FETCHING, which means that we are fetching the track ID now.
+     */
+    private static $trackid = NULL;
 
     /**
      * This variable holds the format used to log any message. Its use varies depending on the log handler used (for
@@ -73,156 +65,208 @@ class SimpleSAML_Logger {
      */
     private static $format = '%date{%b %d %H:%M:%S} %process %level %stat[%trackid] %msg';
 
-/*
-	 *		LOG_ERR				No statistics, only errors
-	 *		LOG_WARNING			No statistics, only warnings/errors
-	 *		LOG_NOTICE			Statistics and errors 
-	 *		LOG_INFO			Verbose logs
-	 *		LOG_DEBUG			Full debug logs - not reccomended for production
-
-*/
-	const EMERG = 0;
-	const ALERT = 1;
-	const CRIT = 2;
-	const ERR = 3;
-	const WARNING = 4;
-	const NOTICE = 5;
-	const INFO = 6;
-	const DEBUG = 7;
-
-	static function emergency($string) {
-		self::log_internal(self::EMERG,$string);
-	}
-
-	static function critical($string) {
-		self::log_internal(self::CRIT,$string);
-	}
-
-	static function alert($string) {
-		self::log_internal(self::ALERT,$string);
-	}
-
-	static function error($string) {
-		self::log_internal(self::ERR,$string);
-	}
-
-	static function warning($string) {
-		self::log_internal(self::WARNING,$string);
-	}
-
-	/**
-	 * We reserve the notice level for statistics, so do not use
-	 * this level for other kind of log messages.
-	 */
-	static function notice($string) {
-		self::log_internal(self::NOTICE,$string);
-	}
-
-	/**
-	 * Info messages is abit less verbose than debug messages. This is useful
-	 * for tracing a session. 
-	 */
-	static function info($string) {
-		self::log_internal(self::INFO,$string);
-	}
-	
-	/**
-	 * Debug messages is very verbose, and will contain more inforation than 
-	 * what is neccessary for a production system.
-	 */
-	static function debug($string) {
-		self::log_internal(self::DEBUG,$string);
-	}
-
-	/**
-	 * Statisitics
-	 */
-	static function stats($string) {
-		self::log_internal(self::NOTICE,$string,true);
-	}
-	
-	
-	
-	public static function createLoggingHandler() {
-
-		/* Set to FALSE to indicate that it is being initialized. */
-		self::$loggingHandler = FALSE;
-
-		/* Get the configuration. */
-		$config = SimpleSAML_Configuration::getInstance();
-		assert($config instanceof SimpleSAML_Configuration);
-
-		/* Get the metadata handler option from the configuration. */
-		$handler = $config->getString('logging.handler', 'syslog');
-
-		/*
-		 * setting minimum log_level
-		 */
-		self::$logLevel = $config->getInteger('logging.level',self::INFO);
-
-		$handler = strtolower($handler);
-
-		if($handler === 'syslog') {
-			$sh = new SimpleSAML_Logger_LoggingHandlerSyslog();
-
-		} elseif ($handler === 'file')  {
-			$sh = new SimpleSAML_Logger_LoggingHandlerFile();
-		} elseif ($handler === 'errorlog')  {
-			$sh = new SimpleSAML_Logger_LoggingHandlerErrorLog();
-		} else {
-			throw new Exception('Invalid value for the [logging.handler] configuration option. Unknown handler: ' . $handler);
-		}
+    const EMERG = 0;
+    const ALERT = 1;
+    const CRIT = 2;
+    const ERR = 3;
+    const WARNING = 4;
+    const NOTICE = 5;
+    const INFO = 6;
+    const DEBUG = 7;
+
+
+    /**
+     * Log an emergency message.
+     *
+     * @var string $string The message to log.
+     */
+    public static function emergency($string)
+    {
+        self::log(self::EMERG, $string);
+    }
+
+
+    /**
+     * Log a critical message.
+     *
+     * @var string $string The message to log.
+     */
+    public static function critical($string)
+    {
+        self::log(self::CRIT, $string);
+    }
+
+
+    /**
+     * Log an alert.
+     *
+     * @var string $string The message to log.
+     */
+    public static function alert($string)
+    {
+        self::log(self::ALERT, $string);
+    }
+
+
+    /**
+     * Log an error.
+     *
+     * @var string $string The message to log.
+     */
+    public static function error($string)
+    {
+        self::log(self::ERR, $string);
+    }
+
+
+    /**
+     * Log a warning.
+     *
+     * @var string $string The message to log.
+     */
+    public static function warning($string)
+    {
+        self::log(self::WARNING, $string);
+    }
+
+    /**
+     * We reserve the notice level for statistics, so do not use this level for other kind of log messages.
+     *
+     * @var string $string The message to log.
+     */
+    public static function notice($string)
+    {
+        self::log(self::NOTICE, $string);
+    }
+
+
+    /**
+     * Info messages are a bit less verbose than debug messages. This is useful to trace a session.
+     *
+     * @var string $string The message to log.
+     */
+    public static function info($string)
+    {
+        self::log(self::INFO, $string);
+    }
+
+
+    /**
+     * Debug messages are very verbose, and will contain more information than what is necessary for a production
+     * system.
+     *
+     * @var string $string The message to log.
+     */
+    public static function debug($string)
+    {
+        self::log(self::DEBUG, $string);
+    }
+
+
+    /**
+     * Statistics.
+     *
+     * @var string $string The message to log.
+     */
+    public static function stats($string)
+    {
+        self::log(self::NOTICE, $string, TRUE);
+    }
+
+
+    /**
+     * Set the logger to capture logs.
+     *
+     * @var boolean $val Whether to capture logs or not. Defaults to TRUE.
+     */
+    public static function setCaptureLog($val = TRUE)
+    {
+        self::$captureLog = $val;
+    }
+
+
+    /**
+     * Get the captured log.
+     */
+    public static function getCapturedLog()
+    {
+        return self::$capturedLog;
+    }
+
+
+    private static function createLoggingHandler()
+    {
+        // set to FALSE to indicate that it is being initialized
+        self::$loggingHandler = FALSE;
+
+        // get the configuration
+        $config = SimpleSAML_Configuration::getInstance();
+        assert($config instanceof SimpleSAML_Configuration);
+
+        // get the metadata handler option from the configuration
+        $handler = $config->getString('logging.handler', 'syslog');
+
+        // setting minimum log_level
+        self::$logLevel = $config->getInteger('logging.level', self::INFO);
+
+        $handler = strtolower($handler);
+
+        if ($handler === 'syslog') {
+            $sh = new SimpleSAML_Logger_LoggingHandlerSyslog();
+        } elseif ($handler === 'file') {
+            $sh = new SimpleSAML_Logger_LoggingHandlerFile();
+        } elseif ($handler === 'errorlog') {
+            $sh = new SimpleSAML_Logger_LoggingHandlerErrorLog();
+        } else {
+            throw new Exception(
+                'Invalid value for the [logging.handler] configuration option. Unknown handler: ' . $handler
+            );
+        }
 
         self::$format = $config->getString('logging.format', self::$format);
         $sh->setLogFormat(self::$format);
 
-		/* Set the session handler. */
-		self::$loggingHandler = $sh;
-	}
-	
-	public static function setCaptureLog($val = TRUE) {
-		self::$captureLog = $val;
-	}
-	
-	public static function getCapturedLog() {
-		return self::$capturedLog;
-	}	
-	
-	static function log_internal($level,$string,$statsLog = false) {
-		if (self::$loggingHandler === NULL) {
-			/* Initialize logging. */
-			self::createLoggingHandler();
-
-			if (!empty(self::$earlyLog)) {
-				error_log('----------------------------------------------------------------------');
-				/* Output messages which were logged before we initialized to the proper log. */
-				foreach (self::$earlyLog as $msg) {
-					self::log_internal($msg['level'], $msg['string'], $msg['statsLog']);
-				}
-			}
-
-		} elseif (self::$loggingHandler === FALSE) {
-			/* Some error occurred while initializing logging. */
-			if (empty(self::$earlyLog)) {
-				/* This is the first message. */
-				error_log('--- Log message(s) while initializing logging ------------------------');
-			}
-			error_log($string);
-
-			self::$earlyLog[] = array('level' => $level, 'string' => $string, 'statsLog' => $statsLog);
-			return;
-		}
-
-		
-		if (self::$captureLog) {
-			$ts = microtime(TRUE);
-			$msecs = (int)(($ts - (int)$ts) * 1000);
-			$ts = GMdate('H:i:s', $ts) . sprintf('.%03d', $msecs) . 'Z';
-			self::$capturedLog[] = $ts . ' ' . $string;
-		}
-		
-		if (self::$logLevel >= $level || $statsLog) {
-			if (is_array($string)) $string = implode(",",$string);
+        // set the session handler
+        self::$loggingHandler = $sh;
+    }
+
+
+    private static function log($level, $string, $statsLog = FALSE)
+    {
+        if (self::$loggingHandler === NULL) {
+            /* Initialize logging. */
+            self::createLoggingHandler();
+
+            if (!empty(self::$earlyLog)) {
+                error_log('----------------------------------------------------------------------');
+                // output messages which were logged before we properly initialized logging
+                foreach (self::$earlyLog as $msg) {
+                    self::log($msg['level'], $msg['string'], $msg['statsLog']);
+                }
+            }
+        } elseif (self::$loggingHandler === FALSE) {
+            // some error occurred while initializing logging
+            if (empty(self::$earlyLog)) {
+                // this is the first message
+                error_log('--- Log message(s) while initializing logging ------------------------');
+            }
+            error_log($string);
+
+            self::$earlyLog[] = array('level' => $level, 'string' => $string, 'statsLog' => $statsLog);
+            return;
+        }
+
+        if (self::$captureLog) {
+            $ts = microtime(TRUE);
+            $msecs = (int) (($ts - (int) $ts) * 1000);
+            $ts = GMdate('H:i:s', $ts).sprintf('.%03d', $msecs).'Z';
+            self::$capturedLog[] = $ts.' '.$string;
+        }
+
+        if (self::$logLevel >= $level || $statsLog) {
+            if (is_array($string)) {
+                $string = implode(",", $string);
+            }
 
             $formats = array('%trackid', '%msg', '%srcip', '%stat');
             $replacements = array(self::getTrackId(), $string, $_SERVER['REMOTE_ADDR']);
@@ -234,39 +278,38 @@ class SimpleSAML_Logger {
             array_push($replacements, $stat);
 
             $string = str_replace($formats, $replacements, self::$format);
-            self::$loggingHandler->log_internal($level, $string);
-		}
-	}
-	
-
-	/**
-	 * Retrieve the trackid we should use for logging.
-	 *
-	 * It is used to avoid infinite recursion between the logger class and the session class.
-	 *
-	 * @return The trackid we should use for logging, or 'NA' if we detect recursion.
-	 */
-	private static function getTrackId() {
-
-		if(self::$trackid === self::$TRACKID_FETCHING) {
-			/* Recursion detected. */
-			return 'NA';
-		}
-
-		if(self::$trackid === NULL) {
-			/* No trackid yet, fetch it from the session class. */
-
-			/* Mark it as currently being fetched. */
-			self::$trackid = self::$TRACKID_FETCHING;
-
-			/* Get the current session. This could cause recursion back to the logger class. */
-			$session = SimpleSAML_Session::getInstance();
-
-			/* Update the trackid. */
-			self::$trackid = $session->getTrackID();
-		}
-
-		assert('is_string(self::$trackid)');
-		return self::$trackid;
-	}
+            self::$loggingHandler->log($level, $string);
+        }
+    }
+
+
+    /**
+     * Retrieve the track ID we should use for logging. It is used to avoid infinite recursion between the logger class
+     * and the session class.
+     *
+     * @return string The track ID we should use for logging, or 'NA' if we detect recursion.
+     */
+    private static function getTrackId()
+    {
+        if (self::$trackid === self::$TRACKID_FETCHING) {
+            // recursion detected!
+            return 'NA';
+        }
+
+        if (self::$trackid === NULL) {
+            // no track ID yet, fetch it from the session class
+
+            // mark it as currently being fetched
+            self::$trackid = self::$TRACKID_FETCHING;
+
+            // get the current session. This could cause recursion back to the logger class
+            $session = SimpleSAML_Session::getInstance();
+
+            // update the track ID
+            self::$trackid = $session->getTrackID();
+        }
+
+        assert('is_string(self::$trackid)');
+        return self::$trackid;
+    }
 }
diff --git a/lib/SimpleSAML/Logger/LoggingHandler.php b/lib/SimpleSAML/Logger/LoggingHandler.php
new file mode 100644
index 0000000..d4d07cd
--- /dev/null
+++ b/lib/SimpleSAML/Logger/LoggingHandler.php
@@ -0,0 +1,27 @@
+<?php
+/**
+ * The interface that must be implemented by any log handler.
+ *
+ * @author Jaime Perez Crespo, UNINETT AS.
+ * @package simpleSAMLphp
+ * @version $ID$
+ */
+
+interface SimpleSAML_Logger_LoggingHandler
+{
+    /**
+     * Log a message to its destination.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string);
+
+
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format);
+}
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php b/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
index cd9404d..7dcfc7b 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerErrorLog.php
@@ -9,38 +9,53 @@
  * @package simpleSAMLphp
  * @version $ID$
  */
-class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_LoggingHandler {
-
-	/**
-	 * This array contains the mappings from syslog loglevel to names.
-	 */
-	private static $levelNames = array(
-		SimpleSAML_Logger::EMERG => 'EMERG',
-		SimpleSAML_Logger::ALERT => 'ALERT',
-		SimpleSAML_Logger::CRIT => 'CRIT',
-		SimpleSAML_Logger::ERR => 'ERR',
-		SimpleSAML_Logger::WARNING => 'WARNING',
-		SimpleSAML_Logger::NOTICE => 'NOTICE',
-		SimpleSAML_Logger::INFO => 'INFO',
-		SimpleSAML_Logger::DEBUG => 'DEBUG',
-	);
-
-
-    function setLogFormat($format) {
+class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_LoggingHandler
+{
+
+    /**
+     * This array contains the mappings from syslog loglevel to names.
+     */
+    private static $levelNames = array(
+        SimpleSAML_Logger::EMERG   => 'EMERG',
+        SimpleSAML_Logger::ALERT   => 'ALERT',
+        SimpleSAML_Logger::CRIT    => 'CRIT',
+        SimpleSAML_Logger::ERR     => 'ERR',
+        SimpleSAML_Logger::WARNING => 'WARNING',
+        SimpleSAML_Logger::NOTICE  => 'NOTICE',
+        SimpleSAML_Logger::INFO    => 'INFO',
+        SimpleSAML_Logger::DEBUG   => 'DEBUG',
+    );
+    private $format;
+
+
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-	function log_internal($level, $string) {
-		$config = SimpleSAML_Configuration::getInstance();
-		assert($config instanceof SimpleSAML_Configuration);
-		$processname = $config->getString('logging.processname','simpleSAMLphp');
-		
-		if(array_key_exists($level, self::$levelNames)) {
-			$levelName = self::$levelNames[$level];
-		} else {
-			$levelName = sprintf('UNKNOWN%d', $level);
-		}
+    /**
+     * Log a message to syslog.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        $config = SimpleSAML_Configuration::getInstance();
+        assert($config instanceof SimpleSAML_Configuration);
+        $processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
+        if (array_key_exists($level, self::$levelNames)) {
+            $levelName = self::$levelNames[$level];
+        } else {
+            $levelName = sprintf('UNKNOWN%d', $level);
+        }
 
         $formats = array('%process', '%level');
         $replacements = array($processname, $levelName);
@@ -48,8 +63,6 @@ class SimpleSAML_Logger_LoggingHandlerErrorLog implements SimpleSAML_Logger_Logg
         $string = preg_replace('/%\w+(\{[^\}]+\})?/', '', $string);
         $string = trim($string);
 
-		error_log($string);
-	}
+        error_log($string);
+    }
 }
-
-?>
\ No newline at end of file
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerFile.php b/lib/SimpleSAML/Logger/LoggingHandlerFile.php
index cb5e2b1..9ed795e 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerFile.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerFile.php
@@ -9,60 +9,82 @@
  * @version $ID$
  */
 
-class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingHandler {
-    
-	/**
-	 * This array contains the mappings from syslog loglevel to names. Copied
-	 * more or less directly from SimpleSAML_Logger_LoggingHandlerErrorLog.
-	 */
-	private static $levelNames = array(
-		SimpleSAML_Logger::EMERG => 'EMERGENCY',
-		SimpleSAML_Logger::ALERT => 'ALERT',
-		SimpleSAML_Logger::CRIT => 'CRITICAL',
-		SimpleSAML_Logger::ERR => 'ERROR',
-		SimpleSAML_Logger::WARNING => 'WARNING',
-		SimpleSAML_Logger::NOTICE => 'NOTICE',
-		SimpleSAML_Logger::INFO => 'INFO',
-		SimpleSAML_Logger::DEBUG => 'DEBUG',
-	);
-
-    private $logFile = null;
-    private $processname = null;
+class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingHandler
+{
+    /**
+     * This array contains the mappings from syslog loglevel to names. Copied
+     * more or less directly from SimpleSAML_Logger_LoggingHandlerErrorLog.
+     */
+    private static $levelNames = array(
+        SimpleSAML_Logger::EMERG   => 'EMERGENCY',
+        SimpleSAML_Logger::ALERT   => 'ALERT',
+        SimpleSAML_Logger::CRIT    => 'CRITICAL',
+        SimpleSAML_Logger::ERR     => 'ERROR',
+        SimpleSAML_Logger::WARNING => 'WARNING',
+        SimpleSAML_Logger::NOTICE  => 'NOTICE',
+        SimpleSAML_Logger::INFO    => 'INFO',
+        SimpleSAML_Logger::DEBUG   => 'DEBUG',
+    );
+    private $logFile = NULL;
+    private $processname = NULL;
     private $format;
 
-    function __construct() {
+
+    /**
+     * Build a new logging handler based on files.
+     */
+    public function __construct()
+    {
         $config = SimpleSAML_Configuration::getInstance();
         assert($config instanceof SimpleSAML_Configuration);
 
-        /* Get the metadata handler option from the configuration. */
-        $this->logFile = $config->getPathValue('loggingdir', 'log/').$config->getString('logging.logfile', 'simplesamlphp.log');
-		$this->processname = $config->getString('logging.processname','simpleSAMLphp');
-		
+        // get the metadata handler option from the configuration
+        $this->logFile = $config->getPathValue('loggingdir', 'log/') .
+            $config->getString('logging.logfile', 'simplesamlphp.log');
+        $this->processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
         if (@file_exists($this->logFile)) {
-            if (!@is_writeable($this->logFile)) throw new Exception("Could not write to logfile: ".$this->logFile);
-        }
-        else
-        {
-            if (!@touch($this->logFile))  throw new Exception("Could not create logfile: ".$this->logFile." Loggingdir is not writeable for the webserver user.");
+            if (!@is_writeable($this->logFile)) {
+                throw new Exception("Could not write to logfile: " . $this->logFile);
+            }
+        } else {
+            if (!@touch($this->logFile)) {
+                throw new Exception(
+                    "Could not create logfile: " . $this->logFile .
+                    " Loggingdir is not writeable for the webserver user."
+                );
+            }
         }
 
-	SimpleSAML_Utilities::initTimezone();
+        SimpleSAML_Utilities::initTimezone();
     }
 
 
-    function setLogFormat($format) {
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-    function log_internal($level, $string) {
-        if ($this->logFile != null) {
-            
-            // Set human-readable log level. Copied from SimpleSAML_Logger_LoggingHandlerErrorLog.
-        	if(array_key_exists($level, self::$levelNames))
-			    $levelName = self::$levelNames[$level];
-		    else
-			    $levelName = sprintf('UNKNOWN%d', $level);
+    /**
+     * Log a message to the log file.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        if ($this->logFile != NULL) {
+            // set human-readable log level. Copied from SimpleSAML_Logger_LoggingHandlerErrorLog.
+            $levelName = sprintf('UNKNOWN%d', $level);
+            if (array_key_exists($level, self::$levelNames)) {
+                $levelName = self::$levelNames[$level];
+            }
 
             $formats = array('%process', '%level');
             $replacements = array($this->processname, $levelName);
@@ -79,9 +101,7 @@ class SimpleSAML_Logger_LoggingHandlerFile implements SimpleSAML_Logger_LoggingH
             }
 
             $string = str_replace($formats, $replacements, $string);
-            file_put_contents($this->logFile, $string . PHP_EOL, FILE_APPEND);
+            file_put_contents($this->logFile, $string.PHP_EOL, FILE_APPEND);
         }
     }
 }
-
-?>
\ No newline at end of file
diff --git a/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php b/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
index 58fc183..f6d58b1 100644
--- a/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
+++ b/lib/SimpleSAML/Logger/LoggingHandlerSyslog.php
@@ -9,44 +9,60 @@
  * @version $ID$
  */
 
-class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_LoggingHandler {
+class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_LoggingHandler
+{
+    private $isWindows = FALSE;
+    private $format;
 
-	private $isWindows = false;
 
-    function __construct() {
+    /**
+     * Build a new logging handler based on syslog.
+     */
+    public function __construct()
+    {
         $config = SimpleSAML_Configuration::getInstance();
         assert($config instanceof SimpleSAML_Configuration);
         $facility = $config->getInteger('logging.facility', defined('LOG_LOCAL5') ? constant('LOG_LOCAL5') : LOG_USER);
 
-        $processname = $config->getString('logging.processname','simpleSAMLphp');
-        /*
-         * OS Check 
-         * Setting facility to LOG_USER (only valid in Windows), enable log level rewrite on windows systems.
-         */
+        $processname = $config->getString('logging.processname', 'simpleSAMLphp');
+
+        // Setting facility to LOG_USER (only valid in Windows), enable log level rewrite on windows systems.
         if (SimpleSAML_Utilities::isWindowsOS()) {
-        	$this->isWindows = true;
-        	$facility = LOG_USER;
+            $this->isWindows = TRUE;
+            $facility = LOG_USER;
         }
-        	
+
         openlog($processname, LOG_PID, $facility);
     }
 
 
-    function setLogFormat($format) {
+    /**
+     * Set the format desired for the logs.
+     *
+     * @param string $format The format used for logs.
+     */
+    public function setLogFormat($format)
+    {
         $this->format = $format;
     }
 
 
-    function log_internal($level,$string) {
-    	/*
-    	 * Changing log level to supported levels if OS is Windows
-    	 */
-    	if ($this->isWindows) {
-    		if ($level <= 4)
-				$level = LOG_ERR;
-			else
-				$level = LOG_INFO;			
-    	}
+    /**
+     * Log a message to syslog.
+     *
+     * @param int $level The log level.
+     * @param string $string The formatted message to log.
+     */
+    public function log($level, $string)
+    {
+        // changing log level to supported levels if OS is Windows
+        if ($this->isWindows) {
+            if ($level <= 4) {
+                $level = LOG_ERR;
+            } else {
+                $level = LOG_INFO;
+            }
+        }
 
         $formats = array('%process', '%level');
         $replacements = array('', $level);
@@ -57,4 +73,3 @@ class SimpleSAML_Logger_LoggingHandlerSyslog implements SimpleSAML_Logger_Loggin
         syslog($level, $string);
     }
 }
-?>
\ No newline at end of file