Use IClientService to check for remote ownCloud instances

1. Allows to set a timeout (though still not perfect but way better than before)
2. Allows to have unit tests
3. I also added unit tests for the existing controller code
4. Corrected PHPDoc on IClient

apps/files_sharing/ajax/testremote.php

@@ -1,46 +0,0 @@
-<?php
-/**
- * @author Lukas Reschke <lukas@owncloud.com>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Robin Appelman <icewind@owncloud.com>
- *
- * @copyright Copyright (c) 2015, ownCloud, Inc.
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program.  If not, see <http://www.gnu.org/licenses/>
- *
- */
-
-OCP\JSON::callCheck();
-OCP\JSON::checkAppEnabled('files_sharing');
-
-$remote = $_GET['remote'];
-
-function testUrl($url) {
-	try {
-		$result = file_get_contents($url);
-		$data = json_decode($result);
-		// public link mount is only supported in ownCloud 7+
-		return is_object($data) and !empty($data->version) and version_compare($data->version, '7.0.0', '>=');
-	} catch (Exception $e) {
-		return false;
-	}
-}
-
-if (testUrl('https://' . $remote . '/status.php')) {
-	echo 'https';
-} elseif (testUrl('http://' . $remote . '/status.php')) {
-	echo 'http';
-} else {
-	echo 'false';
-}

apps/files_sharing/appinfo/application.php

@@ -62,7 +62,8 @@ class Application extends App {
 				$c->query('AppName'),
 				$c->query('Request'),
 				$c->query('IsIncomingShareEnabled'),
-				$c->query('ExternalManager')
+				$c->query('ExternalManager'),
+				$c->query('HttpClientService')
 			);
 		});
 
@@ -78,6 +79,9 @@ class Application extends App {
 		$container->registerService('UserManager', function (SimpleContainer $c) use ($server) {
 			return $server->getUserManager();
 		});
+		$container->registerService('HttpClientService', function (SimpleContainer $c) use ($server) {
+			return $server->getHTTPClientService();
+		});
 		$container->registerService('IsIncomingShareEnabled', function (SimpleContainer $c) {
 			return Helper::isIncomingServer2serverShareEnabled();
 		});

apps/files_sharing/appinfo/routes.php

@@ -33,7 +33,14 @@ $application = new Application();
 $application->registerRoutes($this, [
 	'resources' => [
 		'ExternalShares' => ['url' => '/api/externalShares'],
-	]
+	],
+	'routes' => [
+		[
+			'name' => 'externalShares#testRemote',
+			'url' => '/testremote',
+			'verb' => 'GET'
+		],
+	],
 ]);
 
 /** @var $this \OCP\Route\IRouter */
@@ -50,8 +57,6 @@ $this->create('sharing_external_shareinfo', '/shareinfo')
 	->actionInclude('files_sharing/ajax/shareinfo.php');
 $this->create('sharing_external_add', '/external')
 	->actionInclude('files_sharing/ajax/external.php');
-$this->create('sharing_external_test_remote', '/testremote')
-	->actionInclude('files_sharing/ajax/testremote.php');
 
 // OCS API
 

apps/files_sharing/lib/controllers/externalsharescontroller.php

@@ -23,11 +23,11 @@
 
 namespace OCA\Files_Sharing\Controllers;
 
-use OC;
-use OCP;
 use OCP\AppFramework\Controller;
 use OCP\IRequest;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\Http\Client\IClientService;
+use OCP\AppFramework\Http\DataResponse;
 
 /**
  * Class ExternalSharesController
@@ -40,20 +40,25 @@ class ExternalSharesController extends Controller {
 	private $incomingShareEnabled;
 	/** @var \OCA\Files_Sharing\External\Manager */
 	private $externalManager;
