diff --git a/apps/files_sharing/lib/external/manager.php b/apps/files_sharing/lib/external/manager.php index 7b8b756dd5..0246e2d9c8 100644 --- a/apps/files_sharing/lib/external/manager.php +++ b/apps/files_sharing/lib/external/manager.php @@ -174,7 +174,7 @@ class Manager { */ private function getShare($id) { $getShare = $this->connection->prepare(' - SELECT `remote`, `share_token`, `name` + SELECT `remote`, `remote_id`, `share_token`, `name` FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?'); $result = $getShare->execute(array($id, $this->uid)); @@ -203,7 +203,7 @@ class Manager { `mountpoint_hash` = ? WHERE `id` = ? AND `user` = ?'); $acceptShare->execute(array(1, $mountPoint, $hash, $id, $this->uid)); - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $id, 'accept'); + $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'accept'); } } @@ -220,7 +220,7 @@ class Manager { $removeShare = $this->connection->prepare(' DELETE FROM `*PREFIX*share_external` WHERE `id` = ? AND `user` = ?'); $removeShare->execute(array($id, $this->uid)); - $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $id, 'decline'); + $this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline'); } } @@ -229,13 +229,13 @@ class Manager { * * @param string $remote * @param string $token - * @param int $id + * @param int $remoteId Share id on the remote host * @param string $feedback * @return boolean */ - private function sendFeedbackToRemote($remote, $token, $id, $feedback) { + private function sendFeedbackToRemote($remote, $token, $remoteId, $feedback) { - $url = $remote . \OCP\Share::BASE_PATH_TO_SHARE_API . '/' . $id . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT; + $url = rtrim($remote, '/') . \OCP\Share::BASE_PATH_TO_SHARE_API . '/' . $remoteId . '/' . $feedback . '?format=' . \OCP\Share::RESPONSE_FORMAT; $fields = array('token' => $token); $result = $this->httpHelper->post($url, $fields); diff --git a/apps/files_sharing/tests/external/managertest.php b/apps/files_sharing/tests/external/managertest.php index 3fd1f5444b..f7b216530d 100644 --- a/apps/files_sharing/tests/external/managertest.php +++ b/apps/files_sharing/tests/external/managertest.php @@ -22,16 +22,20 @@ namespace OCA\Files_Sharing\Tests\External; use OC\Files\Storage\StorageFactory; +use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\Tests\TestCase; class ManagerTest extends TestCase { - /** @var \OCA\Files_Sharing\External\Manager **/ + /** @var Manager **/ private $manager; /** @var \OC\Files\Mount\Manager */ private $mountManager; + /** @var \PHPUnit_Framework_MockObject_MockObject */ + private $httpHelper; + private $uid; protected function setUp() { @@ -39,16 +43,19 @@ class ManagerTest extends TestCase { $this->uid = $this->getUniqueID('user'); $this->mountManager = new \OC\Files\Mount\Manager(); - $this->manager = new \OCA\Files_Sharing\External\Manager( + $this->httpHelper = $httpHelper = $this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(); + /** @var \OC\HTTPHelper $httpHelper */ + $this->manager = new Manager( \OC::$server->getDatabaseConnection(), $this->mountManager, new StorageFactory(), - $this->getMockBuilder('\OC\HTTPHelper')->disableOriginalConstructor()->getMock(), + $httpHelper, $this->uid ); } public function testAddShare() { + $shareData1 = [ 'remote' => 'http://localhost', 'token' => 'token1', @@ -86,6 +93,10 @@ class ManagerTest extends TestCase { $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->httpHelper->expects($this->at(0)) + ->method('post') + ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id']), $this->anything()); + // Accept the first share $this->manager->acceptShare($openShares[0]['id']); @@ -117,6 +128,10 @@ class ManagerTest extends TestCase { $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->httpHelper->expects($this->at(0)) + ->method('post') + ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[1]['remote_id'] . '/decline'), $this->anything()); + // Decline the third share $this->manager->declineShare($openShares[1]['id']); @@ -140,6 +155,13 @@ class ManagerTest extends TestCase { $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1'); + $this->httpHelper->expects($this->at(0)) + ->method('post') + ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $openShares[0]['remote_id'] . '/decline'), $this->anything()); + $this->httpHelper->expects($this->at(1)) + ->method('post') + ->with($this->stringStartsWith('http://localhost/ocs/v1.php/cloud/shares/' . $acceptedShares[0]['remote_id'] . '/decline'), $this->anything()); + $this->manager->removeUserShares($this->uid); $this->assertEmpty(\Test_Helper::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted'); diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 0991fa0f38..38f763bfe8 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -2395,15 +2395,17 @@ class Share extends Constants { * * @param string $url * @param array $fields post parameters - * @return bool + * @return array */ private static function tryHttpPost($url, $fields) { $protocol = 'https://'; - $success = false; + $result = [ + 'success' => false, + 'result' => '', + ]; $try = 0; - while ($success === false && $try < 2) { + while ($result['success'] === false && $try < 2) { $result = \OC::$server->getHTTPHelper()->post($protocol . $url, $fields); - $success = $result['success']; $try++; $protocol = 'http://'; } @@ -2426,7 +2428,7 @@ class Share extends Constants { list($user, $remote) = explode('@', $shareWith, 2); if ($user && $remote) { - $url = $remote . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT; + $url = rtrim($remote, '/') . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT; $local = \OC::$server->getURLGenerator()->getAbsoluteURL('/'); @@ -2459,8 +2461,9 @@ class Share extends Constants { * @return bool */ private static function sendRemoteUnshare($remote, $id, $token) { - $url = $remote . self::BASE_PATH_TO_SHARE_API . '/' . $id . '/unshare?format=' . self::RESPONSE_FORMAT; + $url = rtrim($remote, '/') . self::BASE_PATH_TO_SHARE_API . '/' . $id . '/unshare?format=' . self::RESPONSE_FORMAT; $fields = array('token' => $token, 'format' => 'json'); + $url = self::removeProtocolFromUrl($url); $result = self::tryHttpPost($url, $fields); $status = json_decode($result['result'], true); diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index cda895a437..abdddfb558 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -105,6 +105,12 @@ class Test_Share extends \Test\TestCase { parent::tearDown(); } + protected function setHttpHelper($httpHelper) { + \OC::$server->registerService('HTTPHelper', function () use ($httpHelper) { + return $httpHelper; + }); + } + public function testShareInvalidShareType() { $message = 'Share type foobar is not valid for test.txt'; try { @@ -1024,10 +1030,58 @@ class Test_Share 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) { + $oldHttpHelper = \OC::$server->query('HTTPHelper'); + $httpHelperMock = $this->getMockBuilder('OC\HttpHelper') + ->disableOriginalConstructor() + ->getMock(); + $this->setHttpHelper($httpHelperMock); + + $httpHelperMock->expects($this->at(0)) + ->method('post') + ->with($this->stringStartsWith('https://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything()) + ->willReturn(['success' => false, 'result' => 'Exception']); + $httpHelperMock->expects($this->at(1)) + ->method('post') + ->with($this->stringStartsWith('http://' . $urlHost . '/ocs/v1.php/cloud/shares'), $this->anything()) + ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); + + \OCP\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/v1.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/v1.php/cloud/shares/' . $share['id'] . '/unshare'), $this->anything()) + ->willReturn(['success' => true, 'result' => json_encode(['ocs' => ['meta' => ['statuscode' => 100]]])]); + + \OCP\Share::unshare('test', 'test.txt', \OCP\Share::SHARE_TYPE_REMOTE, $shareWith); + $this->setHttpHelper($oldHttpHelper); + } + /** * @dataProvider dataProviderTestGroupItems - * @param type $ungrouped - * @param type $grouped + * @param array $ungrouped + * @param array $grouped */ function testGroupItems($ungrouped, $grouped) {