diff --git a/changelog/unreleased/36413 b/changelog/unreleased/36413 new file mode 100644 index 000000000000..a3aa30733ed7 --- /dev/null +++ b/changelog/unreleased/36413 @@ -0,0 +1,11 @@ +Bugfix: Set 599 HTTP code on error + +Previously, a hard crash, such as DB being down, was being reported with a stacktrace +and a 200 HTTP code. In order to homogenize the behaviour with all the endpoints, we've +changed the behaviour to return a 599 HTTP code and an empty content. + +In addition, we've included a new option in the config.php, "crashdirectory", which +defaults to the "datadirectory" set, where the crash logs will be created. Note that +normal errors will still be reported normally and will log into the owncloud.log file. + +https://github.com/owncloud/core/pull/36413 \ No newline at end of file diff --git a/config/config.sample.php b/config/config.sample.php index b76335f71927..94bd3e64efe5 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -82,6 +82,17 @@ */ 'datadirectory' => '/var/www/owncloud/data', +/** + * Define the directory where the crash logs will be stored. + * By default, this will be the same as the one configured as "datadirectory". + * The directory MUST EXIST and be WRITABLE by the web server. + * Note that crashes are extremely rare (although they can come in burst due to + * multiple requests), so the default location is usually fine. + * Also note that the log can contain sensitive information, but it should be useful + * to pinpoint where is the problem. + */ +'crashdirectory' => '/var/www/owncloud/data', + /** * Current version number of your ownCloud installation * This is set up during installation and update, so you shouldn't need to change it. diff --git a/console.php b/console.php index 92365fa227a6..72f139f7639d 100644 --- a/console.php +++ b/console.php @@ -56,9 +56,19 @@ } function exceptionHandler($exception) { - echo 'An unhandled exception has been thrown:' . PHP_EOL; - echo $exception; - exit(1); + try { + // try to log the exception + \OC::$server->getLogger()->logException($ex, ['app' => 'index']); + } catch (\Throwable $ex) { + // if we can't log normally, use the crashLog + \OC::crashLog($exception); + \OC::crashLog($ex); + } finally { + // always show the exception in the console + echo 'An unhandled exception has been thrown:' . PHP_EOL; + echo $exception; + exit(1); + } } try { require_once __DIR__ . '/lib/base.php'; @@ -104,8 +114,6 @@ function exceptionHandler($exception) { $application = new Application(\OC::$server->getConfig(), \OC::$server->getEventDispatcher(), \OC::$server->getRequest()); $application->loadCommands(new ArgvInput(), new ConsoleOutput()); $application->run(); -} catch (Exception $ex) { - exceptionHandler($ex); -} catch (Error $ex) { +} catch (\Throwable $ex) { exceptionHandler($ex); } diff --git a/index.php b/index.php index 0e36dddc8ed6..3621bc3e5343 100644 --- a/index.php +++ b/index.php @@ -67,23 +67,20 @@ } catch (\OCP\Files\ForbiddenException $ex) { OC_Response::setStatus(OC_Response::STATUS_FORBIDDEN); OC_Template::printErrorPage($ex->getMessage()); -} catch (Exception $ex) { +} catch (\Throwable $ex) { try { \OC::$server->getLogger()->logException($ex, ['app' => 'index']); //show the user a detailed error page OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); OC_Template::printExceptionErrorPage($ex); - } catch (\Exception $ex2) { + } catch (\Throwable $ex2) { // with some env issues, it can happen that the logger couldn't log properly, // so print out the exception directly - echo(''); - echo('Exception occurred while logging exception: ' . $ex->getMessage() . '
'); - echo(\str_replace("\n", '
', $ex->getTraceAsString())); - echo(''); + // NOTE: If we've reached this point, something has gone really wrong because + // we couldn't even get the logger, so don't rely on ownCloud here. + \header("{$_SERVER['SERVER_PROTOCOL']} 599 Broken"); + \OC::crashLog($ex); + \OC::crashLog($ex2); } -} catch (Error $ex) { - \OC::$server->getLogger()->logException($ex, ['app' => 'index']); - OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); - OC_Template::printExceptionErrorPage($ex); } diff --git a/lib/base.php b/lib/base.php index 24346a921341..56c9d643473c 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1000,6 +1000,59 @@ protected static function handleAuthHeaders() { } } } + + /** + * Log an ownCloud's crash. This means that ownCloud isn't usable and can't log through + * normal ownCloud's logger facilities. + * This crash log can't use the normal "owncloud.log" file because the ownCloud's logger + * requires additional context that can't be easily replicated. We'll use a different file. + * The crash log will create a "crash-Y-m-d.log" file in the ownCloud's data directory, unless + * the "crashdirectory" is set in the config.php file (make sure the "crashdirectory" exists and + * it's writeable for the web server). The filename will be reused for all the crashes that happen + * during the same day, and a new one will be used the next day. + * The crash file will be created only if needed. + * + * Note: This is mainly for internal purposes. You're encouraged to use the ownCloud's logger + * for anything you need to log. + */ + public static function crashLog(\Throwable $ex) { + $dataDir = self::$config->getValue('datadirectory', self::$SERVERROOT . '/data'); + $crashDir = self::$config->getValue('crashdirectory', $dataDir); + + $filename = "${crashDir}/crash-" . \date('Y-m-d') . '.log'; + + $date = \date('c'); + $currentEntryId = \uniqid(\md5($date), true); + $entry = [ + 'date' => $date, + 'parentId' => null, + 'id' => $currentEntryId, + 'class' => \get_class($ex), + 'message' => $ex->getMessage(), + 'stacktrace' => \array_map(function ($elem) { + unset($elem['args'], $elem['type']); + return $elem; + }, $ex->getTrace()), + ]; + \file_put_contents($filename, \json_encode($entry, JSON_PRETTY_PRINT) . PHP_EOL, FILE_APPEND | LOCK_EX); + + while (($ex = $ex->getPrevious()) !== null) { + $previousEntryId = $currentEntryId; + $currentEntryId = \uniqid(\md5($date), true); + $entry = [ + 'date' => $date, + 'parentId' => $previousEntryId, + 'id' => $currentEntryId, + 'class' => \get_class($ex), + 'message' => $ex->getMessage(), + 'stacktrace' => \array_map(function ($elem) { + unset($elem['args'], $elem['type']); + return $elem; + }, $ex->getTrace()), + ]; + \file_put_contents($filename, \json_encode($entry, JSON_PRETTY_PRINT) . PHP_EOL, FILE_APPEND | LOCK_EX); + } + } } OC::init(); diff --git a/lib/private/Log/ErrorHandler.php b/lib/private/Log/ErrorHandler.php index ad5c70624b18..7c0075cc786d 100644 --- a/lib/private/Log/ErrorHandler.php +++ b/lib/private/Log/ErrorHandler.php @@ -50,9 +50,6 @@ public static function register($debug=false) { } else { \set_error_handler([$handler, 'onError']); } - \OC::$server->getShutdownHandler()->register(function () use ($handler) { - $handler->onShutdown(); - }); \set_exception_handler([$handler, 'onException']); } @@ -60,16 +57,6 @@ public static function setLogger(ILogger $logger) { self::$logger = $logger; } - //Fatal errors handler - public static function onShutdown() { - $error = \error_get_last(); - if ($error && self::$logger) { - //ob_end_clean(); - $msg = $error['message'] . ' at ' . $error['file'] . '#' . $error['line']; - self::$logger->critical(self::removePassword($msg), ['app' => 'PHP']); - } - } - /** * Uncaught exception handler * diff --git a/lib/private/Setup.php b/lib/private/Setup.php index ce6a05697740..a93ab4b8d11c 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -480,6 +480,7 @@ public static function updateHtaccess() { if ($rewriteBase !== '') { $content .= "\n"; $content .= "\n Options -MultiViews"; + $content .= "\n RewriteRule ^favicon.ico$ core/img/favicon.ico [L]"; $content .= "\n RewriteRule ^core/js/oc.js$ index.php [PT,E=PATH_INFO:$1]"; $content .= "\n RewriteRule ^core/preview.png$ index.php [PT,E=PATH_INFO:$1]"; $content .= "\n RewriteCond %{REQUEST_FILENAME} !\\.(css|js|svg|gif|png|html|ttf|woff|ico|jpg|jpeg|json)$"; diff --git a/public.php b/public.php index 29820fcf7e1c..5431a988f568 100644 --- a/public.php +++ b/public.php @@ -73,18 +73,20 @@ $baseuri = OC::$WEBROOT . '/public.php/' . $service . '/'; require_once OC_App::getAppPath($app) . '/' . $parts[1]; -} catch (Exception $ex) { - if ($ex instanceof \OC\ServiceUnavailableException) { - OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE); - } else { - OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); +} catch (\Throwable $ex) { + try { + if ($ex instanceof \OC\ServiceUnavailableException) { + OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE); + } else { + OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); + } + //show the user a detailed error page + \OC::$server->getLogger()->logException($ex, ['app' => 'public']); + OC_Template::printExceptionErrorPage($ex); + } catch (\Throwable $ex2) { + // log through the crashLog + \header("{$_SERVER['SERVER_PROTOCOL']} 599 Broken"); + \OC::crashLog($ex); + \OC::crashLog($ex2); } - //show the user a detailed error page - \OC::$server->getLogger()->logException($ex, ['app' => 'public']); - OC_Template::printExceptionErrorPage($ex); -} catch (Error $ex) { - //show the user a detailed error page - OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); - \OC::$server->getLogger()->logException($ex, ['app' => 'public']); - OC_Template::printExceptionErrorPage($ex); } diff --git a/remote.php b/remote.php index 87d67db8dd4a..967902855c1f 100644 --- a/remote.php +++ b/remote.php @@ -163,8 +163,13 @@ function resolveService($service) { } $baseuri = OC::$WEBROOT . '/remote.php/'.$service.'/'; require_once $file; -} catch (Exception $ex) { - handleException($ex); -} catch (Error $e) { - handleException($e); +} catch (\Throwable $ex) { + try { + handleException($ex); + } catch (\Throwable $ex2) { + // log through the crashLog + \header("{$_SERVER['SERVER_PROTOCOL']} 599 Broken"); + \OC::crashLog($ex); + \OC::crashLog($ex2); + } } diff --git a/status.php b/status.php index 93955d83c577..0f3a5c11a16f 100644 --- a/status.php +++ b/status.php @@ -48,7 +48,14 @@ \header('Content-Type: application/json'); echo \json_encode($values); } -} catch (Exception $ex) { - OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); - \OCP\Util::writeLog('remote', $ex->getMessage(), \OCP\Util::FATAL); +} catch (\Throwable $ex) { + try { + OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); + \OCP\Util::writeLog('remote', $ex->getMessage(), \OCP\Util::FATAL); + } catch (\Throwable $ex2) { + // log through the crashLog + \header("{$_SERVER['SERVER_PROTOCOL']} 599 Broken"); + \OC::crashLog($ex); + \OC::crashLog($ex2); + } }