+	/** @var IClientService */
+	private $clientService;
 
 	/**
 	 * @param string $appName
 	 * @param IRequest $request
 	 * @param bool $incomingShareEnabled
 	 * @param \OCA\Files_Sharing\External\Manager $externalManager
+	 * @param IClientService $clientService
 	 */
 	public function __construct($appName,
 								IRequest $request,
 								$incomingShareEnabled,
-								\OCA\Files_Sharing\External\Manager $externalManager) {
+								\OCA\Files_Sharing\External\Manager $externalManager,
+								IClientService $clientService) {
 		parent::__construct($appName, $request);
 		$this->incomingShareEnabled = $incomingShareEnabled;
 		$this->externalManager = $externalManager;
+		$this->clientService = $clientService;
 	}
 
 	/**
@@ -97,4 +102,43 @@ class ExternalSharesController extends Controller {
 		return new JSONResponse();
 	}
 
+	/**
+	 * Test whether the specified remote is accessible
+	 *
+	 * @param string $remote
+	 * @return bool
+	 */
+	protected function testUrl($remote) {
+		try {
+			$client = $this->clientService->newClient();
+			$response = json_decode($client->get(
+				$remote,
+				[
+					'timeout' => 3,
+					'connect_timeout' => 3,
+				]
+			)->getBody());
+
+			return !empty($response->version) && version_compare($response->version, '7.0.0', '>=');
+		} catch (\Exception $e) {
+			return false;
+		}
+	}
+
+	/**
+	 * @PublicPage
+	 *
+	 * @param string $remote
+	 * @return DataResponse
+	 */
+	public function testRemote($remote) {
+		if ($this->testUrl('https://' . $remote . '/status.php')) {
+			return new DataResponse('https');
+		} elseif ($this->testUrl('http://' . $remote . '/status.php')) {
+			return new DataResponse('http');
+		} else {
+			return new DataResponse(false);
+		}
+	}
+
 }

apps/files_sharing/tests/controller/externalsharecontroller.php

@@ -0,0 +1,187 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Files_Sharing\Controllers;
+
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http\JSONResponse;
+use OCP\Http\Client\IClientService;
+use OCP\IRequest;
+
+/**
+ * Class ExternalShareControllerTest
+ *
+ * @package OCA\Files_Sharing\Controllers
+ */
+class ExternalShareControllerTest extends \Test\TestCase {
+	/** @var bool */
+	private $incomingShareEnabled;
+	/** @var IRequest */
+	private $request;
+	/** @var \OCA\Files_Sharing\External\Manager */
+	private $externalManager;
+	/** @var IClientService */
+	private $clientService;
+
+	public function setUp() {
+		$this->request = $this->getMockBuilder('\\OCP\\IRequest')
+			->disableOriginalConstructor()->getMock();
+		$this->externalManager = $this->getMockBuilder('\\OCA\\Files_Sharing\\External\\Manager')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService = $this->getMockBuilder('\\OCP\Http\\Client\\IClientService')
+			->disableOriginalConstructor()->getMock();
+	}
+
+	/**
+	 * @return ExternalSharesController
+	 */
+	public function getExternalShareController() {
+		return new ExternalSharesController(
+			'files_sharing',
+			$this->request,
+			$this->incomingShareEnabled,
+			$this->externalManager,
+			$this->clientService
+		);
+	}
+
+	public function testIndexDisabled() {
+		$this->externalManager
+			->expects($this->never())
+			->method('getOpenShares');
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->index());
+	}
+
+	public function testIndexEnabled() {
+		$this->incomingShareEnabled = true;
+		$this->externalManager
+			->expects($this->once())
+			->method('getOpenShares')
+			->will($this->returnValue(['MyDummyArray']));
+
+		$this->assertEquals(new JSONResponse(['MyDummyArray']), $this->getExternalShareController()->index());
+	}
+
+	public function testCreateDisabled() {
+		$this->externalManager
+			->expects($this->never())
+			->method('acceptShare');
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4));
+	}
+
+	public function testCreateEnabled() {
+		$this->incomingShareEnabled = true;
+		$this->externalManager
+			->expects($this->once())
+			->method('acceptShare')
+			->with(4);
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4));
+	}
+
+	public function testDestroyDisabled() {
+		$this->externalManager
+			->expects($this->never())
+			->method('destroy');
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy(4));
+	}
+
+	public function testDestroyEnabled() {
+		$this->incomingShareEnabled = true;
+		$this->externalManager
+			->expects($this->once())
+			->method('declineShare')
+			->with(4);
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy(4));
+	}
+
+	public function testRemoteWithValidHttps() {
+		$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
+			->disableOriginalConstructor()->getMock();
+		$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client
+			->expects($this->once())
+			->method('get')
+			->with(
+				'https://owncloud.org/status.php',
+				[
+					'timeout' => 3,
+					'connect_timeout' => 3,
+				]
+			)->will($this->returnValue($response));
+		$response
+			->expects($this->once())
+			->method('getBody')
+			->will($this->returnValue('{"installed":true,"maintenance":false,"version":"8.1.0.8","versionstring":"8.1.0","edition":""}'));
+
+		$this->clientService
+			->expects($this->once())
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->assertEquals(new DataResponse('https'), $this->getExternalShareController()->testRemote('owncloud.org'));
+	}
+
+	public function testRemoteWithWorkingHttp() {
+		$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
+			->disableOriginalConstructor()->getMock();
+		$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client
+			->method('get')
+			->will($this->onConsecutiveCalls($response, $response));
+		$response
+			->expects($this->exactly(2))
+			->method('getBody')
+			->will($this->onConsecutiveCalls('Certainly not a JSON string', '{"installed":true,"maintenance":false,"version":"8.1.0.8","versionstring":"8.1.0","edition":""}'));
+		$this->clientService
+			->expects($this->exactly(2))
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->assertEquals(new DataResponse('http'), $this->getExternalShareController()->testRemote('owncloud.org'));
+	}
+
+	public function testRemoteWithInvalidRemote() {
+		$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
+			->disableOriginalConstructor()->getMock();
+		$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
+			->disableOriginalConstructor()->getMock();
+		$client
+			->method('get')
+			->will($this->onConsecutiveCalls($response, $response));
+		$response
+			->expects($this->exactly(2))
+			->method('getBody')
+			->will($this->returnValue('Certainly not a JSON string'));
+		$this->clientService
+			->expects($this->exactly(2))
+			->method('newClient')
+			->will($this->returnValue($client));
+
+		$this->assertEquals(new DataResponse(false), $this->getExternalShareController()->testRemote('owncloud.org'));
+	}
+}

