From bc8632856a1d2588ba8675a768ed6fe700677e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Tue, 9 Feb 2016 13:58:13 +0100 Subject: [PATCH] Forward exception message to the admin in case of errors and in case the remote server version is to low and appropriate message is displayed as well --- .../controller/settingscontroller.php | 1 - apps/federation/lib/trustedservers.php | 36 ++++++++--------- .../middleware/addservermiddleware.php | 2 +- .../tests/lib/trustedserverstest.php | 40 ++++++++++++------- .../middleware/addservermiddlewaretest.php | 3 +- 5 files changed, 46 insertions(+), 36 deletions(-) diff --git a/apps/federation/controller/settingscontroller.php b/apps/federation/controller/settingscontroller.php index e5e46606f1..3adb6fced6 100644 --- a/apps/federation/controller/settingscontroller.php +++ b/apps/federation/controller/settingscontroller.php @@ -26,7 +26,6 @@ use OCA\Federation\TrustedServers; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; -use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; diff --git a/apps/federation/lib/trustedservers.php b/apps/federation/lib/trustedservers.php index e3ce8228cc..340accfdbd 100644 --- a/apps/federation/lib/trustedservers.php +++ b/apps/federation/lib/trustedservers.php @@ -23,6 +23,7 @@ namespace OCA\Federation; +use OC\HintException; use OCP\AppFramework\Http; use OCP\BackgroundJob\IJobList; use OCP\Http\Client\IClientService; @@ -202,34 +203,33 @@ class TrustedServers { public function isOwnCloudServer($url) { $isValidOwnCloud = false; $client = $this->httpClientService->newClient(); - try { - $result = $client->get( - $url . '/status.php', - [ - 'timeout' => 3, - 'connect_timeout' => 3, - ] - ); - if ($result->getStatusCode() === Http::STATUS_OK) { - $isValidOwnCloud = $this->checkOwnCloudVersion($result->getBody()); - } - } catch (\Exception $e) { - $this->logger->error($e->getMessage(), ['app' => 'federation']); - return false; + $result = $client->get( + $url . '/status.php', + [ + 'timeout' => 3, + 'connect_timeout' => 3, + ] + ); + if ($result->getStatusCode() === Http::STATUS_OK) { + $isValidOwnCloud = $this->checkOwnCloudVersion($result->getBody()); } + return $isValidOwnCloud; } /** * check if ownCloud version is >= 9.0 * - * @param $statusphp + * @param $status * @return bool */ - protected function checkOwnCloudVersion($statusphp) { - $decoded = json_decode($statusphp, true); + protected function checkOwnCloudVersion($status) { + $decoded = json_decode($status, true); if (!empty($decoded) && isset($decoded['version'])) { - return version_compare($decoded['version'], '9.0.0', '>='); + if (!version_compare($decoded['version'], '9.0.0', '>=')) { + throw new HintException('Remote server version is too low. ownCloud 9.0 is required.'); + } + return true; } return false; } diff --git a/apps/federation/middleware/addservermiddleware.php b/apps/federation/middleware/addservermiddleware.php index cd9ccff440..10abd9db70 100644 --- a/apps/federation/middleware/addservermiddleware.php +++ b/apps/federation/middleware/addservermiddleware.php @@ -58,7 +58,7 @@ class AddServerMiddleware extends Middleware { if ($exception instanceof HintException) { $message = $exception->getHint(); } else { - $message = $this->l->t('Unknown error'); + $message = $exception->getMessage(); } return new JSONResponse( diff --git a/apps/federation/tests/lib/trustedserverstest.php b/apps/federation/tests/lib/trustedserverstest.php index e57391ed19..130a0e3bb2 100644 --- a/apps/federation/tests/lib/trustedserverstest.php +++ b/apps/federation/tests/lib/trustedserverstest.php @@ -23,6 +23,7 @@ namespace OCA\Federation\Tests\lib; +use OC\HintException; use OCA\Federation\DbHandler; use OCA\Federation\TrustedServers; use OCP\BackgroundJob\IJobList; @@ -282,41 +283,50 @@ class TrustedServersTest extends TestCase { ]; } + /** + * @expectedException \Exception + * @expectedExceptionMessage simulated exception + */ public function testIsOwnCloudServerFail() { $server = 'server1'; $this->httpClientService->expects($this->once())->method('newClient') ->willReturn($this->httpClient); - $this->logger->expects($this->once())->method('error') - ->with('simulated exception', ['app' => 'federation']); - $this->httpClient->expects($this->once())->method('get')->with($server . '/status.php') ->willReturnCallback(function () { throw new \Exception('simulated exception'); }); - $this->assertFalse($this->trustedServers->isOwnCloudServer($server)); - + $this->trustedServers->isOwnCloudServer($server); } /** * @dataProvider dataTestCheckOwnCloudVersion - * - * @param $statusphp - * @param $expected */ - public function testCheckOwnCloudVersion($statusphp, $expected) { - $this->assertSame($expected, - $this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$statusphp]) - ); + public function testCheckOwnCloudVersion($status) { + $this->assertTrue($this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$status])); } public function dataTestCheckOwnCloudVersion() { return [ - ['{"version":"8.4.0"}', false], - ['{"version":"9.0.0"}', true], - ['{"version":"9.1.0"}', true] + ['{"version":"9.0.0"}'], + ['{"version":"9.1.0"}'] + ]; + } + + /** + * @dataProvider dataTestCheckOwnCloudVersionTooLow + * @expectedException \OC\HintException + * @expectedExceptionMessage Remote server version is too low. ownCloud 9.0 is required. + */ + public function testCheckOwnCloudVersionTooLow($status) { + $this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$status]); + } + + public function dataTestCheckOwnCloudVersionTooLow() { + return [ + ['{"version":"8.2.3"}'], ]; } diff --git a/apps/federation/tests/middleware/addservermiddlewaretest.php b/apps/federation/tests/middleware/addservermiddlewaretest.php index a94d907ae7..49e34cc73d 100644 --- a/apps/federation/tests/middleware/addservermiddlewaretest.php +++ b/apps/federation/tests/middleware/addservermiddlewaretest.php @@ -27,6 +27,7 @@ use OC\HintException; use OCA\Federation\Middleware\AddServerMiddleware; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\ILogger; use Test\TestCase; class AddServerMiddlewareTest extends TestCase { @@ -93,7 +94,7 @@ class AddServerMiddlewareTest extends TestCase { public function dataTestAfterException() { return [ [new HintException('message', 'hint'), 'message', 'hint'], - [new \Exception('message'), 'message', 'Unknown error'], + [new \Exception('message'), 'message', 'message'], ]; }