From 7785c3752fbfef792cd33dc5da2ee63e8263b9fa Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 13 Mar 2018 16:30:41 +0100 Subject: [PATCH] Remove deprecated HTTPHelper * Remove the HTTP Helper * Remove from Server Containter * Removed legacy share tests that use it Signed-off-by: Roeland Jago Douma --- .../RequestHandlerControllerTest.php | 32 ----- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/HTTPHelper.php | 120 ----------------- lib/private/Server.php | 17 --- lib/private/Share/Share.php | 17 ++- lib/public/IServerContainer.php | 8 -- tests/lib/HTTPHelperTest.php | 123 ------------------ tests/lib/ServerTest.php | 1 - tests/lib/Share/ShareTest.php | 82 ------------ 10 files changed, 16 insertions(+), 386 deletions(-) delete mode 100644 lib/private/HTTPHelper.php delete mode 100644 tests/lib/HTTPHelperTest.php diff --git a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php index d04c75b9a8..035b7da15c 100644 --- a/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php +++ b/apps/federatedfilesharing/tests/Controller/RequestHandlerControllerTest.php @@ -85,13 +85,6 @@ class RequestHandlerControllerTest extends TestCase { self::loginHelper(self::TEST_FILES_SHARING_API_USER1); \OC\Share\Share::registerBackend('test', 'Test\Share\Backend'); - $config = $this->getMockBuilder(IConfig::class) - ->disableOriginalConstructor()->getMock(); - $clientService = $this->getMockBuilder(IClientService::class)->getMock(); - $httpHelperMock = $this->getMockBuilder('\OC\HTTPHelper') - ->setConstructorArgs([$config, $clientService]) - ->getMock(); - $httpHelperMock->expects($this->any())->method('post')->with($this->anything())->will($this->returnValue(true)); $this->share = $this->getMockBuilder(IShare::class)->getMock(); $this->federatedShareProvider = $this->getMockBuilder('OCA\FederatedFileSharing\FederatedShareProvider') ->disableOriginalConstructor()->getMock(); @@ -110,8 +103,6 @@ class RequestHandlerControllerTest extends TestCase { $this->cloudIdManager = new CloudIdManager(); - $this->registerHttpHelper($httpHelperMock); - $this->s2s = new RequestHandlerController( 'federatedfilesharing', \OC::$server->getRequest(), @@ -134,32 +125,9 @@ class RequestHandlerControllerTest extends TestCase { $query = \OCP\DB::prepare('DELETE FROM `*PREFIX*share`'); $query->execute(); - $this->restoreHttpHelper(); - parent::tearDown(); } - /** - * Register an http helper mock for testing purposes. - * @param \OC\HTTPHelper $httpHelper helper mock - */ - private function registerHttpHelper($httpHelper) { - $this->oldHttpHelper = \OC::$server->query('HTTPHelper'); - \OC::$server->registerService('HTTPHelper', function ($c) use ($httpHelper) { - return $httpHelper; - }); - } - - /** - * Restore the original http helper - */ - private function restoreHttpHelper() { - $oldHttpHelper = $this->oldHttpHelper; - \OC::$server->registerService('HTTPHelper', function ($c) use ($oldHttpHelper) { - return $oldHttpHelper; - }); - } - /** * @medium */ diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index fb699e35a6..49e0717948 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -696,7 +696,6 @@ return array( 'OC\\Group\\Group' => $baseDir . '/lib/private/Group/Group.php', 'OC\\Group\\Manager' => $baseDir . '/lib/private/Group/Manager.php', 'OC\\Group\\MetaData' => $baseDir . '/lib/private/Group/MetaData.php', - 'OC\\HTTPHelper' => $baseDir . '/lib/private/HTTPHelper.php', 'OC\\HintException' => $baseDir . '/lib/private/HintException.php', 'OC\\Hooks\\BasicEmitter' => $baseDir . '/lib/private/Hooks/BasicEmitter.php', 'OC\\Hooks\\Emitter' => $baseDir . '/lib/private/Hooks/Emitter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 40b9c6237f..d876cf0e23 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -726,7 +726,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Group\\Group' => __DIR__ . '/../../..' . '/lib/private/Group/Group.php', 'OC\\Group\\Manager' => __DIR__ . '/../../..' . '/lib/private/Group/Manager.php', 'OC\\Group\\MetaData' => __DIR__ . '/../../..' . '/lib/private/Group/MetaData.php', - 'OC\\HTTPHelper' => __DIR__ . '/../../..' . '/lib/private/HTTPHelper.php', 'OC\\HintException' => __DIR__ . '/../../..' . '/lib/private/HintException.php', 'OC\\Hooks\\BasicEmitter' => __DIR__ . '/../../..' . '/lib/private/Hooks/BasicEmitter.php', 'OC\\Hooks\\Emitter' => __DIR__ . '/../../..' . '/lib/private/Hooks/Emitter.php', diff --git a/lib/private/HTTPHelper.php b/lib/private/HTTPHelper.php deleted file mode 100644 index d58f81fbbd..0000000000 --- a/lib/private/HTTPHelper.php +++ /dev/null @@ -1,120 +0,0 @@ - - * @author Lukas Reschke - * @author Morris Jobke - * - * @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 - * - */ - -namespace OC; - -use OCP\Http\Client\IClientService; -use OCP\IConfig; - -/** - * Class HTTPHelper - * - * @package OC - * @deprecated Use \OCP\Http\Client\IClientService - */ -class HTTPHelper { - const USER_AGENT = 'ownCloud Server Crawler'; - - /** @var \OCP\IConfig */ - private $config; - /** @var IClientService */ - private $clientService; - - /** - * @param IConfig $config - * @param IClientService $clientService - */ - public function __construct(IConfig $config, - IClientService $clientService) { - $this->config = $config; - $this->clientService = $clientService; - } - - /** - * Get URL content - * @param string $url Url to get content - * @throws \Exception If the URL does not start with http:// or https:// - * @return string of the response or false on error - * This function get the content of a page via curl, if curl is enabled. - * If not, file_get_contents is used. - * @deprecated Use \OCP\Http\Client\IClientService - */ - public function getUrlContent($url) { - try { - $client = $this->clientService->newClient(); - $response = $client->get($url); - return $response->getBody(); - } catch (\Exception $e) { - return false; - } - } - - /** - * Returns the response headers of a HTTP URL without following redirects - * @param string $location Needs to be a HTTPS or HTTP URL - * @return array - * @deprecated Use \OCP\Http\Client\IClientService - */ - public function getHeaders($location) { - $client = $this->clientService->newClient(); - $response = $client->get($location); - return $response->getHeaders(); - } - - /** - * Checks whether the supplied URL begins with HTTPS:// or HTTP:// (case insensitive) - * @param string $url - * @return bool - */ - public function isHTTPURL($url) { - return stripos($url, 'https://') === 0 || stripos($url, 'http://') === 0; - } - - /** - * send http post request - * - * @param string $url - * @param array $fields data send by the request - * @return array - * @deprecated Use \OCP\Http\Client\IClientService - */ - public function post($url, array $fields) { - $client = $this->clientService->newClient(); - - try { - $response = $client->post( - $url, - [ - 'body' => $fields, - 'connect_timeout' => 10, - ] - ); - } catch (\Exception $e) { - return ['success' => false, 'result' => $e->getMessage()]; - } - - return ['success' => true, 'result' => $response->getBody()]; - } - -} diff --git a/lib/private/Server.php b/lib/private/Server.php index af739c91b0..09807b185d 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -630,13 +630,6 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerAlias('DatabaseConnection', IDBConnection::class); - $this->registerService('HTTPHelper', function (Server $c) { - $config = $c->getConfig(); - return new HTTPHelper( - $config, - $c->getHTTPClientService() - ); - }); $this->registerService(\OCP\Http\Client\IClientService::class, function (Server $c) { $user = \OC_User::getUser(); @@ -1582,16 +1575,6 @@ class Server extends ServerContainer implements IServerContainer { return $this->query('CredentialsManager'); } - /** - * Returns an instance of the HTTP helper class - * - * @deprecated Use getHTTPClientService() - * @return \OC\HTTPHelper - */ - public function getHTTPHelper() { - return $this->query('HTTPHelper'); - } - /** * Get the certificate manager for the user * diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index e6056679c1..4514cdbae1 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -1991,7 +1991,22 @@ class Share extends Constants { while ($result['success'] === false && $try < 2) { $federationEndpoints = $discoveryService->discover($protocol . $remoteDomain, 'FEDERATED_SHARING'); $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; - $result = \OC::$server->getHTTPHelper()->post($protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, $fields); + $client = \OC::$server->getHTTPClientService()->newClient(); + + try { + $response = $client->post( + $protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, + [ + 'body' => $fields, + 'connect_timeout' => 10, + ] + ); + + $result = ['success' => true, 'result' => $response->getBody()]; + } catch (\Exception $e) { + $result = ['success' => false, 'result' => $e->getMessage()]; + } + $try++; $protocol = 'http://'; } diff --git a/lib/public/IServerContainer.php b/lib/public/IServerContainer.php index a80fc262ee..19c9578ee2 100644 --- a/lib/public/IServerContainer.php +++ b/lib/public/IServerContainer.php @@ -347,14 +347,6 @@ interface IServerContainer extends IContainer { */ public function createEventSource(); - /** - * Returns an instance of the HTTP helper class - * @return \OC\HTTPHelper - * @deprecated 8.1.0 Use \OCP\Http\Client\IClientService - * @since 8.0.0 - */ - public function getHTTPHelper(); - /** * Returns an instance of the HTTP client service * diff --git a/tests/lib/HTTPHelperTest.php b/tests/lib/HTTPHelperTest.php deleted file mode 100644 index d39cc34c46..0000000000 --- a/tests/lib/HTTPHelperTest.php +++ /dev/null @@ -1,123 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace Test; - -use OCP\Http\Client\IClientService; -use OCP\IConfig; - -class HTTPHelperTest extends \Test\TestCase { - - /** @var \OCP\IConfig*/ - private $config; - /** @var \OC\HTTPHelper */ - private $httpHelperMock; - /** @var \OCP\Http\Client\IClientService */ - private $clientService; - - protected function setUp() { - parent::setUp(); - - $this->config = $this->getMockBuilder(IConfig::class) - ->disableOriginalConstructor()->getMock(); - $this->clientService = $this->createMock(IClientService::class); - $this->httpHelperMock = $this->getMockBuilder('\OC\HTTPHelper') - ->setConstructorArgs(array($this->config, $this->clientService)) - ->setMethods(array('getHeaders')) - ->getMock(); - } - - public function isHttpTestData() { - return array( - array('http://wwww.owncloud.org/enterprise/', true), - array('https://wwww.owncloud.org/enterprise/', true), - array('HTTPS://WWW.OWNCLOUD.ORG', true), - array('HTTP://WWW.OWNCLOUD.ORG', true), - array('FILE://WWW.OWNCLOUD.ORG', false), - array('file://www.owncloud.org', false), - array('FTP://WWW.OWNCLOUD.ORG', false), - array('ftp://www.owncloud.org', false), - ); - } - - /** - * @dataProvider isHttpTestData - */ - public function testIsHTTP($url, $expected) { - $this->assertSame($expected, $this->httpHelperMock->isHTTPURL($url)); - } - - public function testPostSuccess() { - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $this->clientService - ->expects($this->once()) - ->method('newClient') - ->will($this->returnValue($client)); - $response = $this->getMockBuilder('\OCP\Http\Client\IResponse') - ->disableOriginalConstructor()->getMock(); - $client - ->expects($this->once()) - ->method('post') - ->with( - 'https://owncloud.org', - [ - 'body' => [ - 'Foo' => 'Bar', - ], - 'connect_timeout' => 10, - - ] - ) - ->will($this->returnValue($response)); - $response - ->expects($this->once()) - ->method('getBody') - ->will($this->returnValue('Body of the requested page')); - - - $response = $this->httpHelperMock->post('https://owncloud.org', ['Foo' => 'Bar']); - $expected = [ - 'success' => true, - 'result' => 'Body of the requested page' - ]; - $this->assertSame($expected, $response); - } - - public function testPostException() { - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $this->clientService - ->expects($this->once()) - ->method('newClient') - ->will($this->returnValue($client)); - $client - ->expects($this->once()) - ->method('post') - ->with( - 'https://owncloud.org', - [ - 'body' => [ - 'Foo' => 'Bar', - ], - 'connect_timeout' => 10, - - ] - ) - ->will($this->throwException(new \Exception('Something failed'))); - - - $response = $this->httpHelperMock->post('https://owncloud.org', ['Foo' => 'Bar']); - $expected = [ - 'success' => false, - 'result' => 'Something failed' - ]; - $this->assertSame($expected, $response); - } - -} diff --git a/tests/lib/ServerTest.php b/tests/lib/ServerTest.php index 63642c997a..e76b2b96db 100644 --- a/tests/lib/ServerTest.php +++ b/tests/lib/ServerTest.php @@ -91,7 +91,6 @@ class ServerTest extends \Test\TestCase { ['Hasher', '\OC\Security\Hasher'], ['Hasher', '\OCP\Security\IHasher'], - ['HTTPHelper', '\OC\HTTPHelper'], ['HttpClientService', '\OC\Http\Client\ClientService'], ['HttpClientService', '\OCP\Http\Client\IClientService'], diff --git a/tests/lib/Share/ShareTest.php b/tests/lib/Share/ShareTest.php index 18d1944f19..273482e175 100644 --- a/tests/lib/Share/ShareTest.php +++ b/tests/lib/Share/ShareTest.php @@ -542,53 +542,6 @@ class ShareTest extends \Test\TestCase { ); } - public function dataRemoteShareUrlCalls() { - return [ - ['admin@localhost', 'localhost'], - ['admin@https://localhost', 'localhost'], - ['admin@http://localhost', 'localhost'], - ['admin@localhost/subFolder', 'localhost/subFolder'], - ]; - } - - /** - * @dataProvider dataRemoteShareUrlCalls - * - * @param string $shareWith - * @param string $urlHost - */ - public function testRemoteShareUrlCalls($shareWith, $urlHost) { - $httpHelperMock = $this->getMockBuilder('OC\HTTPHelper') - ->disableOriginalConstructor() - ->getMock(); - $this->overwriteService('HTTPHelper', $httpHelperMock); - - $httpHelperMock->expects($this->at(0)) - ->method('post') - ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v2.php/cloud/shares'), $this->anything()) - ->willReturn(['success' => false, 'result' => 'Exception']); - $httpHelperMock->expects($this->at(1)) - ->method('post') - ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v2.php/cloud/shares'), $this->anything()) - ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); - - \OC\Share\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith, \OCP\Constants::PERMISSION_READ); - $shares = \OCP\Share::getItemShared('test', 'test.txt'); - $share = array_shift($shares); - - $httpHelperMock->expects($this->at(0)) - ->method('post') - ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) - ->willReturn(['success' => false, 'result' => 'Exception']); - $httpHelperMock->expects($this->at(1)) - ->method('post') - ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) - ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); - - \OC\Share\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith); - $this->restoreService('HTTPHelper'); - } - /** * @dataProvider dataProviderTestGroupItems * @param array $ungrouped @@ -666,41 +619,6 @@ class ShareTest extends \Test\TestCase { ); } - /** - * Make sure that a user cannot have multiple identical shares to remote users - */ - public function testOnlyOneRemoteShare() { - $httpHelperMock = $this->getMockBuilder('OC\HTTPHelper') - ->disableOriginalConstructor() - ->getMock(); - $this->overwriteService('HTTPHelper', $httpHelperMock); - - $httpHelperMock->expects($this->at(0)) - ->method('post') - ->with($this->stringStartsWith('https://localhost/ocs/v2.php/cloud/shares'), $this->anything()) - ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); - - \OC\Share\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost', \OCP\Constants::PERMISSION_READ); - $shares = \OCP\Share::getItemShared('test', 'test.txt'); - $share = array_shift($shares); - - //Try share again - try { - \OC\Share\Share::shareItem('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost', \OCP\Constants::PERMISSION_READ); - $this->fail('Identical remote shares are not allowed'); - } catch (\Exception $e) { - $this->assertEquals('Sharing test.txt failed, because this item is already shared with foo@localhost', $e->getMessage()); - } - - $httpHelperMock->expects($this->at(0)) - ->method('post') - ->with($this->stringStartsWith('https://localhost/ocs/v2.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) - ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); - - \OC\Share\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, 'foo@localhost'); - $this->restoreService('HTTPHelper'); - } - /** * Test case for #17560 */