lib/private/http/client/client.php

@@ -144,6 +144,7 @@ class Client implements IClient {
 	 *              'debug' => true,
 	 *              'timeout' => 5,
 	 * @return Response
+	 * @throws \Exception If the request could not get completed
 	 */
 	public function head($uri, $options = []) {
 		$response = $this->client->head($uri, $options);
@@ -176,6 +177,7 @@ class Client implements IClient {
 	 *              'debug' => true,
 	 *              'timeout' => 5,
 	 * @return Response
+	 * @throws \Exception If the request could not get completed
 	 */
 	public function post($uri, array $options = []) {
 		$response = $this->client->post($uri, $options);
@@ -208,6 +210,7 @@ class Client implements IClient {
 	 *              'debug' => true,
 	 *              'timeout' => 5,
 	 * @return Response
+	 * @throws \Exception If the request could not get completed
 	 */
 	public function put($uri, array $options = []) {
 		$response = $this->client->put($uri, $options);
@@ -240,6 +243,7 @@ class Client implements IClient {
 	 *              'debug' => true,
 	 *              'timeout' => 5,
 	 * @return Response
+	 * @throws \Exception If the request could not get completed
 	 */
 	public function delete($uri, array $options = []) {
 		$response = $this->client->delete($uri, $options);
@@ -273,6 +277,7 @@ class Client implements IClient {
 	 *              'debug' => true,
 	 *              'timeout' => 5,
 	 * @return Response
+	 * @throws \Exception If the request could not get completed
 	 */
 	public function options($uri, array $options = []) {
 		$response = $this->client->options($uri, $options);

lib/public/http/client/iclient.php

@@ -80,6 +80,7 @@ interface IClient {
 	 *              'verify' => true, // bool or string to CA file
 	 *              'debug' => true,
 	 * @return IResponse
+	 * @throws \Exception If the request could not get completed
 	 * @since 8.1.0
 	 */
 	public function head($uri, $options = []);
@@ -109,6 +110,7 @@ interface IClient {
 	 *              'verify' => true, // bool or string to CA file
 	 *              'debug' => true,
 	 * @return IResponse
+	 * @throws \Exception If the request could not get completed
 	 * @since 8.1.0
 	 */
 	public function post($uri, array $options = []);
@@ -138,6 +140,7 @@ interface IClient {
 	 *              'verify' => true, // bool or string to CA file
 	 *              'debug' => true,
 	 * @return IResponse
+	 * @throws \Exception If the request could not get completed
 	 * @since 8.1.0
 	 */
 	public function put($uri, array $options = []);
@@ -167,6 +170,7 @@ interface IClient {
 	 *              'verify' => true, // bool or string to CA file
 	 *              'debug' => true,
 	 * @return IResponse
+	 * @throws \Exception If the request could not get completed
 	 * @since 8.1.0
 	 */
 	public function delete($uri, array $options = []);
@@ -196,6 +200,7 @@ interface IClient {
 	 *              'verify' => true, // bool or string to CA file
 	 *              'debug' => true,
 	 * @return IResponse
+	 * @throws \Exception If the request could not get completed
 	 * @since 8.1.0
 	 */
 	public function options($uri, array $options = []);