Merge pull request #11019 from owncloud/do-not-show-exception-to-enduser

Do not show exception to the end-user - use a proper error page instead
This commit is contained in:
Lukas Reschke 2014-09-23 18:36:40 +02:00
commit 75e45ac786
8 changed files with 75 additions and 63 deletions

View File

@ -2,7 +2,9 @@
<?php foreach($_["errors"] as $error):?> <?php foreach($_["errors"] as $error):?>
<li class='error'> <li class='error'>
<?php p($error['error']) ?><br/> <?php p($error['error']) ?><br/>
<p class='hint'><?php if(isset($error['hint']))print_unescaped($error['hint']) ?></p> <?php if(isset($error['hint']) && $error['hint']): ?>
<p class='hint'><?php print_unescaped($error['hint']) ?></p>
<?php endif;?>
</li> </li>
<?php endforeach ?> <?php endforeach ?>
</ul> </ul>

View File

@ -0,0 +1,29 @@
<?php
/** @var array $_ */
/** @var OC_L10N $l */
?>
<span class="error error-wide">
<h2><strong><?php p('Internal Server Error') ?></strong></h2>
<p><?php p($l->t('The server encountered an internal error and was unable to complete your request.')) ?></p>
<p><?php p($l->t('Please contact the server administrator if this error reappears multiple times, please include the technical details below in your report.')) ?></p>
<p><?php p($l->t('More details can be found in the server log.')) ?></p>
<br />
<h2><strong><?php p($l->t('Technical details')) ?></strong></h2>
<ul>
<li><?php p($l->t('Remote Address: %s', $_['remoteAddr'])) ?></li>
<li><?php p($l->t('Request ID: %s', $_['requestID'])) ?></li>
<?php if($_['debugMode']): ?>
<li><?php p($l->t('Code: %s', $_['errorCode'])) ?></li>
<li><?php p($l->t('Message: %s', $_['errorMsg'])) ?></li>
<li><?php p($l->t('File: %s', $_['file'])) ?></li>
<li><?php p($l->t('Line: %s', $_['line'])) ?></li>
<?php endif; ?>
</ul>
<?php if($_['debugMode']): ?>
<br />
<h2><strong><?php p($l->t('Trace')) ?></strong></h2>
<pre><?php p($_['trace']) ?></pre>
<?php endif; ?>
</span>

View File

