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.

Sign up for GitHub

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

Server error/hint pages with a 500 error code to avoid it being seen … #9995

Merged
merged 2 commits into from
Jun 26, 2018
Merged

Server error/hint pages with a 500 error code to avoid it being seen … #9995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c155cd
Server error/hint pages with a 500 error code to avoid it being seen …
MorrisJobke Jun 26, 2018
1399f6b
Server exception error pages by default with a 500 status code
MorrisJobke Jun 26, 2018
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
15 changes: 5 additions & 10 deletions index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,19 @@
\OC::$server->getLogger()->logException($ex, array('app' => 'index'));

//show the user a detailed error page
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
OC_Template::printExceptionErrorPage($ex);
OC_Template::printExceptionErrorPage($ex, \OC_Response::STATUS_SERVICE_UNAVAILABLE);
} catch (\OC\HintException $ex) {
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
try {
OC_Template::printErrorPage($ex->getMessage(), $ex->getHint());
OC_Template::printErrorPage($ex->getMessage(), $ex->getHint(), OC_Response::STATUS_SERVICE_UNAVAILABLE);
} catch (Exception $ex2) {
\OC::$server->getLogger()->logException($ex, array('app' => 'index'));
\OC::$server->getLogger()->logException($ex2, array('app' => 'index'));

//show the user a detailed error page
OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR);
OC_Template::printExceptionErrorPage($ex);
OC_Template::printExceptionErrorPage($ex, \OC_Response::STATUS_INTERNAL_SERVER_ERROR);
}
} catch (\OC\User\LoginException $ex) {
OC_Response::setStatus(OC_Response::STATUS_FORBIDDEN);
OC_Template::printErrorPage($ex->getMessage(), $ex->getMessage());
OC_Template::printErrorPage($ex->getMessage(), $ex->getMessage(), OC_Response::STATUS_FORBIDDEN);
} catch (Exception $ex) {
\OC::$server->getLogger()->logException($ex, array('app' => 'index'));

Expand Down Expand Up @@ -92,6 +88,5 @@

throw $e;
}
OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR);
OC_Template::printExceptionErrorPage($ex);
OC_Template::printExceptionErrorPage($ex, \OC_Response::STATUS_INTERNAL_SERVER_ERROR);
}
11 changes: 5 additions & 6 deletions lib/base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ public static function checkConfig() {
$l->t('This can usually be fixed by giving the webserver write access to the config directory. See %s',
[ $urlGenerator->linkToDocs('admin-dir_permissions') ]) . '. '
. $l->t('Or, if you prefer to keep config.php file read only, set the option "config_is_read_only" to true in it. See %s',
[ $urlGenerator->linkToDocs('admin-config') ] )
[ $urlGenerator->linkToDocs('admin-config') ] ),
\OC_Response::STATUS_SERVICE_UNAVAILABLE
);
}
}
Expand Down Expand Up @@ -433,8 +434,7 @@ public static function initSession() {
} catch (Exception $e) {
\OC::$server->getLogger()->logException($e, ['app' => 'base']);
//show the user a detailed error page
OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR);
OC_Template::printExceptionErrorPage($e);
OC_Template::printExceptionErrorPage($e, \OC_Response::STATUS_INTERNAL_SERVER_ERROR);
die();
}

