From c67e49ea00f6d2975b8100ab294790c73dcb0aaa Mon Sep 17 00:00:00 2001
From: Florent Bruneau <florent.bruneau@polytechnique.org>
Date: Mon, 1 Nov 2010 15:54:40 +0100
Subject: [PATCH 1/1] Several improvements on how PHP errors are managed.

Use PlBacktrace to handle PHP error instead of $GLOBAL['pl_errors']. When
output is in json add a special key with the traces. When site is in debug
mode, errors are automatically dumped by PlBacktrace. When site is in
production mode, use the legacy pl_print_errors to print the errors at the
bottom on the page.

This commit also introduce a new solution to build json handlers: if the
handler return PL_JSON, then the site will produce json from the variables
previously assigned vi jsonAssign.

Note:  Don't use jsonDisplay() for the new json output because
jsonDisplay() output contains user-controlled content (I mean, content
passed by the user through Post, Get or Cookie) and this is not a good
idea: the site should never output something it does not fully control.

Signed-off-by: Florent Bruneau <florent.bruneau@polytechnique.org>
---
 classes/platal.php     | 10 ++++++++--
 classes/plpage.php     | 17 ++++++++++++++---
 include/platal.inc.php | 43 ++++++++++++++++++++++++-------------------
 3 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/classes/platal.php b/classes/platal.php
index 4560f6e..e5187e4 100644
--- a/classes/platal.php
+++ b/classes/platal.php
@@ -23,6 +23,7 @@ define('PL_DO_AUTH',   300);
 define('PL_FORBIDDEN', 403);
 define('PL_NOT_FOUND', 404);
 define('PL_WIKI',      500);
+define('PL_JSON',      501);
 
 abstract class PlHook
 {
@@ -361,7 +362,8 @@ abstract class Platal
 
         try {
             $page->assign('platal', $this);
-            switch ($this->call_hook($page)) {
+            $res = $this->call_hook($page);
+            switch ($res) {
               case PL_FORBIDDEN:
                 $this->mods['core']->handler_403($page);
                 break;
@@ -385,7 +387,11 @@ abstract class Platal
         }
 
         $page->assign('platal', $this);
-        $page->run();
+        if ($res == PL_JSON) {
+            $page->runJSon();
+        } else {
+            $page->run();
+        }
     }
 
     public function error403()
diff --git a/classes/plpage.php b/classes/plpage.php
index 1fbbe23..c13410e 100644
--- a/classes/plpage.php
+++ b/classes/plpage.php
@@ -164,6 +164,7 @@ abstract class PlPage extends Smarty
         if (!$globals->debug) {
             error_reporting(0);
             $this->display($skin);
+            pl_print_errors(true);
             exit;
         }
 
@@ -370,9 +371,8 @@ abstract class PlPage extends Smarty
     protected function jsonDisplay()
     {
         pl_content_headers("text/javascript");
-        if (!empty($GLOBALS['pl_errors'])) {
-            $this->jsonAssign('pl_errors', join("\n", $GLOBALS['pl_errors']));
-            $GLOBALS['pl_errors'] = array();
+        if (!empty(PlBacktrace::$bt)) {
+            $this->jsonAssign('pl_backtraces', PlBacktrace::$bt);
         }
         array_walk_recursive($this->_jsonVars, "escape_XDB");
         $jsonbegin = Env::v('jsonBegin');
@@ -388,6 +388,17 @@ abstract class PlPage extends Smarty
         exit;
     }
     // }}}
+
+    public function runJSon()
+    {
+        pl_content_headers("text/javascript");
+        if (!empty(PlBacktrace::$bt)) {
+            $this->jsonAssign('pl_backtraces', PlBacktrace::$bt);
+        }
+        echo json_encode($this->_jsonVars);
+        exit;
+    }
+
     // {{{ function jsonAssign
     public function jsonAssign($var, $value)
     {
diff --git a/include/platal.inc.php b/include/platal.inc.php
index 069a661..761dcaa 100644
--- a/include/platal.inc.php
+++ b/include/platal.inc.php
@@ -65,6 +65,7 @@ function pl_core_include($file)
 
 function pl_error_handler($errno, $errstr, $errfile, $errline)
 {
+
     static $errortype;
     if (!error_reporting())
         return;
@@ -93,23 +94,15 @@ function pl_error_handler($errno, $errstr, $errfile, $errline)
             return;
         }
     }
-
     $type = isset($errortype[$errno]) ? $errortype[$errno] : $errno;
-    $errstr = utf8_encode(htmlentities($errstr));
-    if (php_sapi_name() == 'cli') {
-        $GLOBALS['pl_errors'] = "$type: $errstr\n  $errfile:$errline\n";
-    } else {
-        $GLOBALS['pl_errors'][] =
-            "<div class='phperror'>".
-            "<strong>{$type}</strong> <em>$errstr</em><br />".
-            "<tt>$errfile : $errline</tt>".
-            "</div>";
+    $error = strpos($type, 'Warning') !== false || strpos($type, 'Error') !==false;
+    if (!isset(PlBacktrace::$bt['PHP Errors'])) {
+        new PlBacktrace('PHP Errors');
     }
-}
-
-function pl_clear_errors()
-{
-    unset($GLOBALS['pl_errors']);
+    PlBacktrace::$bt['PHP Errors']->newEvent("$type: $errstr",
+                                             0, $error ? $errstr : null,
+                                             array(array('file' => $errfile,
+                                                         'line' => $errline)));
 }
 
 function pl_dump_env()
@@ -122,15 +115,27 @@ function pl_dump_env()
     echo "</pre></div>";
 }
 
-function pl_print_errors()
+function pl_print_errors($html = false)
 {
-    if (!empty($GLOBALS['pl_errors'])) {
-        print join("\n", $GLOBALS['pl_errors']);
+    if (!isset(PlBacktrace::$bt['PHP Errors'])) {
+        return;
+    }
+    foreach (PlBacktrace::$bt['PHP Errors']->traces as $trace) {
+        if ($html) {
+            echo "<pre>";
+        }
+        print "{$trace['action']}\n";
+        print "  {$trace['data'][0]['file']}: {$trace['data'][0]['line']}\n";
+        if ($html) {
+            echo "</pre>";
+        }
     }
 }
 
 set_error_handler('pl_error_handler', E_ALL | E_STRICT);
-register_shutdown_function('pl_print_errors');
+if (php_sapi_name() == 'cli') {
+    register_shutdown_function('pl_print_errors');
+}
 //register_shutdown_function('pl_dump_env');
 
 /** Check if the string is utf8
-- 
2.1.4