Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[For 10.4] Set 599 HTTP code on error #36413

Merged
merged 5 commits into from
Dec 4, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/unreleased/36413
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions config/config.sample.php
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 14 additions & 6 deletions console.php
Original file line number Diff line number Diff line change
@@ -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);
}
17 changes: 7 additions & 10 deletions index.php
Original file line number Diff line number Diff line change
@@ -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('<html><body>');
echo('Exception occurred while logging exception: ' . $ex->getMessage() . '<br/>');
echo(\str_replace("\n", '<br/>', $ex->getTraceAsString()));
echo('</body></html>');
// 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);
}
53 changes: 53 additions & 0 deletions lib/base.php
Original file line number Diff line number Diff line change
@@ -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();
13 changes: 0 additions & 13 deletions lib/private/Log/ErrorHandler.php
Original file line number Diff line number Diff line change
@@ -50,26 +50,13 @@ 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']);
}

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
*
1 change: 1 addition & 0 deletions lib/private/Setup.php
Original file line number Diff line number Diff line change
@@ -480,6 +480,7 @@ public static function updateHtaccess() {
if ($rewriteBase !== '') {
$content .= "\n<IfModule mod_rewrite.c>";
$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)$";
28 changes: 15 additions & 13 deletions public.php
Original file line number Diff line number Diff line change
@@ -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);
}
13 changes: 9 additions & 4 deletions remote.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
13 changes: 10 additions & 3 deletions status.php
Original file line number Diff line number Diff line change
@@ -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);
}
}