Do not print exception message

In case of an error the error message often contains sensitive data such as the full path which potentially leads to a full path disclosure.

Thus the error message should not directly get displayed to the user and instead be logged.
This commit is contained in:
Lukas Reschke 2015-10-13 14:12:10 +02:00
parent 3891cd9068
commit abdbf10ebc
3 changed files with 64 additions and 15 deletions

View File

@ -85,7 +85,8 @@ class Application extends App {
$c->query('L10N'), $c->query('L10N'),
$c->query('UserManager'), $c->query('UserManager'),
$c->query('UserSession'), $c->query('UserSession'),
$c->query('UserFolder') $c->query('UserFolder'),
$c->query('Logger')
); );
}); });
@ -128,6 +129,9 @@ class Application extends App {
$container->registerService('Mailer', function(SimpleContainer $c) { $container->registerService('Mailer', function(SimpleContainer $c) {
return $c->query('ServerContainer')->getMailer(); return $c->query('ServerContainer')->getMailer();
}); });
$container->registerService('Logger', function(SimpleContainer $c) {
return $c->query('ServerContainer')->getLogger();
});
$container->registerService('TimeFactory', function(SimpleContainer $c) { $container->registerService('TimeFactory', function(SimpleContainer $c) {
return new TimeFactory(); return new TimeFactory();
}); });

View File

@ -30,7 +30,7 @@ use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\IAvatarManager; use OCP\IAvatarManager;
use OCP\ICache; use OCP\ILogger;
use OCP\IL10N; use OCP\IL10N;
use OCP\IRequest; use OCP\IRequest;
use OCP\IUserManager; use OCP\IUserManager;
@ -62,6 +62,9 @@ class AvatarController extends Controller {
/** @var Folder */ /** @var Folder */
protected $userFolder; protected $userFolder;
/** @var ILogger */
protected $logger;
/** /**
* @param string $appName * @param string $appName
* @param IRequest $request * @param IRequest $request
@ -71,6 +74,7 @@ class AvatarController extends Controller {
* @param IUserManager $userManager * @param IUserManager $userManager
* @param IUserSession $userSession * @param IUserSession $userSession
* @param Folder $userFolder * @param Folder $userFolder
* @param ILogger $logger
*/ */
public function __construct($appName, public function __construct($appName,
IRequest $request, IRequest $request,
@ -79,7 +83,8 @@ class AvatarController extends Controller {
IL10N $l10n, IL10N $l10n,
IUserManager $userManager, IUserManager $userManager,
IUserSession $userSession, IUserSession $userSession,
Folder $userFolder) { Folder $userFolder,
ILogger $logger) {
parent::__construct($appName, $request); parent::__construct($appName, $request);
$this->avatarManager = $avatarManager; $this->avatarManager = $avatarManager;
@ -88,6 +93,7 @@ class AvatarController extends Controller {
$this->userManager = $userManager; $this->userManager = $userManager;
$this->userSession = $userSession; $this->userSession = $userSession;
$this->userFolder = $userFolder; $this->userFolder = $userFolder;
$this->logger = $logger;
} }
/** /**
@ -218,11 +224,8 @@ class AvatarController extends Controller {
); );
} }
} catch (\Exception $e) { } catch (\Exception $e) {
return new DataResponse( $this->logger->logException($e, ['app' => 'core']);
['data' => ['message' => $e->getMessage()]], return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_OK, $headers);
Http::STATUS_OK,
$headers
);
} }
} }
@ -239,7 +242,8 @@ class AvatarController extends Controller {
$avatar->remove(); $avatar->remove();
return new DataResponse(); return new DataResponse();
} catch (\Exception $e) { } catch (\Exception $e) {
return new DataResponse(['data' => ['message' => $e->getMessage()]], Http::STATUS_BAD_REQUEST); $this->logger->logException($e, ['app' => 'core']);
return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
} }
} }
@ -307,10 +311,9 @@ class AvatarController extends Controller {
} catch (\OC\NotSquareException $e) { } catch (\OC\NotSquareException $e) {
return new DataResponse(['data' => ['message' => $this->l->t('Crop is not square')]], return new DataResponse(['data' => ['message' => $this->l->t('Crop is not square')]],
Http::STATUS_BAD_REQUEST); Http::STATUS_BAD_REQUEST);
} catch (\Exception $e) { } catch (\Exception $e) {
return new DataResponse(['data' => ['message' => $e->getMessage()]], $this->logger->logException($e, ['app' => 'core']);
Http::STATUS_BAD_REQUEST); return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
} }
} }
} }

View File

@ -76,6 +76,8 @@ class AvatarControllerTest extends \Test\TestCase {
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->container['UserFolder'] = $this->getMockBuilder('OCP\Files\Folder') $this->container['UserFolder'] = $this->getMockBuilder('OCP\Files\Folder')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
$this->container['Logger'] = $this->getMockBuilder('OCP\ILogger')
->disableOriginalConstructor()->getMock();
$this->avatarMock = $this->getMockBuilder('OCP\IAvatar') $this->avatarMock = $this->getMockBuilder('OCP\IAvatar')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();
@ -217,8 +219,11 @@ class AvatarControllerTest extends \Test\TestCase {
$this->avatarMock->method('remove')->will($this->throwException(new \Exception("foo"))); $this->avatarMock->method('remove')->will($this->throwException(new \Exception("foo")));
$this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock);
$response = $this->avatarController->deleteAvatar(); $this->container['Logger']->expects($this->once())
$this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus()); ->method('logException')
->with(new \Exception("foo"));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$this->assertEquals($expectedResponse, $this->avatarController->deleteAvatar());
} }
/** /**
@ -328,6 +333,26 @@ class AvatarControllerTest extends \Test\TestCase {
$this->assertEquals('notsquare', $response->getData()['data']); $this->assertEquals('notsquare', $response->getData()['data']);
} }
/**
* Test what happens if the upload of the avatar fails
*/
public function testPostAvatarException() {
$this->container['Cache']->expects($this->once())
->method('set')
->will($this->throwException(new \Exception("foo")));
$file = $this->getMockBuilder('OCP\Files\File')
->disableOriginalConstructor()->getMock();
$file->method('getContent')->willReturn(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
$this->container['UserFolder']->method('get')->willReturn($file);
$this->container['Logger']->expects($this->once())
->method('logException')
->with(new \Exception("foo"));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
}
/** /**
* Test invalid crop argument * Test invalid crop argument
*/ */
@ -371,6 +396,23 @@ class AvatarControllerTest extends \Test\TestCase {
$this->assertEquals('success', $response->getData()['status']); $this->assertEquals('success', $response->getData()['status']);
} }
/**
* Test what happens if the cropping of the avatar fails
*/
public function testPostCroppedAvatarException() {
$this->container['Cache']->method('get')->willReturn(file_get_contents(OC::$SERVERROOT.'/tests/data/testimage.jpg'));
$this->avatarMock->method('set')->will($this->throwException(new \Exception('foo')));
$this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock);
$this->container['Logger']->expects($this->once())
->method('logException')
->with(new \Exception('foo'));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$this->assertEquals($expectedResponse, $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]));
}
/** /**
* Check for proper reply on proper crop argument * Check for proper reply on proper crop argument
*/ */