Expand Down Expand Up @@ -750,11 +750,10 @@ public static function init() {
// Check whether the sample configuration has been copied
if($systemConfig->getValue('copied_sample_config', false)) {
$l = \OC::$server->getL10N('lib');
header('HTTP/1.1 503 Service Temporarily Unavailable');
header('Status: 503 Service Temporarily Unavailable');
OC_Template::printErrorPage(
$l->t('Sample configuration detected'),
$l->t('It has been detected that the sample configuration has been copied. This can break your installation and is unsupported. Please read the documentation before performing changes on config.php')
$l->t('It has been detected that the sample configuration has been copied. This can break your installation and is unsupported. Please read the documentation before performing changes on config.php'),
\OC_Response::STATUS_SERVICE_UNAVAILABLE
);
return;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/private/legacy/files.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,18 +198,18 @@ public static function get($dir, $files, $params = null) {
OC::$server->getLogger()->logException($ex);
$l = \OC::$server->getL10N('core');
$hint = method_exists($ex, 'getHint') ? $ex->getHint() : '';
\OC_Template::printErrorPage($l->t('File is currently busy, please try again later'), $hint);
\OC_Template::printErrorPage($l->t('File is currently busy, please try again later'), $hint, 200);
} catch (\OCP\Files\ForbiddenException $ex) {
self::unlockAllTheFiles($dir, $files, $getType, $view, $filename);
OC::$server->getLogger()->logException($ex);
$l = \OC::$server->getL10N('core');
\OC_Template::printErrorPage($l->t('Can\'t read file'), $ex->getMessage());
\OC_Template::printErrorPage($l->t('Can\'t read file'), $ex->getMessage(), 200);
} catch (\Exception $ex) {
self::unlockAllTheFiles($dir, $files, $getType, $view, $filename);
OC::$server->getLogger()->logException($ex);
$l = \OC::$server->getL10N('core');
$hint = method_exists($ex, 'getHint') ? $ex->getHint() : '';
\OC_Template::printErrorPage($l->t('Can\'t read file'), $hint);
\OC_Template::printErrorPage($l->t('Can\'t read file'), $hint, 200);
}
}

Expand Down
36 changes: 6 additions & 30 deletions lib/private/legacy/template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,10 @@ public static function printGuestPage( $application, $name, $parameters = array(
* Print a fatal error page and terminates the script
* @param string $error_msg The error message to show
* @param string $hint An optional hint message - needs to be properly escape
* @param int $statusCode
* @suppress PhanAccessMethodInternal
*/
public static function printErrorPage( $error_msg, $hint = '' ) {
public static function printErrorPage( $error_msg, $hint = '', $statusCode = 500) {
if (\OC::$server->getAppManager()->isEnabledForUser('theming') && !\OC_App::isAppLoaded('theming')) {
\OC_App::loadApp('theming');
}
Expand All @@ -317,6 +318,7 @@ public static function printErrorPage( $error_msg, $hint = '' ) {
$hint = '';
}

http_response_code($statusCode);
try {
$content = new \OC_Template( '', 'error', 'error', false );
$errors = array(array('error' => $error_msg, 'hint' => $hint));
Expand All @@ -327,7 +329,6 @@ public static function printErrorPage( $error_msg, $hint = '' ) {
$logger->error("$error_msg $hint", ['app' => 'core']);
$logger->logException($e, ['app' => 'core']);

header(self::getHttpProtocol() . ' 500 Internal Server Error');
header('Content-Type: text/plain; charset=utf-8');
print("$error_msg $hint");
}
Expand All @@ -337,11 +338,12 @@ public static function printErrorPage( $error_msg, $hint = '' ) {
/**
* print error page using Exception details
* @param Exception|Throwable $exception
* @param bool $fetchPage
* @param int $statusCode
* @return bool|string
* @suppress PhanAccessMethodInternal
*/
public static function printExceptionErrorPage($exception, $fetchPage = false) {
public static function printExceptionErrorPage($exception, $statusCode = 503) {
http_response_code($statusCode);
try {
$request = \OC::$server->getRequest();
$content = new \OC_Template('', 'exception', 'error', false);
Expand All @@ -354,16 +356,12 @@ public static function printExceptionErrorPage($exception, $fetchPage = false) {
$content->assign('debugMode', \OC::$server->getSystemConfig()->getValue('debug', false));
$content->assign('remoteAddr', $request->getRemoteAddress());
$content->assign('requestID', $request->getId());
if ($fetchPage) {
return $content->fetchPage();
}
$content->printPage();
} catch (\Exception $e) {
$logger = \OC::$server->getLogger();
$logger->logException($exception, ['app' => 'core']);
$logger->logException($e, ['app' => 'core']);

header(self::getHttpProtocol() . ' 500 Internal Server Error');
header('Content-Type: text/plain; charset=utf-8');
print("Internal Server Error\n\n");
print("The server encountered an internal error and was unable to complete your request.\n");
Expand All @@ -372,26 +370,4 @@ public static function printExceptionErrorPage($exception, $fetchPage = false) {
}
die();
}

/**
* This is only here to reduce the dependencies in case of an exception to
* still be able to print a plain error message.
*
* Returns the used HTTP protocol.
*
* @return string HTTP protocol. HTTP/2, HTTP/1.1 or HTTP/1.0.
* @internal Don't use this - use AppFramework\Http\Request->getHttpProtocol instead
*/
protected static function getHttpProtocol() {
$claimedProtocol = strtoupper($_SERVER['SERVER_PROTOCOL']);
$validProtocols = [
'HTTP/1.0',
'HTTP/1.1',
'HTTP/2',
];
if(in_array($claimedProtocol, $validProtocols, true)) {
return $claimedProtocol;
}
return 'HTTP/1.1';
}
}
12 changes: 5 additions & 7 deletions public.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
if (\OCP\Util::needUpgrade()) {
// since the behavior of apps or remotes are unpredictable during
// an upgrade, return a 503 directly
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
OC_Template::printErrorPage('Service unavailable');
OC_Template::printErrorPage('Service unavailable', '', OC_Response::STATUS_SERVICE_UNAVAILABLE);
exit;
}

Expand Down Expand Up @@ -80,16 +79,15 @@

} catch (Exception $ex) {
if ($ex instanceof \OC\ServiceUnavailableException) {
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
$status = OC_Response::STATUS_SERVICE_UNAVAILABLE;
} else {
OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR);
$status = OC_Response::STATUS_INTERNAL_SERVER_ERROR;
}
//show the user a detailed error page
\OC::$server->getLogger()->logException($ex, ['app' => 'public']);
OC_Template::printExceptionErrorPage($ex);
OC_Template::printExceptionErrorPage($ex, $status);
} 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);
OC_Template::printExceptionErrorPage($ex, OC_Response::STATUS_INTERNAL_SERVER_ERROR);
}
6 changes: 2 additions & 4 deletions remote.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,10 @@ function handleException($e) {
}
if ($e instanceof RemoteException) {
// we shall not log on RemoteException
OC_Response::setStatus($e->getCode());
OC_Template::printErrorPage($e->getMessage());
OC_Template::printErrorPage($e->getMessage(), '', $e->getCode());
} else {
\OC::$server->getLogger()->logException($e, ['app' => 'remote']);
OC_Response::setStatus($statusCode);
OC_Template::printExceptionErrorPage($e);
OC_Template::printExceptionErrorPage($e, $statusCode);
}
}
}
Expand Down