@ -33,6 +33,9 @@ try {
//show the user a detailed error page //show the user a detailed error page
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE); OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
OC_Template::printExceptionErrorPage($ex); OC_Template::printExceptionErrorPage($ex);
} catch (\OC\HintException $ex) {
OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE);
OC_Template::printErrorPage($ex->getMessage(), $ex->getHint());
} catch (Exception $ex) { } catch (Exception $ex) {
\OCP\Util::logException('index', $ex); \OCP\Util::logException('index', $ex);

View File

@ -12,7 +12,7 @@ class HintException extends \Exception {
private $hint; private $hint;
public function __construct($message, $hint = '', $code = 0, Exception $previous = null) { public function __construct($message, $hint = '', $code = 0, \Exception $previous = null) {
$this->hint = $hint; $this->hint = $hint;
parent::__construct($message, $code, $previous); parent::__construct($message, $code, $previous);
} }

View File

@ -28,7 +28,6 @@
class OC_Log_Owncloud { class OC_Log_Owncloud {
static protected $logFile; static protected $logFile;
static protected $reqId;
/** /**
* Init class data * Init class data
@ -69,19 +68,17 @@ class OC_Log_Owncloud {
$timezone = new DateTimeZone('UTC'); $timezone = new DateTimeZone('UTC');
} }
$time = new DateTime(null, $timezone); $time = new DateTime(null, $timezone);
$reqId = \OC_Request::getRequestID();
$remoteAddr = \OC_Request::getRemoteAddress();
// remove username/passwords from URLs before writing the to the log file // remove username/passwords from URLs before writing the to the log file
$time = $time->format($format); $time = $time->format($format);
if($minLevel == OC_Log::DEBUG) { if($minLevel == OC_Log::DEBUG) {
if(empty(self::$reqId)) {
self::$reqId = uniqid();
}
$reqId = self::$reqId;
$url = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '--'; $url = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '--';
$method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : '--'; $method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : '--';
$entry = compact('reqId', 'app', 'message', 'level', 'time', 'method', 'url'); $entry = compact('reqId', 'remoteAddr', 'app', 'message', 'level', 'time', 'method', 'url');
} }
else { else {
$entry = compact('app', 'message', 'level', 'time'); $entry = compact('reqId', 'remoteAddr', 'app', 'message', 'level', 'time');
} }
$entry = json_encode($entry); $entry = json_encode($entry);
$handle = @fopen(self::$logFile, 'a'); $handle = @fopen(self::$logFile, 'a');

View File

@ -14,6 +14,7 @@ class OC_Request {
const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#'; const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#';
const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)(:[0-9]+|)$/'; const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)(:[0-9]+|)$/';
static protected $reqId;
/** /**
* Returns the remote address, if the connection came from a trusted proxy and `forwarded_for_headers` has been configured * Returns the remote address, if the connection came from a trusted proxy and `forwarded_for_headers` has been configured
@ -22,7 +23,7 @@ class OC_Request {
* @return string IP address * @return string IP address
*/ */
public static function getRemoteAddress() { public static function getRemoteAddress() {
$remoteAddress = $_SERVER['REMOTE_ADDR']; $remoteAddress = isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '';
$trustedProxies = \OC::$server->getConfig()->getSystemValue('trusted_proxies', array()); $trustedProxies = \OC::$server->getConfig()->getSystemValue('trusted_proxies', array());
if(is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) { if(is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) {
@ -43,6 +44,17 @@ class OC_Request {
return $remoteAddress; return $remoteAddress;
} }
/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* @return string
*/
public static function getRequestID() {
if(self::$reqId === null) {
self::$reqId = hash('md5', microtime().\OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(20));
}
return self::$reqId;
}
/** /**
* Check overwrite condition * Check overwrite condition
* @param string $type * @param string $type

View File

@ -250,8 +250,7 @@ class OC_Template extends \OC\Template\Base {
/** /**
* Print a fatal error page and terminates the script * Print a fatal error page and terminates the script
* @param string $error_msg The error message to show * @param string $error_msg The error message to show
* @param string $hint An optional hint message * @param string $hint An optional hint message - needs to be properly escaped
* Warning: All data passed to $hint needs to get sanitized using OC_Util::sanitizeHTML
*/ */
public static function printErrorPage( $error_msg, $hint = '' ) { public static function printErrorPage( $error_msg, $hint = '' ) {
$content = new \OC_Template( '', 'error', 'error', false ); $content = new \OC_Template( '', 'error', 'error', false );
@ -266,28 +265,16 @@ class OC_Template extends \OC\Template\Base {
* @param Exception $exception * @param Exception $exception
*/ */
public static function printExceptionErrorPage(Exception $exception) { public static function printExceptionErrorPage(Exception $exception) {
$error_msg = $exception->getMessage(); $content = new \OC_Template('', 'exception', 'error', false);
if ($exception->getCode()) { $content->assign('errorMsg', $exception->getMessage());
$error_msg = '['.$exception->getCode().'] '.$error_msg; $content->assign('errorCode', $exception->getCode());
} $content->assign('file', $exception->getFile());
if (defined('DEBUG') and DEBUG) { $content->assign('line', $exception->getLine());
$hint = $exception->getTraceAsString(); $content->assign('trace', $exception->getTraceAsString());
if (!empty($hint)) { $content->assign('debugMode', defined('DEBUG') && DEBUG === true);
$hint = '<pre>'.OC_Util::sanitizeHTML($hint).'</pre>'; $content->assign('remoteAddr', OC_Request::getRemoteAddress());
} $content->assign('requestID', OC_Request::getRequestID());
while (method_exists($exception, 'previous') && $exception = $exception->previous()) { $content->printPage();
$error_msg .= '<br/>Caused by:' . ' '; die();
if ($exception->getCode()) {
$error_msg .= '['.OC_Util::sanitizeHTML($exception->getCode()).'] ';
}
$error_msg .= OC_Util::sanitizeHTML($exception->getMessage());
};
} else {
$hint = '';
if ($exception instanceof \OC\HintException) {
$hint = OC_Util::sanitizeHTML($exception->getHint());
}
}
self::printErrorPage($error_msg, $hint);
} }
} }

View File

@ -83,38 +83,20 @@ class Util {
} }
/** /**
* write exception into the log. Include the stack trace * write exception into the log
* if DEBUG mode is enabled
* @param string $app app name * @param string $app app name
* @param \Exception $ex exception to log * @param \Exception $ex exception to log
* @param string $level log level, defaults to \OCP\Util::FATAL * @param int $level log level, defaults to \OCP\Util::FATAL
*/ */
public static function logException( $app, \Exception $ex, $level = \OCP\Util::FATAL ) { public static function logException( $app, \Exception $ex, $level = \OCP\Util::FATAL ) {
$class = get_class($ex); $exception = array(
$message = $class . ': ' . $ex->getMessage(); 'Message' => $ex->getMessage(),
if ($ex->getCode()) { 'Code' => $ex->getCode(),
$message .= ' [' . $ex->getCode() . ']'; 'Trace' => $ex->getTraceAsString(),
} 'File' => $ex->getFile(),
\OCP\Util::writeLog($app, $message, $level); 'Line' => $ex->getLine(),
if (defined('DEBUG') and DEBUG) { );
// also log stack trace \OCP\Util::writeLog($app, 'Exception: ' . json_encode($exception), $level);
$stack = explode("\n", $ex->getTraceAsString());
// first element is empty
array_shift($stack);
foreach ($stack as $s) {
\OCP\Util::writeLog($app, 'Exception: ' . $s, $level);
}
// include cause
while (method_exists($ex, 'getPrevious') && $ex = $ex->getPrevious()) {
$message .= ' - Caused by:' . ' ';
$message .= $ex->getMessage();
if ($ex->getCode()) {
$message .= '[' . $ex->getCode() . '] ';
}
\OCP\Util::writeLog($app, 'Exception: ' . $message, $level);
}
}
} }
/** /**