From 642e976cb685105e1684f43a5d99c79f258029f4 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 13 Jul 2020 19:05:12 +0200 Subject: [PATCH 1/9] introduce a global error handler This transfers old style PHP errors into Exceptions and installs a global exception handler. All exceptions caught by this handler are logged to an error log and a meassage is shown to the end user. This should finally get rid of "blank page" bug reports. --- inc/ErrorHandler.php | 98 ++++++++++++++++++++++++++++++ inc/Extension/PluginController.php | 68 ++++++++++++--------- inc/init.php | 3 + 3 files changed, 139 insertions(+), 30 deletions(-) create mode 100644 inc/ErrorHandler.php diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php new file mode 100644 index 000000000..2a590320c --- /dev/null +++ b/inc/ErrorHandler.php @@ -0,0 +1,98 @@ +getMessage()); + $msg = 'An unforeseen error has occured. This is most likely a bug somewhere.'; + $logged = self::logException($e) + ? 'More info has been written to the DokuWiki _error.log' + : $e->getFile() . ':' . $e->getLine(); + + echo << + +$title + +
+

$title

+

$msg

+

$logged

+
+ + +EOT; + } + + /** + * Convenience method to display an error message for the given Exception + * + * @param \Throwable $e + * @param string $intro + */ + public static function showExceptionMsg($e, $intro = 'Error!') + { + $msg = $intro . get_class($e) . ': ' . $e->getMessage(); + self::logException($e); + msg(hsc($msg), -1); + } + + /** + * Default error handler to convert old school warnings, notices, etc to exceptions + * + * You should not need to call this directly! + * + * @param int $errno + * @param string $errstr + * @param string $errfile + * @param int $errline + * @return bool + * @throws \ErrorException + */ + public static function errorConverter($errno, $errstr, $errfile, $errline) + { + if (!(error_reporting() & $errno)) { + // This error code is not included in error_reporting, so let it fall + // through to the standard PHP error handler + return false; + } + throw new \ErrorException($errstr, 0, $errno, $errfile, $errline); + } + + /** + * Log the given exception to the error log + * + * @param \Throwable $e + * @return bool false if the logging failed + */ + public static function logException($e) + { + global $conf; + + $log = join("\t", [gmdate('c'), get_class($e), $e->getFile() . ':' . $e->getLine(), $e->getMessage()]) . "\n"; + return io_saveFile($conf['cachedir'] . '/_error.log', $log, true); + } +} diff --git a/inc/Extension/PluginController.php b/inc/Extension/PluginController.php index 638fd3946..23115a60b 100644 --- a/inc/Extension/PluginController.php +++ b/inc/Extension/PluginController.php @@ -2,6 +2,8 @@ namespace dokuwiki\Extension; +use dokuwiki\ErrorHandler; + /** * Class to encapsulate access to dokuwiki plugins * @@ -90,42 +92,48 @@ class PluginController $class = $type . '_plugin_' . $name; - //plugin already loaded? - if (!empty($DOKU_PLUGINS[$type][$name])) { - if ($new || !$DOKU_PLUGINS[$type][$name]->isSingleton()) { - return class_exists($class, true) ? new $class : null; + try { + //plugin already loaded? + if (!empty($DOKU_PLUGINS[$type][$name])) { + if ($new || !$DOKU_PLUGINS[$type][$name]->isSingleton()) { + + return class_exists($class, true) ? new $class : null; + } + + return $DOKU_PLUGINS[$type][$name]; } - return $DOKU_PLUGINS[$type][$name]; - } - - //construct class and instantiate - if (!class_exists($class, true)) { - - # the plugin might be in the wrong directory - $inf = confToHash(DOKU_PLUGIN . "$plugin/plugin.info.txt"); - if ($inf['base'] && $inf['base'] != $plugin) { - msg( - sprintf( - "Plugin installed incorrectly. Rename plugin directory '%s' to '%s'.", - hsc($plugin), - hsc( - $inf['base'] - ) - ), -1 - ); - } elseif (preg_match('/^' . DOKU_PLUGIN_NAME_REGEX . '$/', $plugin) !== 1) { - msg( - sprintf( - "Plugin name '%s' is not a valid plugin name, only the characters a-z and 0-9 are allowed. " . - 'Maybe the plugin has been installed in the wrong directory?', hsc($plugin) - ), -1 - ); + //construct class and instantiate + if (!class_exists($class, true)) { + # the plugin might be in the wrong directory + $inf = confToHash(DOKU_PLUGIN . "$plugin/plugin.info.txt"); + if ($inf['base'] && $inf['base'] != $plugin) { + msg( + sprintf( + "Plugin installed incorrectly. Rename plugin directory '%s' to '%s'.", + hsc($plugin), + hsc( + $inf['base'] + ) + ), -1 + ); + } elseif (preg_match('/^' . DOKU_PLUGIN_NAME_REGEX . '$/', $plugin) !== 1) { + msg( + sprintf( + "Plugin name '%s' is not a valid plugin name, only the characters a-z and 0-9 are allowed. " . + 'Maybe the plugin has been installed in the wrong directory?', hsc($plugin) + ), -1 + ); + } + return null; } + $DOKU_PLUGINS[$type][$name] = new $class; + + } catch (\Exception $e) { + ErrorHandler::showExceptionMsg($e, sprintf('Failed to load plugin %s', $plugin)); return null; } - $DOKU_PLUGINS[$type][$name] = new $class; return $DOKU_PLUGINS[$type][$name]; } diff --git a/inc/init.php b/inc/init.php index f9bb53472..f4ca7e8d4 100644 --- a/inc/init.php +++ b/inc/init.php @@ -199,6 +199,9 @@ if (empty($plugin_controller_class)) $plugin_controller_class = dokuwiki\Extensi require_once(DOKU_INC.'vendor/autoload.php'); require_once(DOKU_INC.'inc/load.php'); +// from now on everything is an exception +\dokuwiki\ErrorHandler::register(); + // disable gzip if not available define('DOKU_HAS_BZIP', function_exists('bzopen')); define('DOKU_HAS_GZIP', function_exists('gzopen')); From 7e0b27f2adf6886e1828a6d77152719228566d88 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 13 Jul 2020 19:29:12 +0200 Subject: [PATCH 2/9] guess which plugin was the source of an exception This looks at filenames and classes involved in the stacktrace to see if a plugin is referenced. We're only guessing plugins here, because looking for templates may cause false positives where templates load plugins. --- inc/ErrorHandler.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index 2a590320c..93e816b1a 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -25,8 +25,10 @@ class ErrorHandler */ public static function fatalException($e) { + $plugin = self::guessPlugin($e); $title = hsc(get_class($e) . ': ' . $e->getMessage()); $msg = 'An unforeseen error has occured. This is most likely a bug somewhere.'; + if ($plugin) $msg .= ' It might be a problem in the ' . $plugin . ' plugin.'; $logged = self::logException($e) ? 'More info has been written to the DokuWiki _error.log' : $e->getFile() . ':' . $e->getLine(); @@ -95,4 +97,31 @@ EOT; $log = join("\t", [gmdate('c'), get_class($e), $e->getFile() . ':' . $e->getLine(), $e->getMessage()]) . "\n"; return io_saveFile($conf['cachedir'] . '/_error.log', $log, true); } + + /** + * Checks the the stacktrace for plugin files + * + * @param \Throwable $e + * @return false|string + */ + protected static function guessPlugin($e) + { + foreach ($e->getTrace() as $line) { + if ( + isset($line['class']) && + preg_match('/\w+?_plugin_(\w+)/', $line['class'], $match) + ) { + return $match[1]; + } + + if ( + isset($line['file']) && + preg_match('/lib\/plugins\/(\w+)\//', str_replace('\\', '/', $line['file']), $match) + ) { + return $match[1]; + } + } + + return false; + } } From 0e69c9aff9ff4cba682a19c90e461c1fc38a1c02 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 13 Jul 2020 19:34:59 +0200 Subject: [PATCH 3/9] log stacktrace to error log Ultimately the one responsible for fixing the problem will need this anyway. Better log it right away for (hopefully) better bug reports --- inc/ErrorHandler.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index 93e816b1a..1dc306665 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -94,7 +94,13 @@ EOT; { global $conf; - $log = join("\t", [gmdate('c'), get_class($e), $e->getFile() . ':' . $e->getLine(), $e->getMessage()]) . "\n"; + $log = join("\t", [ + gmdate('c'), + get_class($e), + $e->getFile() . '(' . $e->getLine() . ')', + $e->getMessage(), + ]) . "\n"; + $log .= $e->getTraceAsString() . "\n"; return io_saveFile($conf['cachedir'] . '/_error.log', $log, true); } From ffa84f81215ee805194333e08773b61f12689af9 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 13 Jul 2020 19:48:55 +0200 Subject: [PATCH 4/9] better exception handling on plugin loading Now all important places where plugins are loaded are guarded by a try/except. We're catching Throwables here to be able to catch stuff like syntax errors early on (otherwise they will only be caught by our ErrorConverter much too late). This means that this change requires PHP 7.0 minimum! --- inc/ErrorHandler.php | 6 +++--- inc/Extension/PluginController.php | 2 +- inc/load.php | 18 +++++++++++++++--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index 1dc306665..a2cdb843d 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -57,9 +57,9 @@ EOT; */ public static function showExceptionMsg($e, $intro = 'Error!') { - $msg = $intro . get_class($e) . ': ' . $e->getMessage(); - self::logException($e); - msg(hsc($msg), -1); + $msg = hsc($intro).'
' . hsc(get_class($e) . ': ' . $e->getMessage()); + if(self::logException($e)) $msg .= '
More info is available in the _error.log'; + msg($msg, -1); } /** diff --git a/inc/Extension/PluginController.php b/inc/Extension/PluginController.php index 23115a60b..10aeff713 100644 --- a/inc/Extension/PluginController.php +++ b/inc/Extension/PluginController.php @@ -129,7 +129,7 @@ class PluginController } $DOKU_PLUGINS[$type][$name] = new $class; - } catch (\Exception $e) { + } catch (\Throwable $e) { ErrorHandler::showExceptionMsg($e, sprintf('Failed to load plugin %s', $plugin)); return null; } diff --git a/inc/load.php b/inc/load.php index 46cd91f4c..afcb62637 100644 --- a/inc/load.php +++ b/inc/load.php @@ -108,7 +108,11 @@ function load_autoload($name){ $name = str_replace('/test/', '/_test/', $name); // no underscore in test namespace $file = DOKU_PLUGIN . substr($name, 16) . '.php'; if(file_exists($file)) { - require $file; + try { + require $file; + } catch (\Throwable $e) { + \dokuwiki\ErrorHandler::showExceptionMsg($e, "Error loading plugin $name"); + } return true; } } @@ -118,7 +122,11 @@ function load_autoload($name){ $name = str_replace('/test/', '/_test/', $name); // no underscore in test namespace $file = DOKU_INC.'lib/tpl/' . substr($name, 18) . '.php'; if(file_exists($file)) { - require $file; + try { + require $file; + } catch (\Throwable $e) { + \dokuwiki\ErrorHandler::showExceptionMsg($e, "Error loading template $name"); + } return true; } } @@ -144,7 +152,11 @@ function load_autoload($name){ $c = ((count($m) === 4) ? "/{$m[3]}" : ''); $plg = DOKU_PLUGIN . "{$m[2]}/{$m[1]}$c.php"; if(file_exists($plg)){ - require $plg; + try { + require $plg; + } catch (\Throwable $e) { + \dokuwiki\ErrorHandler::showExceptionMsg($e, "Error loading plugin ${$m[2]}"); + } } return true; } From 03e8a69a1ee6334c8346082ece819be9b115960f Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Mon, 13 Jul 2020 19:59:10 +0200 Subject: [PATCH 5/9] reflow overlong line --- inc/Extension/PluginController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/inc/Extension/PluginController.php b/inc/Extension/PluginController.php index 10aeff713..42aeac498 100644 --- a/inc/Extension/PluginController.php +++ b/inc/Extension/PluginController.php @@ -120,7 +120,8 @@ class PluginController } elseif (preg_match('/^' . DOKU_PLUGIN_NAME_REGEX . '$/', $plugin) !== 1) { msg( sprintf( - "Plugin name '%s' is not a valid plugin name, only the characters a-z and 0-9 are allowed. " . + "Plugin name '%s' is not a valid plugin name, only the characters a-z ". + "and 0-9 are allowed. " . 'Maybe the plugin has been installed in the wrong directory?', hsc($plugin) ), -1 ); From cb4cefebb32128c1e6893c596b119ef03d82a7f2 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Tue, 14 Jul 2020 00:55:22 +0200 Subject: [PATCH 6/9] add shutdown handler to even manage fatal errors We can't catch some errors and they are not passed to the error_handler either. But we can at least react to them in a shutdown function to show a friendly message to the user and write our log. --- inc/ErrorHandler.php | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index a2cdb843d..3dc6d20e4 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -13,6 +13,7 @@ class ErrorHandler set_error_handler([ErrorHandler::class, 'errorConverter']); if (!defined('DOKU_UNITTEST')) { set_exception_handler([ErrorHandler::class, 'fatalException']); + register_shutdown_function([ErrorHandler::class, 'fatalShutdown']); } } @@ -57,8 +58,8 @@ EOT; */ public static function showExceptionMsg($e, $intro = 'Error!') { - $msg = hsc($intro).'
' . hsc(get_class($e) . ': ' . $e->getMessage()); - if(self::logException($e)) $msg .= '
More info is available in the _error.log'; + $msg = hsc($intro) . '
' . hsc(get_class($e) . ': ' . $e->getMessage()); + if (self::logException($e)) $msg .= '
More info is available in the _error.log'; msg($msg, -1); } @@ -76,14 +77,40 @@ EOT; */ public static function errorConverter($errno, $errstr, $errfile, $errline) { + if (!(error_reporting() & $errno)) { // This error code is not included in error_reporting, so let it fall // through to the standard PHP error handler return false; } + throw new \ErrorException($errstr, 0, $errno, $errfile, $errline); } + /** + * Last resort to handle fatal errors that still can't be cought + */ + public static function fatalShutdown() + { + $error = error_get_last(); + // Check if it's a core/fatal error, otherwise it's a normal shutdown + if ( + $error !== null && + in_array( + $error['type'], + [ + E_ERROR, + E_CORE_ERROR, + E_COMPILE_ERROR, + ] + ) + ) { + self::fatalException( + new \ErrorException($error['message'], 0, $error['type'], $error['file'], $error['line']) + ); + } + } + /** * Log the given exception to the error log * From dbe0790d4cb5bac92c977818735d040e84836756 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Tue, 14 Jul 2020 22:16:01 +0200 Subject: [PATCH 7/9] FatalException and proper plugin detection Using our own Exception makes it easier to differentiate fatal errors from others in the log --- inc/ErrorHandler.php | 15 +++++++++++++-- inc/Exception/FatalException.php | 11 +++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 inc/Exception/FatalException.php diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index 3dc6d20e4..49a01a25c 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -2,6 +2,13 @@ namespace dokuwiki; +use dokuwiki\Exception\FatalException; + +/** + * Manage the global handling of errors and exceptions + * + * Developer may use this to log and display exceptions themselves + */ class ErrorHandler { @@ -88,7 +95,7 @@ EOT; } /** - * Last resort to handle fatal errors that still can't be cought + * Last resort to handle fatal errors that still can't be caught */ public static function fatalShutdown() { @@ -106,7 +113,7 @@ EOT; ) ) { self::fatalException( - new \ErrorException($error['message'], 0, $error['type'], $error['file'], $error['line']) + new FatalException($error['message'], 0, $error['type'], $error['file'], $error['line']) ); } } @@ -139,6 +146,10 @@ EOT; */ protected static function guessPlugin($e) { + if (preg_match('/lib\/plugins\/(\w+)\//', str_replace('\\', '/', $e->getFile()), $match)) { + return $match[1]; + } + foreach ($e->getTrace() as $line) { if ( isset($line['class']) && diff --git a/inc/Exception/FatalException.php b/inc/Exception/FatalException.php new file mode 100644 index 000000000..f52cc6647 --- /dev/null +++ b/inc/Exception/FatalException.php @@ -0,0 +1,11 @@ + Date: Tue, 14 Jul 2020 22:18:01 +0200 Subject: [PATCH 8/9] no need to convert Errors to Exceptions we handle Throwables anyway --- inc/ErrorHandler.php | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/inc/ErrorHandler.php b/inc/ErrorHandler.php index 49a01a25c..184caaebb 100644 --- a/inc/ErrorHandler.php +++ b/inc/ErrorHandler.php @@ -11,13 +11,11 @@ use dokuwiki\Exception\FatalException; */ class ErrorHandler { - /** * Register the default error handling */ public static function register() { - set_error_handler([ErrorHandler::class, 'errorConverter']); if (!defined('DOKU_UNITTEST')) { set_exception_handler([ErrorHandler::class, 'fatalException']); register_shutdown_function([ErrorHandler::class, 'fatalShutdown']); @@ -70,30 +68,6 @@ EOT; msg($msg, -1); } - /** - * Default error handler to convert old school warnings, notices, etc to exceptions - * - * You should not need to call this directly! - * - * @param int $errno - * @param string $errstr - * @param string $errfile - * @param int $errline - * @return bool - * @throws \ErrorException - */ - public static function errorConverter($errno, $errstr, $errfile, $errline) - { - - if (!(error_reporting() & $errno)) { - // This error code is not included in error_reporting, so let it fall - // through to the standard PHP error handler - return false; - } - - throw new \ErrorException($errstr, 0, $errno, $errfile, $errline); - } - /** * Last resort to handle fatal errors that still can't be caught */ From 697ca7e2e129a18dbfb89b0238c752930e4bede7 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Thu, 13 Aug 2020 20:32:17 +0200 Subject: [PATCH 9/9] fixed plugin name output on load error --- inc/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/load.php b/inc/load.php index afcb62637..68d425584 100644 --- a/inc/load.php +++ b/inc/load.php @@ -155,7 +155,7 @@ function load_autoload($name){ try { require $plg; } catch (\Throwable $e) { - \dokuwiki\ErrorHandler::showExceptionMsg($e, "Error loading plugin ${$m[2]}"); + \dokuwiki\ErrorHandler::showExceptionMsg($e, "Error loading plugin {$m[2]}"); } } return true;