owncloud · micbar · Dec 4, 2019 · Nov 12, 2019 · Nov 12, 2019 · Nov 12, 2019
diff --git a/changelog/unreleased/36413 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
diff --git 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
@@ -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
@@ -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);
 }
diff --git 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
@@ -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
 	 *

diff --git a/lib/private/Setup.php b/lib/private/Setup.php
@@ -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)$";

diff --git 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
@@ -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
@@ -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);
+	}
 }