From 1c52db21368d2d91490b2ed1b38f5329f1548958 Mon Sep 17 00:00:00 2001
From: Jaime Perez Crespo <jaime.perez@uninett.no>
Date: Wed, 13 Apr 2016 12:34:35 +0200
Subject: [PATCH] Several improvements to SimpleSAML_Error_Exception:

- Exception messages and backtraces are now decoupled, so that they can be logged independently.
- Backtraces are now logged with "debug" log level, and only in case the "debug" configuration option is set.
- A new log() method allows the exception itself to decide which log level to use. This can be used by exceptions overriding this method to change the log level accordingly.
- Add a new parameter to the format() method so that the formatted message is anonymized, safe for display in the browser.

Additionally, this resolves #281.
---
 config-templates/config.php        |   7 +-
 lib/SimpleSAML/Error/Exception.php | 117 ++++++++++++++++++++++-------
 2 files changed, 93 insertions(+), 31 deletions(-)

diff --git a/config-templates/config.php b/config-templates/config.php
index 42f18fe17..ed0b8f003 100644
--- a/config-templates/config.php
+++ b/config-templates/config.php
@@ -124,10 +124,11 @@ $config = array(
      ************************/
 
     /*
-     * If you enable this option, SimpleSAMLphp will log all sent and received messages
-     * to the log file.
+     * If you enable this option SimpleSAMLphp will log the following to the log file:
      *
-     * This option also enables logging of the messages that are encrypted and decrypted.
+     * - All SAML messages sent and received.
+     * - Encrypted and decrypted SAML messages.
+     * - Backtraces on errors.
      *
      * Note: The messages are logged with the DEBUG log level, so you also need to set
      * the 'logging.level' option to LOG_DEBUG.
diff --git a/lib/SimpleSAML/Error/Exception.php b/lib/SimpleSAML/Error/Exception.php
index 55b86ae14..de3de2ab9 100644
--- a/lib/SimpleSAML/Error/Exception.php
+++ b/lib/SimpleSAML/Error/Exception.php
@@ -31,6 +31,22 @@ class SimpleSAML_Error_Exception extends Exception
     private $cause;
 
 
+    /**
+     * Whether debugging is enabled or not.
+     *
+     * @var boolean
+     */
+    private $debug;
+
+
+    /**
+     * The base directory for this SimpleSAMLphp installation.
+     *
+     * @var string
+     */
+    private $basedir;
+
+
     /**
      * Constructor for this error.
      *
@@ -53,6 +69,10 @@ class SimpleSAML_Error_Exception extends Exception
         if ($cause !== null) {
             $this->cause = SimpleSAML_Error_Exception::fromException($cause);
         }
+
+        $config = SimpleSAML_Configuration::getInstance();
+        $this->debug = $config->getBoolean('debug', false);
+        $this->basedir = $config->getBaseDir();
     }
 
 
@@ -144,29 +164,47 @@ class SimpleSAML_Error_Exception extends Exception
      *
      * Create an array of lines for logging.
      *
+     * @param boolean $anonymize Whether the resulting messages should be anonymized or not.
+     *
      * @return array Log lines that should be written out.
      */
-    public function format()
+    public function format($anonymize = false)
     {
+        $ret = array(
+            $this->getClass().': '.$this->getMessage(),
+        );
+        return array_merge($ret, $this->formatBacktrace($anonymize));
+    }
 
+
+    /**
+     * Format the backtrace for logging.
+     *
+     * Create an array of lines for logging from the backtrace.
+     *
+     * @param boolean $anonymize Whether the resulting messages should be anonymized or not.
+     *
+     * @return array All lines of the backtrace, properly formatted.
+     */
+    public function formatBacktrace($anonymize = false)
+    {
         $ret = array();
 
         $e = $this;
         do {
-            $err = $e->getClass().': '.$e->getMessage();
-            if ($e === $this) {
-                $ret[] = $err;
-            } else {
-                $ret[] = 'Caused by: '.$err;
+            if ($e !== $this) {
+                $ret[] = 'Caused by: '.$e->getClass().': '.$e->getMessage();
             }
-
             $ret[] = 'Backtrace:';
 
             $depth = count($e->backtrace);
             foreach ($e->backtrace as $i => $trace) {
+                if ($anonymize) {
+                    $trace = str_replace($this->basedir, '', $trace);
+                }
+
                 $ret[] = ($depth - $i - 1).' '.$trace;
             }
-
             $e = $e->cause;
         } while ($e !== null);
 
@@ -174,6 +212,41 @@ class SimpleSAML_Error_Exception extends Exception
     }
 
 
+    /**
+     * Print the backtrace to the log if the 'debug' option is enabled in the configuration.
+     */
+    protected function logBacktrace()
+    {
+        if (!$this->debug) {
+            return;
+        }
+
+        $backtrace = $this->formatBacktrace();
+        foreach ($backtrace as $line) {
+            SimpleSAML\Logger::debug($line);
+        }
+    }
+
+
+    /**
+     * Print the exception to the log, by default with log level error.
+     *
+     * Override to allow errors extending this class to specify the log level themselves.
+     *
+     * @param int $default_level The log level to use if this method was not overriden.
+     */
+    public function log($default_level)
+    {
+        $fn = array(
+            SimpleSAML\Logger::ERR     => 'logError',
+            SimpleSAML\Logger::WARNING => 'logWarning',
+            SimpleSAML\Logger::INFO    => 'logInfo',
+            SimpleSAML\Logger::DEBUG   => 'logDebug',
+        );
+        call_user_func(array($this, $fn[$default_level]));
+    }
+
+
     /**
      * Print the exception to the log with log level error.
      *
@@ -181,11 +254,8 @@ class SimpleSAML_Error_Exception extends Exception
      */
     public function logError()
     {
-
-        $lines = $this->format();
-        foreach ($lines as $line) {
-            SimpleSAML\Logger::error($line);
-        }
+        SimpleSAML\Logger::error($this->getClass().': '.$this->getMessage());
+        $this->logBacktrace();
     }
 
 
@@ -196,11 +266,8 @@ class SimpleSAML_Error_Exception extends Exception
      */
     public function logWarning()
     {
-
-        $lines = $this->format();
-        foreach ($lines as $line) {
-            SimpleSAML\Logger::warning($line);
-        }
+        SimpleSAML\Logger::warning($this->getClass().': '.$this->getMessage());
+        $this->logBacktrace();
     }
 
 
@@ -211,11 +278,8 @@ class SimpleSAML_Error_Exception extends Exception
      */
     public function logInfo()
     {
-
-        $lines = $this->format();
-        foreach ($lines as $line) {
-            SimpleSAML\Logger::debug($line);
-        }
+        SimpleSAML\Logger::info($this->getClass().': '.$this->getMessage());
+        $this->logBacktrace();
     }
 
 
@@ -226,11 +290,8 @@ class SimpleSAML_Error_Exception extends Exception
      */
     public function logDebug()
     {
-
-        $lines = $this->format();
-        foreach ($lines as $line) {
-            SimpleSAML\Logger::debug($line);
-        }
+        SimpleSAML\Logger::debug($this->getClass().': '.$this->getMessage());
+        $this->logBacktrace();
     }
 
 
-- 
GitLab