From 441c289ffdc960f6b2b6c7d6ae00e24cab60191c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 12 Oct 2017 12:00:38 +0200 Subject: [PATCH 1/6] Catch errors when setting quota in integration tests --- tests/integration/features/bootstrap/Provisioning.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/features/bootstrap/Provisioning.php b/tests/integration/features/bootstrap/Provisioning.php index d9d6d8a91c5e..392f7cfebe30 100644 --- a/tests/integration/features/bootstrap/Provisioning.php +++ b/tests/integration/features/bootstrap/Provisioning.php @@ -640,6 +640,7 @@ public function userHasAQuotaOf($user, $quota) // method used from BasicStructure trait $this->sendingToWith("PUT", "/cloud/users/" . $user, $body); + PHPUnit_Framework_Assert::assertEquals(200, $this->response->getStatusCode()); } /** From 2e0623364f6949d3826fd317c016674aaa810b52 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 12 Oct 2017 13:02:29 +0200 Subject: [PATCH 2/6] Fix quota handling with federated shares Properly detect SPACE_UNLIMITED in quota check. Improve error handling when receiving errors like 507 when uploading files to a remote server. --- apps/dav/lib/Connector/Sabre/QuotaPlugin.php | 2 +- lib/private/Files/Storage/DAV.php | 58 +++++++++++++------ .../federation_features/federated.feature | 22 ++++++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php index 911b02221c90..e775852903df 100644 --- a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php +++ b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php @@ -163,7 +163,7 @@ public function checkQuota($path, $length = null) { $path = rtrim($parentPath, '/') . '/' . $info['name']; } $freeSpace = $this->getFreeSpace($path); - if ($freeSpace !== FileInfo::SPACE_UNKNOWN && $length > $freeSpace) { + if ($freeSpace !== FileInfo::SPACE_UNKNOWN && $freeSpace !== FileInfo::SPACE_UNLIMITED && $length > $freeSpace) { if (isset($chunkHandler)) { $chunkHandler->cleanup(); } diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 4fe50aff88e7..419c5f63ce8a 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -50,6 +50,8 @@ use Sabre\DAV\Xml\Property\ResourceType; use Sabre\HTTP\ClientException; use Sabre\HTTP\ClientHttpException; +use Sabre\DAV\Exception\InsufficientStorage; +use OCA\DAV\Connector\Sabre\Exception\Forbidden; /** * Class DAV @@ -355,7 +357,7 @@ public function fopen($path, $mode) { && $e->getResponse()->getStatusCode() === Http::STATUS_NOT_FOUND) { return false; } else { - throw $e; + $this->convertException($e); } } @@ -503,14 +505,17 @@ protected function uploadFile($path, $target) { $this->statCache->remove($target); $source = fopen($path, 'r'); - $this->httpClientService - ->newClient() - ->put($this->createBaseUri() . $this->encodePath($target), [ - 'body' => $source, - 'auth' => [$this->user, $this->password] - ]); - $this->removeCachedFile($target); + try { + $this->httpClientService + ->newClient() + ->put($this->createBaseUri() . $this->encodePath($target), [ + 'body' => $source, + 'auth' => [$this->user, $this->password] + ]); + } catch (\Exception $e) { + $this->convertException($e); + } } /** {@inheritdoc} */ @@ -828,18 +833,15 @@ private function convertException(Exception $e, $path = '') { \OC::$server->getLogger()->logException($e); Util::writeLog('files_external', $e->getMessage(), Util::ERROR); if ($e instanceof ClientHttpException) { - if ($e->getHttpStatus() === Http::STATUS_LOCKED) { - throw new \OCP\Lock\LockedException($path); - } - if ($e->getHttpStatus() === Http::STATUS_UNAUTHORIZED) { - // either password was changed or was invalid all along - throw new StorageInvalidException(get_class($e) . ': ' . $e->getMessage()); - } else if ($e->getHttpStatus() === Http::STATUS_METHOD_NOT_ALLOWED) { + if ($e->getHttpStatus() === Http::STATUS_METHOD_NOT_ALLOWED) { // ignore exception for MethodNotAllowed, false will be returned return; } - throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); - } else if ($e instanceof ClientException) { + $this->throwByStatusCode($e->getHttpStatus(), $path); + } else if ($e instanceof \GuzzleHttp\Exception\ClientException || $e instanceof \GuzzleHttp\Exception\ServerException) { + if ($e->getResponse() instanceof ResponseInterface) { + $this->throwByStatusCode($e->getResponse()->getStatusCode()); + } // connection timeout or refused, server could be temporarily down throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); } else if ($e instanceof \InvalidArgumentException) { @@ -853,5 +855,27 @@ private function convertException(Exception $e, $path = '') { // TODO: only log for now, but in the future need to wrap/rethrow exception } + + /** + * Throw exception by status code + * + * @param int $statusCode status code + * @param string $path optional path for some exceptions + * @throws \Exception Sabre or ownCloud exceptions + */ + private function throwByStatusCode($statusCode, $path = '') { + switch ($statusCode) { + case Http::STATUS_LOCKED: + throw new \OCP\Lock\LockedException($path); + case Http::STATUS_UNAUTHORIZED: + // either password was changed or was invalid all along + throw new StorageInvalidException(get_class($e) . ': ' . $e->getMessage()); + case Http::STATUS_INSUFFICIENT_STORAGE: + throw new InsufficientStorage(); + case Http::STATUS_FORBIDDEN: + throw new Forbidden(); + } + throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); + } } diff --git a/tests/integration/federation_features/federated.feature b/tests/integration/federation_features/federated.feature index ea0fba3dcee1..ef9e2044fd62 100644 --- a/tests/integration/federation_features/federated.feature +++ b/tests/integration/federation_features/federated.feature @@ -222,16 +222,32 @@ Feature: federated And as an "user0" And etag of element "/PARENT" of user "user0" has changed - @local_storage Scenario: Upload file to received federated share while quota is set on home storage Given using server "REMOTE" + And as an "admin" And user "user1" exists + And user "user1" has a quota of "20 B" And using server "LOCAL" And user "user0" exists And user "user0" from server "LOCAL" shares "/PARENT" with user "user1" from server "REMOTE" And user "user1" from server "REMOTE" accepts last pending share And using server "REMOTE" - When user "user1" uploads file "data/textfile.txt" to "/PARENT/testquota.txt" with all mechanisms + When user "user1" uploads file "data/textfile.txt" to "/PARENT (2)/testquota.txt" with all mechanisms Then the HTTP status code of all upload responses should be "201" - And as "user0" the file "/PARENT/textquota.txt" exists + And using server "LOCAL" + And as "user0" the file "/PARENT (2)/textquota.txt" exists + + Scenario: Upload file to received federated share while quota is set on remote storage + Given using server "REMOTE" + And as an "admin" + And user "user1" exists + And using server "LOCAL" + And as an "admin" + And user "user0" exists + And user "user0" has a quota of "20 B" + And user "user0" from server "LOCAL" shares "/PARENT" with user "user1" from server "REMOTE" + And user "user1" from server "REMOTE" accepts last pending share + And using server "REMOTE" + When user "user1" uploads file "data/textfile.txt" to "/PARENT (2)/testquota.txt" with all mechanisms + Then the HTTP status code of all upload responses should be "507" From c0681c420b7de1322e2832e7dc47692d0c10c23c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 16 Oct 2017 14:31:43 +0200 Subject: [PATCH 3/6] Fix integration tests to properly set quota in some cases --- tests/integration/features/external-storage.feature | 1 + tests/integration/features/sharing-v1.feature | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/integration/features/external-storage.feature b/tests/integration/features/external-storage.feature index ad378a21da0d..631804dabc26 100644 --- a/tests/integration/features/external-storage.feature +++ b/tests/integration/features/external-storage.feature @@ -62,6 +62,7 @@ Feature: external-storage @local_storage Scenario: Upload a file to external storage while quota is set on home storage Given user "user0" exists + And as an "admin" And user "user0" has a quota of "1 B" And as an "user0" When user "user0" uploads file "data/textfile.txt" to "/local_storage/testquota.txt" with all mechanisms diff --git a/tests/integration/features/sharing-v1.feature b/tests/integration/features/sharing-v1.feature index 9b571315c4ed..d8eb3a600555 100644 --- a/tests/integration/features/sharing-v1.feature +++ b/tests/integration/features/sharing-v1.feature @@ -598,6 +598,7 @@ Feature: sharing Given using old dav path And user "user0" exists And user "user1" exists + And as an "admin" And user "user1" has a quota of "0" And user "user0" moved file "/welcome.txt" to "/myfile.txt" And file "myfile.txt" of user "user0" is shared with user "user1" From dcf11b7c6f692937230ef3bf06b5046e7f299d54 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 17 Oct 2017 10:52:03 +0200 Subject: [PATCH 4/6] Update QuotaPluginTest for unlimited quota values --- .../tests/unit/Connector/Sabre/QuotaPluginTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php index b9aabce2005b..d8b5b88b3e26 100644 --- a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php @@ -116,6 +116,11 @@ public function quotaOkayProvider() { [-2, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], [-2, ['CONTENT-LENGTH' => '512']], [-2, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], + // \OCP\Files\FileInfo::SPACE-UNLIMITED = -3 + [-3, []], + [-3, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], + [-3, ['CONTENT-LENGTH' => '512']], + [-3, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], ]; } @@ -159,6 +164,13 @@ public function quotaChunkedOkProvider() { [-2, 128, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], [-2, 128, ['CONTENT-LENGTH' => '512']], [-2, 128, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], + // \OCP\Files\FileInfo::SPACE-UNLIMITED = -3 + [-3, 0, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], + [-3, 0, ['CONTENT-LENGTH' => '512']], + [-3, 0, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], + [-3, 128, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], + [-3, 128, ['CONTENT-LENGTH' => '512']], + [-3, 128, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], ]; } From 60050a2a3c7aeeff483da461a69d72fb2d57fcf9 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 19 Oct 2017 16:20:04 +0200 Subject: [PATCH 5/6] Added WebdavClientService in core + unit tests for Storage\DAV class Added tests for all FS methods of the Storage\DAV class and also fixed some related issues. --- lib/private/Files/Storage/DAV.php | 69 +- .../Http/Client/WebdavClientService.php | 97 ++ lib/private/Server.php | 29 + .../Http/Client/IWebdavClientService.php | 56 + tests/lib/Files/Storage/DavTest.php | 1319 +++++++++++++++++ .../Http/Client/WebdavClientServiceTest.php | 101 ++ 6 files changed, 1632 insertions(+), 39 deletions(-) create mode 100644 lib/private/Http/Client/WebdavClientService.php create mode 100644 lib/public/Http/Client/IWebdavClientService.php create mode 100644 tests/lib/Files/Storage/DavTest.php create mode 100644 tests/lib/Http/Client/WebdavClientServiceTest.php diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 419c5f63ce8a..0939b2941ef4 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -46,7 +46,6 @@ use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; use OCP\Util; -use Sabre\DAV\Client; use Sabre\DAV\Xml\Property\ResourceType; use Sabre\HTTP\ClientException; use Sabre\HTTP\ClientHttpException; @@ -84,6 +83,9 @@ class DAV extends Common { /** @var \OCP\Http\Client\IClientService */ private $httpClientService; + /** @var \OCP\Http\Client\IWebdavClientService */ + private $webdavClientService; + /** * @param array $params * @throws \Exception @@ -91,6 +93,7 @@ class DAV extends Common { public function __construct($params) { $this->statCache = new ArrayCache(); $this->httpClientService = \OC::$server->getHTTPClientService(); + $this->webdavClientService = \OC::$server->getWebdavClientService(); if (isset($params['host']) && isset($params['user']) && isset($params['password'])) { $host = $params['host']; //remove leading http[s], will be generated in createBaseUri() @@ -111,17 +114,6 @@ public function __construct($params) { } else { $this->secure = false; } - if ($this->secure === true) { - // inject mock for testing - $certManager = \OC::$server->getCertificateManager(); - if (is_null($certManager)) { //no user - $certManager = \OC::$server->getCertificateManager(null); - } - $certPath = $certManager->getAbsoluteBundlePath(); - if (file_exists($certPath)) { - $this->certPath = $certPath; - } - } $this->root = isset($params['root']) ? $params['root'] : '/'; if (!$this->root || $this->root[0] != '/') { $this->root = '/' . $this->root; @@ -130,7 +122,7 @@ public function __construct($params) { $this->root .= '/'; } } else { - throw new \Exception('Invalid webdav storage configuration'); + throw new \InvalidArgumentException('Invalid webdav storage configuration'); } } @@ -143,22 +135,13 @@ protected function init() { $settings = [ 'baseUri' => $this->createBaseUri(), 'userName' => $this->user, - 'password' => $this->password, + 'password' => $this->password ]; if (isset($this->authType)) { $settings['authType'] = $this->authType; } - $proxy = \OC::$server->getConfig()->getSystemValue('proxy', ''); - if($proxy !== '') { - $settings['proxy'] = $proxy; - } - - $this->client = new Client($settings); - $this->client->setThrowExceptions(true); - if ($this->secure === true && $this->certPath) { - $this->client->addCurlSetting(CURLOPT_CAINFO, $this->certPath); - } + $this->client = $this->webdavClientService->newClient($settings); } /** @@ -236,6 +219,13 @@ public function opendir($path) { $content[] = $file; } return IteratorDirectory::wrap($content); + } catch (ClientHttpException $e) { + if ($e->getHttpStatus() === Http::STATUS_NOT_FOUND) { + $this->statCache->clear($path . '/'); + $this->statCache->set($path, false); + return false; + } + $this->convertException($e, $path); } catch (\Exception $e) { $this->convertException($e, $path); } @@ -366,6 +356,7 @@ public function fopen($path, $mode) { throw new \OCP\Lock\LockedException($path); } else { Util::writeLog("webdav client", 'Guzzle get returned status code ' . $response->getStatusCode(), Util::ERROR); + // FIXME: why not returning false here ?! } } @@ -444,7 +435,7 @@ public function free_space($path) { public function touch($path, $mtime = null) { $this->init(); if (is_null($mtime)) { - $mtime = time(); + $mtime = \OC::$server->getTimeFactory()->getTime(); } $path = $this->cleanPath($path); @@ -456,6 +447,7 @@ public function touch($path, $mtime = null) { // non-owncloud clients might not have accepted the property, need to recheck it $response = $this->client->propfind($this->encodePath($path), ['{DAV:}getlastmodified'], 0); if ($response === false) { + // file disappeared since ? return false; } if (isset($response['{DAV:}getlastmodified'])) { @@ -666,6 +658,9 @@ private function simpleResponse($method, $path, $body, $expected) { $this->statCache->set($path, false); return false; } + if ($e->getHttpStatus() === Http::STATUS_METHOD_NOT_ALLOWED && $method === 'MKCOL') { + return false; + } $this->convertException($e, $path); } catch (\Exception $e) { @@ -780,10 +775,7 @@ public function hasUpdated($path, $time) { } if (isset($response['{DAV:}getetag'])) { $cachedData = $this->getCache()->get($path); - $etag = null; - if (isset($response['{DAV:}getetag'])) { - $etag = trim($response['{DAV:}getetag'], '"'); - } + $etag = trim($response['{DAV:}getetag'], '"'); if (!empty($etag) && $cachedData['etag'] !== $etag) { return true; } else if (isset($response['{http://open-collaboration-services.org/ns}share-permissions'])) { @@ -833,14 +825,10 @@ private function convertException(Exception $e, $path = '') { \OC::$server->getLogger()->logException($e); Util::writeLog('files_external', $e->getMessage(), Util::ERROR); if ($e instanceof ClientHttpException) { - if ($e->getHttpStatus() === Http::STATUS_METHOD_NOT_ALLOWED) { - // ignore exception for MethodNotAllowed, false will be returned - return; - } - $this->throwByStatusCode($e->getHttpStatus(), $path); - } else if ($e instanceof \GuzzleHttp\Exception\ClientException || $e instanceof \GuzzleHttp\Exception\ServerException) { + $this->throwByStatusCode($e->getHttpStatus(), $e, $path); + } else if ($e instanceof \GuzzleHttp\Exception\RequestException) { if ($e->getResponse() instanceof ResponseInterface) { - $this->throwByStatusCode($e->getResponse()->getStatusCode()); + $this->throwByStatusCode($e->getResponse()->getStatusCode(), $e); } // connection timeout or refused, server could be temporarily down throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); @@ -848,7 +836,10 @@ private function convertException(Exception $e, $path = '') { // parse error because the server returned HTML instead of XML, // possibly temporarily down throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); - } else if (($e instanceof StorageNotAvailableException) || ($e instanceof StorageInvalidException)) { + } else if (($e instanceof StorageNotAvailableException) + || ($e instanceof StorageInvalidException) + || ($e instanceof \Sabre\DAV\Exception + )) { // rethrow throw $e; } @@ -863,7 +854,7 @@ private function convertException(Exception $e, $path = '') { * @param string $path optional path for some exceptions * @throws \Exception Sabre or ownCloud exceptions */ - private function throwByStatusCode($statusCode, $path = '') { + private function throwByStatusCode($statusCode, $e, $path = '') { switch ($statusCode) { case Http::STATUS_LOCKED: throw new \OCP\Lock\LockedException($path); @@ -873,7 +864,7 @@ private function throwByStatusCode($statusCode, $path = '') { case Http::STATUS_INSUFFICIENT_STORAGE: throw new InsufficientStorage(); case Http::STATUS_FORBIDDEN: - throw new Forbidden(); + throw new Forbidden('Forbidden'); } throw new StorageNotAvailableException(get_class($e) . ': ' . $e->getMessage()); } diff --git a/lib/private/Http/Client/WebdavClientService.php b/lib/private/Http/Client/WebdavClientService.php new file mode 100644 index 000000000000..ff07909d9f0d --- /dev/null +++ b/lib/private/Http/Client/WebdavClientService.php @@ -0,0 +1,97 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @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\Http\Client; + +use OCP\Http\Client\IWebdavClientService; +use OCP\IConfig; +use OCP\ICertificateManager; +use Sabre\DAV\Client; + +/** + * Class WebdavClientService + * + * @package OC\Http + */ +class WebdavClientService implements IWebdavClientService { + /** @var IConfig */ + private $config; + /** @var ICertificateManager */ + private $certificateManager; + + /** + * @param IConfig $config + * @param ICertificateManager $certificateManager + */ + public function __construct(IConfig $config, + ICertificateManager $certificateManager) { + $this->config = $config; + $this->certificateManager = $certificateManager; + } + + /** + * Instantiate new Sabre client + * + * Settings are provided through the 'settings' argument. The following + * settings are supported: + * + * * baseUri + * * userName (optional) + * * password (optional) + * * proxy (optional) + * * authType (optional) + * * encoding (optional) + * + * authType must be a bitmap, using self::AUTH_BASIC, self::AUTH_DIGEST + * and self::AUTH_NTLM. If you know which authentication method will be + * used, it's recommended to set it, as it will save a great deal of + * requests to 'discover' this information. + * + * Encoding is a bitmap with one of the ENCODING constants. + * + * @param array $settings Sabre client settings + * @return Client + */ + public function newClient($settings) { + if (!isset($settings['proxy'])) { + $proxy = $this->config->getSystemValue('proxy', ''); + if($proxy !== '') { + $settings['proxy'] = $proxy; + } + } + + $certPath = null; + if (strpos($settings['baseUri'], 'https') === 0) { + $certPath = $this->certificateManager->getAbsoluteBundlePath(); + if (!file_exists($certPath)) { + $certPath = null; + } + } + + $client = new Client($settings); + $client->setThrowExceptions(true); + + if ($certPath !== null) { + $client->addCurlSetting(CURLOPT_CAINFO, $certPath); + } + return $client; + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index a20801cf889b..3717335f8b6f 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -100,6 +100,7 @@ use OC\Files\External\Service\UserGlobalStoragesService; use OC\Files\External\Service\GlobalStoragesService; use OC\Files\External\Service\DBConfigService; +use OC\Http\Client\WebDavClientService; use Symfony\Component\EventDispatcher\GenericEvent; /** @@ -275,6 +276,10 @@ public function __construct($webRoot, \OC\Config $config) { return new \OC\Authentication\Token\DefaultTokenProvider($mapper, $crypto, $config, $logger, $timeFactory); }); $this->registerAlias('OC\Authentication\Token\IProvider', 'OC\Authentication\Token\DefaultTokenProvider'); + $this->registerService('TimeFactory', function() { + return new TimeFactory(); + }); + $this->registerAlias('OCP\AppFramework\Utility\ITimeFactory', 'TimeFactory'); $this->registerService('UserSession', function (Server $c) { $manager = $c->getUserManager(); $session = new \OC\Session\Memory(''); @@ -496,6 +501,14 @@ public function __construct($webRoot, \OC\Config $config) { new \OC\Security\CertificateManager($uid, new View(), $c->getConfig()) ); }); + $this->registerService('WebdavClientService', function (Server $c) { + $user = \OC_User::getUser(); + $uid = $user ? $user : null; + return new WebdavClientService( + $c->getConfig(), + new \OC\Security\CertificateManager($uid, new View(), $c->getConfig()) + ); + }); $this->registerService('EventLogger', function (Server $c) { $eventLogger = new EventLogger(); if ($c->getSystemConfig()->getValue('debug', false)) { @@ -1269,6 +1282,15 @@ public function getHTTPClientService() { return $this->query('HttpClientService'); } + /** + * Returns an instance of the Webdav client service + * + * @return \OCP\Http\Client\IWebdavClientService + */ + public function getWebdavClientService() { + return $this->query('WebdavClientService'); + } + /** * Create a new event source * @@ -1522,4 +1544,11 @@ public function getShareManager() { public function getThemeService() { return $this->query('\OCP\Theme\IThemeService'); } + + /** + * @return ITimeFactory + */ + public function getTimeFactory() { + return $this->query('\OCP\AppFramework\Utility\ITimeFactory'); + } } diff --git a/lib/public/Http/Client/IWebdavClientService.php b/lib/public/Http/Client/IWebdavClientService.php new file mode 100644 index 000000000000..d6e8b48f47f3 --- /dev/null +++ b/lib/public/Http/Client/IWebdavClientService.php @@ -0,0 +1,56 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @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 OCP\Http\Client; + +use Sabre\HTTP\Client; + +/** + * Interface IWebdavClientService + * + * @package OCP\Http + * @since 10.0.4 + */ +interface IWebdavClientService { + /** + * Settings are provided through the 'settings' argument. The following + * settings are supported: + * + * * baseUri + * * userName (optional) + * * password (optional) + * * proxy (optional) + * * authType (optional) + * * encoding (optional) + * + * authType must be a bitmap, using self::AUTH_BASIC, self::AUTH_DIGEST + * and self::AUTH_NTLM. If you know which authentication method will be + * used, it's recommended to set it, as it will save a great deal of + * requests to 'discover' this information. + * + * Encoding is a bitmap with one of the ENCODING constants. + * + * @param $settings Sabre client settings + * @return Client + * @since 10.0.4 + */ + public function newClient($settings); +} diff --git a/tests/lib/Files/Storage/DavTest.php b/tests/lib/Files/Storage/DavTest.php new file mode 100644 index 000000000000..3ca57b9f3884 --- /dev/null +++ b/tests/lib/Files/Storage/DavTest.php @@ -0,0 +1,1319 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @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 Test\Files\Storage; + +use Test\TestCase; +use OCP\Http\Client\IClientService; +use OCP\Http\Client\IClient; +use Sabre\DAV\Client; +use OCP\Http\Client\IWebdavClientService; +use Sabre\HTTP\ClientHttpException; +use OCP\Lock\LockedException; +use OCP\AppFramework\Http; +use OCP\Files\StorageInvalidException; +use GuzzleHttp\Exception\ServerException; +use GuzzleHttp\Exception\ClientException; +use Sabre\DAV\Exception\InsufficientStorage; +use Sabre\DAV\Exception\Forbidden; +use OCP\Files\StorageNotAvailableException; +use OC\Files\Storage\DAV; +use OCP\Files\FileInfo; +use OCP\AppFramework\Utility\ITimeFactory; +use OC\Files\Cache\Cache; + +/** + * Class DavTest + * + * @group DB + * + * @package Test\Files\Storage + */ +class DavTest extends TestCase { + + /** + * @var DAV + */ + private $instance; + + /** + * @var IClientService + */ + private $httpClientService; + + /** + * @var IWebdavClientService + */ + private $webdavClientService; + + /** + * @var Client + */ + private $davClient; + + /** + * @var IClient + **/ + private $httpClient; + + /** + * @var ITimeFactory + */ + private $timeFactory; + + /** + * @var Cache + */ + private $cache; + + protected function setUp() { + parent::setUp(); + + $this->httpClientService = $this->createMock(IClientService::class); + $this->overwriteService('HttpClientService', $this->httpClientService); + + $this->webdavClientService = $this->createMock(IWebdavClientService::class); + $this->overwriteService('WebdavClientService', $this->webdavClientService); + + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->overwriteService('TimeFactory', $this->timeFactory); + + $this->httpClient = $this->createMock(IClient::class); + $this->httpClientService->method('newClient')->willReturn($this->httpClient); + + $this->davClient = $this->createMock(Client::class); + $this->webdavClientService->method('newClient')->willReturn($this->davClient); + + $this->instance = $this->getMockBuilder(\OC\Files\Storage\DAV::class) + ->setConstructorArgs([[ + 'user' => 'davuser', + 'password' => 'davpassword', + 'host' => 'davhost', + 'root' => 'davroot', + 'secure' => true + ]]) + ->setMethods(['getCache']) + ->getMock(); + + $this->cache = $this->createMock(Cache::class); + $this->instance->method('getCache')->willReturn($this->cache); + } + + protected function tearDown() { + $this->restoreService('HttpClientService'); + $this->restoreService('WebdavClientService'); + $this->restoreService('TimeFactory'); + parent::tearDown(); + } + + public function testId() { + $this->assertEquals('webdav::davuser@davhost//davroot/', $this->instance->getId()); + } + + public function instantiateWebdavClientDataProvider() { + return [ + [false, 'http'], + [true, 'https'], + ]; + } + + /** + * @dataProvider instantiateWebdavClientDataProvider + */ + public function testInstantiateWebdavClient($secure, $protocol) { + $this->restoreService('WebdavClientService'); + $this->webdavClientService = $this->createMock(IWebdavClientService::class); + $this->overwriteService('WebdavClientService', $this->webdavClientService); + $this->webdavClientService->expects($this->once()) + ->method('newClient') + ->with([ + 'baseUri' => $protocol . '://davhost/davroot/', + 'userName' => 'davuser', + 'password' => 'davpassword', + 'authType' => 'basic', + ]) + ->willReturn($this->davClient); + + $this->instance = new \OC\Files\Storage\DAV([ + 'user' => 'davuser', + 'password' => 'davpassword', + 'host' => 'davhost', + 'root' => 'davroot', + 'authType' => 'basic', + 'secure' => $secure + ]); + + // trigger lazy init + $this->instance->mkdir('/test'); + } + + public function invalidConfigDataProvider() { + return [ + [[ + 'user' => 'davuser', + 'password' => 'davpassword', + 'root' => 'davroot', + ], [ + 'user' => 'davuser', + 'host' => 'davhost', + 'root' => 'davroot', + ], [ + 'password' => 'davpassword', + 'host' => 'davhost', + 'root' => 'davroot', + ]], + ]; + } + + /** + * @dataProvider invalidConfigDataProvider + * @expectedException \InvalidArgumentException + */ + public function testInstantiateWebdavClientInvalidConfig($params) { + new \OC\Files\Storage\DAV($params); + } + + private function createClientHttpException($statusCode) { + $response = $this->createMock(\Sabre\HTTP\ResponseInterface::class); + $response->method('getStatusText')->willReturn(''); + $response->method('getStatus')->willReturn($statusCode); + return new ClientHttpException($response); + } + + private function createGuzzleClientException($statusCode) { + $request = $this->createMock(\GuzzleHttp\Message\RequestInterface::class); + $response = $this->createMock(\GuzzleHttp\Message\ResponseInterface::class); + $response->method('getStatusCode')->willReturn($statusCode); + return new ClientException('ClientException', $request, $response); + } + + private function createGuzzleServerException($statusCode) { + $request = $this->createMock(\GuzzleHttp\Message\RequestInterface::class); + $response = $this->createMock(\GuzzleHttp\Message\ResponseInterface::class); + $response->method('getStatusCode')->willReturn($statusCode); + return new ServerException('ServerException', $request, $response); + } + + public function convertExceptionDataProvider() { + $statusCases = [ + [Http::STATUS_UNAUTHORIZED, StorageInvalidException::class], + [Http::STATUS_LOCKED, LockedException::class], + [Http::STATUS_INSUFFICIENT_STORAGE, InsufficientStorage::class], + [Http::STATUS_FORBIDDEN, Forbidden::class], + [Http::STATUS_INTERNAL_SERVER_ERROR, StorageNotAvailableException::class], + ]; + + $testCases = [ + [new \Sabre\DAV\Exception\Forbidden('Forbidden'), \Sabre\DAV\Exception\Forbidden::class], + [new \InvalidArgumentException(), StorageNotAvailableException::class], + [new StorageNotAvailableException(), StorageNotAvailableException::class], + [new StorageInvalidException(), StorageInvalidException::class], + ]; + + // map to ClientHttpException + foreach ($statusCases as $statusCase) { + $testCases[] = [$this->createClientHttpException($statusCase[0]), $statusCase[1]]; + $testCases[] = [$this->createGuzzleClientException($statusCase[0]), $statusCase[1]]; + $testCases[] = [$this->createGuzzleServerException($statusCase[0]), $statusCase[1]]; + } + + // one case where Guzzle response is null, for whatever reason + $testCases[] = [ + new ServerException( + 'ServerException with no response', + $this->createMock(\GuzzleHttp\Message\RequestInterface::class), + null + ), + StorageNotAvailableException::class + ]; + + return $testCases; + } + + /** + * @dataProvider convertExceptionDataProvider + */ + public function testConvertException($inputException, $expectedExceptionClass) { + $this->davClient->method('propfind')->will($this->throwException($inputException)); + + $thrownException = null; + try { + $this->instance->opendir('/test'); + } catch (\Exception $e) { + $thrownException = $e; + } + + $this->assertNotNull($thrownException); + $this->assertInstanceOf($expectedExceptionClass, $thrownException); + } + + public function testMkdir() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('MKCOL', 'new%25dir', null) + ->willReturn(['statusCode' => Http::STATUS_CREATED]); + + $this->assertTrue($this->instance->mkdir('/new%dir')); + } + + public function testMkdirAlreadyExists() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('MKCOL', 'new%25dir', null) + ->willThrowException($this->createClientHttpException(Http::STATUS_METHOD_NOT_ALLOWED)); + + $this->assertFalse($this->instance->mkdir('/new%dir')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testMkdirException() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('MKCOL', 'new%25dir', null) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->mkdir('/new%dir'); + } + + public function testRmdir() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25dir', null) + ->willReturn(['statusCode' => Http::STATUS_NO_CONTENT]); + + $this->assertTrue($this->instance->rmdir('/old%dir')); + } + + public function testRmdirUnexist() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25dir', null) + ->willReturn(['statusCode' => Http::STATUS_NOT_FOUND]); + + $this->assertFalse($this->instance->rmdir('/old%dir')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testRmdirException() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25dir', null) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->rmdir('/old%dir'); + } + + public function testOpenDir() { + $responseBody = [ + // root entry + 'some%25dir' => [], + 'some%25dir/first%25folder' => [], + 'some%25dir/second' => [], + ]; + + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', [], 1) + ->willReturn($responseBody); + + // do operation twice, second time will not trigger propfind + // due to stat cache + $i = 0; + while ($i < 1) { + $i++; + + $dir = $this->instance->opendir('/some%dir'); + $entries = []; + while ($entry = readdir($dir)) { + $entries[] = $entry; + } + + $this->assertCount(2, $entries); + $this->assertEquals('first%folder', $entries[0]); + $this->assertEquals('second', $entries[1]); + } + } + + public function testOpenDirNotFound() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', [], 1) + ->willThrowException($this->createClientHttpException(Http::STATUS_NOT_FOUND)); + + $this->assertFalse($this->instance->opendir('/some%dir')); + } + + public function testOpenDirNotFound2() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', [], 1) + ->willReturn(false); + + $this->assertFalse($this->instance->opendir('/some%dir')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testOpenDirException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', [], 1) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->opendir('/some%dir'); + } + + private function getResourceTypeResponse($isDir) { + $resourceTypeObj = $this->getMockBuilder('\stdclass') + ->setMethods(['getValue']) + ->getMock(); + if ($isDir) { + $resourceTypeObj->method('getValue') + ->willReturn(['{DAV:}collection']); + } else { + $resourceTypeObj->method('getValue') + ->willReturn([]); + } + return $resourceTypeObj; + } + + public function testFileTypeDir() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->contains('{DAV:}resourcetype')) + ->willReturn([ + '{DAV:}resourcetype' => $this->getResourceTypeResponse(true) + ]); + + $this->assertEquals('dir', $this->instance->filetype('/some%dir/file%type')); + } + + public function testFileTypeFile() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->contains('{DAV:}resourcetype')) + ->willReturn([]); + + $this->assertEquals('file', $this->instance->filetype('/some%dir/file%type')); + } + + public function testFileTypeNotFound() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->contains('{DAV:}resourcetype')) + ->willThrowException($this->createClientHttpException(Http::STATUS_NOT_FOUND)); + + $this->assertFalse($this->instance->filetype('/some%dir/file%type')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testFileTypeException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->contains('{DAV:}resourcetype')) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->filetype('/some%dir/file%type'); + } + + public function testFileExists() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn(true); + + $this->assertTrue($this->instance->file_exists('/some%dir/file%.txt')); + + // stat cache: calling again does not redo a propfind + $this->assertTrue($this->instance->file_exists('/some%dir/file%.txt')); + } + + public function testFileExistsDoesNot() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn(false); + + $this->assertFalse($this->instance->file_exists('/some%dir/file%.txt')); + + // stat cache: calling again does not redo a propfind + $this->assertFalse($this->instance->file_exists('/some%dir/file%.txt')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testFileExistsException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->file_exists('/some%dir/file%.txt'); + } + + public function testUnlink() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25file.txt', null) + ->willReturn(['statusCode' => Http::STATUS_NO_CONTENT]); + + $this->assertTrue($this->instance->unlink('/old%file.txt')); + } + + public function testUnlinkUnexist() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25file.txt', null) + ->willReturn(['statusCode' => Http::STATUS_NOT_FOUND]); + + $this->assertFalse($this->instance->unlink('/old%file.txt')); + } + + public function testUnlinkUnexist2() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25file.txt', null) + ->willThrowException($this->createClientHttpException(Http::STATUS_NOT_FOUND)); + + $this->assertFalse($this->instance->unlink('/old%file.txt')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testUnlinkException() { + $this->davClient->expects($this->once()) + ->method('request') + ->with('DELETE', 'old%25file.txt', null) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->unlink('/old%file.txt'); + } + + public function testFopenRead() { + $response = $this->createMock(\GuzzleHttp\Message\ResponseInterface::class); + $response->method('getStatusCode')->willReturn(Http::STATUS_OK); + $response->method('getBody')->willReturn(fopen('data://text/plain,response body', 'r')); + + $this->httpClient->expects($this->once()) + ->method('get') + ->with( + 'https://davhost/davroot/some%25dir/file%25.txt', [ + 'auth' => ['davuser', 'davpassword'], + 'stream' => true + ] + ) + ->willReturn($response); + + $fh = $this->instance->fopen('/some%dir/file%.txt', 'r'); + $contents = stream_get_contents($fh); + fclose($fh); + + $this->assertEquals('response body', $contents); + } + + public function testFopenReadNotFound() { + $this->httpClient->expects($this->once()) + ->method('get') + ->with( + 'https://davhost/davroot/some%25dir/file%25.txt', [ + 'auth' => ['davuser', 'davpassword'], + 'stream' => true + ] + ) + ->willThrowException($this->createGuzzleClientException(Http::STATUS_NOT_FOUND)); + + $this->assertFalse($this->instance->fopen('/some%dir/file%.txt', 'r')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testFopenReadException() { + $this->httpClient->expects($this->once()) + ->method('get') + ->with( + 'https://davhost/davroot/some%25dir/file%25.txt', [ + 'auth' => ['davuser', 'davpassword'], + 'stream' => true + ] + ) + ->willThrowException($this->createGuzzleClientException(Http::STATUS_FORBIDDEN)); + + $this->instance->fopen('/some%dir/file%.txt', 'r'); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testFopenReadLockedException() { + $response = $this->createMock(\GuzzleHttp\Message\ResponseInterface::class); + $response->method('getStatusCode')->willReturn(Http::STATUS_LOCKED); + $response->method('getBody')->willReturn(fopen('data://text/plain,response body', 'r')); + + $this->httpClient->expects($this->once()) + ->method('get') + ->with( + 'https://davhost/davroot/some%25dir/file%25.txt', [ + 'auth' => ['davuser', 'davpassword'], + 'stream' => true + ] + ) + ->willReturn($response); + + $this->instance->fopen('/some%dir/file%.txt', 'r'); + } + + public function testFopenWriteNewFile() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn(false); + + // isCreatable on parent / getPermissions + $this->davClient->expects($this->at(1)) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'RDWCK' + ]); + + $uploadUrl = null; + $uploadOptions = null; + $this->httpClient->expects($this->once()) + ->method('put') + ->will($this->returnCallback(function($url, $options) use (&$uploadUrl, &$uploadOptions) { + $uploadUrl = $url; + $uploadOptions = $options; + })); + + $fh = $this->instance->fopen('/some%dir/file%.txt', 'w'); + fwrite($fh, 'whatever'); + fclose($fh); + + $this->assertEquals('https://davhost/davroot/some%25dir/file%25.txt', $uploadUrl); + $this->assertEquals(['davuser', 'davpassword'], $uploadOptions['auth']); + $this->assertEquals('whatever', stream_get_contents($uploadOptions['body'])); + } + + public function testFopenWriteNewFileNoPermission() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn(false); + + // isCreatable on parent / getPermissions + $this->davClient->expects($this->at(1)) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'R' + ]); + + $this->httpClient->expects($this->never()) + ->method('put'); + + $this->assertFalse($this->instance->fopen('/some%dir/file%.txt', 'w')); + } + + public function testFopenWriteExistingFile() { + // file_exists, and cached response for isUpdatable / getPermissions + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'RDWCK' + ]); + + $uploadUrl = null; + $uploadOptions = null; + $this->httpClient->expects($this->once()) + ->method('put') + ->will($this->returnCallback(function($url, $options) use (&$uploadUrl, &$uploadOptions) { + $uploadUrl = $url; + $uploadOptions = $options; + })); + + $fh = $this->instance->fopen('/some%dir/file%.txt', 'w'); + fwrite($fh, 'whatever'); + fclose($fh); + + $this->assertEquals('https://davhost/davroot/some%25dir/file%25.txt', $uploadUrl); + $this->assertEquals(['davuser', 'davpassword'], $uploadOptions['auth']); + $this->assertEquals('whatever', stream_get_contents($uploadOptions['body'])); + } + + public function testFopenWriteExistingFileNoPermission() { + // file_exists, and cached response for isUpdatable / getPermissions + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'R' + ]); + + $this->httpClient->expects($this->never()) + ->method('put'); + + $this->assertFalse($this->instance->fopen('/some%dir/file%.txt', 'w')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testFopenWriteExceptionEarly() { + // file_exists, and cached response for isUpdatable / getPermissions + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->httpClient->expects($this->never()) + ->method('put'); + + $this->instance->fopen('/some%dir/file%.txt', 'w'); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testFopenWriteExceptionLate() { + // file_exists, and cached response for isUpdatable / getPermissions + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'RDWCK' + ]); + + $uploadUrl = null; + $uploadOptions = null; + $this->httpClient->expects($this->once()) + ->method('put') + ->willThrowException($this->createGuzzleClientException(Http::STATUS_FORBIDDEN)); + + $fh = $this->instance->fopen('/some%dir/file%.txt', 'w'); + fwrite($fh, 'whatever'); + fclose($fh); + } + + public function freespaceProvider() { + return [ + [false, FileInfo::SPACE_UNKNOWN], + [['{DAV:}quota-available-bytes' => 123], 123], + [['{DAV:}quota-available-bytes' => FileInfo::SPACE_UNKNOWN], FileInfo::SPACE_UNKNOWN], + [[], FileInfo::SPACE_UNKNOWN], + ]; + } + + /** + * @dataProvider freespaceProvider + */ + public function testFreeSpace($propFindResponse, $apiResponse) { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}quota-available-bytes'), 0) + ->willReturn($propFindResponse); + + $this->assertEquals($apiResponse, $this->instance->free_space('/some%dir')); + } + + public function testFreeSpaceException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->assertEquals(FileInfo::SPACE_UNKNOWN, $this->instance->free_space('/some%dir')); + } + + public function touchProvider() { + return [ + // server accepted mtime + [1508496363, null, '2017-10-20T12:46:03+02:00', true], + // server did not accept mtime + [1508496363, null, '2017-10-20T12:40:00+02:00', false], + // time factory generated mtime + [null, 1508496363, '2017-10-20T12:46:03+02:00', true], + ]; + } + + /** + * @dataProvider touchProvider + */ + public function testTouchExisting($setMtime, $factoryTime, $readMtime, $expectedResult) { + if ($factoryTime !== null) { + $this->timeFactory->expects($this->once()) + ->method('getTime') + ->willReturn($factoryTime); + } else { + $this->timeFactory->expects($this->never()) + ->method('getTime'); + } + + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir') + ->willReturn([]); + + $this->davClient->expects($this->at(1)) + ->method('proppatch') + ->with('some%25dir', ['{DAV:}lastmodified' => $setMtime || $factoryTime]); + + // propfind after proppatch, to check if applied + $this->davClient->expects($this->at(2)) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getlastmodified'), 0) + ->willReturn([ + '{DAV:}getlastmodified' => $readMtime + ]); + + $this->assertEquals($expectedResult, $this->instance->touch('/some%dir', $setMtime)); + } + + public function testTouchNonExisting() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir/file%25.txt') + ->willReturn(false); + + // isCreatable on parent / getPermissions + $this->davClient->expects($this->at(1)) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn([ + '{http://owncloud.org/ns}permissions' => 'RDWCK' + ]); + + $uploadUrl = null; + $uploadOptions = null; + $this->httpClient->expects($this->once()) + ->method('put') + ->will($this->returnCallback(function($url, $options) use (&$uploadUrl, &$uploadOptions) { + $uploadUrl = $url; + $uploadOptions = $options; + })); + + $this->assertTrue($this->instance->touch('/some%dir/file%.txt')); + + $this->assertEquals('https://davhost/davroot/some%25dir/file%25.txt', $uploadUrl); + $this->assertEquals(['davuser', 'davpassword'], $uploadOptions['auth']); + $this->assertEquals('', stream_get_contents($uploadOptions['body'])); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testTouchException() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir') + ->willReturn([]); + + $this->davClient->expects($this->at(1)) + ->method('proppatch') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->touch('/some%dir', 1508496363); + } + + public function testTouchNotFound() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir') + ->willReturn([]); + + $this->davClient->expects($this->at(1)) + ->method('proppatch'); + + // maybe the file disappeared in-between ? + $this->davClient->expects($this->at(2)) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getlastmodified'), 0) + ->willReturn(false); + + $this->assertFalse($this->instance->touch('/some%dir', 1508496363)); + } + + public function testTouchNoServerSupport() { + // file_exists + $this->davClient->expects($this->at(0)) + ->method('propfind') + ->with('some%25dir') + ->willReturn([]); + + $this->davClient->expects($this->at(1)) + ->method('proppatch') + ->willThrowException($this->createClientHttpException(Http::STATUS_NOT_IMPLEMENTED)); + + $this->assertFalse($this->instance->touch('/some%dir', 1508496363)); + } + + public function renameDataProvider() { + return [ + ['MOVE', 'rename', false, ''], + ['MOVE', 'rename', true, '/'], + ['COPY', 'copy', false, ''], + ['COPY', 'copy', true, '/'], + ]; + } + + /** + * @dataProvider renameDataProvider + */ + public function testRename($httpMethod, $storageMethod, $isDir, $extra) { + $mock = $this->davClient->expects($this->once()) + ->method('propfind') + ->with('new%25path/new%25file.txt', $this->contains('{DAV:}resourcetype')); + $mock->willReturn([ + '{DAV:}resourcetype' => $this->getResourceTypeResponse($isDir) + ]); + + $this->davClient->expects($this->once()) + ->method('request') + ->with($httpMethod, 'old%25path/old%25file.txt', null, ['Destination' => 'https://davhost/davroot/new%25path/new%25file.txt' . $extra]) + ->willReturn(['statusCode' => Http::STATUS_OK]); + + $this->assertTrue($this->instance->$storageMethod('/old%path/old%file.txt', '/new%path/new%file.txt')); + } + + public function statDataProvider() { + return [ + [[ + '{DAV:}getlastmodified' => '2017-10-20T12:46:03+02:00', + '{DAV:}getcontentlength' => 1024, + ], [ + 'mtime' => 1508496363, + 'size' => 1024 + ]], + [[ + '{DAV:}getlastmodified' => '2017-10-20T12:46:03+02:00', + ], [ + 'mtime' => 1508496363, + 'size' => 0 + ]], + ]; + } + + /** + * @dataProvider statDataProvider + */ + public function testStat($davResponse, $apiResponse) { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->logicalAnd($this->contains('{DAV:}getlastmodified'), $this->contains('{DAV:}getcontentlength'))) + ->willReturn($davResponse); + + $this->assertEquals($apiResponse, $this->instance->stat('/some%dir/file%type')); + } + + public function testStatRoot() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('', $this->logicalAnd($this->contains('{DAV:}getlastmodified'), $this->contains('{DAV:}getcontentlength'))) + ->willReturn(['{DAV:}getlastmodified' => '2017-10-20T12:46:03+02:00']); + + $this->assertEquals(['mtime' => 1508496363, 'size' => 0], $this->instance->stat('')); + $this->assertEquals(['mtime' => 1508496363, 'size' => 0], $this->instance->stat('')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testStatException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->stat('/some%dir/file%type'); + } + + public function mimeTypeDataProvider() { + return [ + [ + [ + '{DAV:}resourcetype' => $this->getResourceTypeResponse(true), + ], + 'httpd/unix-directory' + ], + [ + [ + '{DAV:}resourcetype' => $this->getResourceTypeResponse(false), + '{DAV:}getcontenttype' => 'text/plain' + ], + 'text/plain' + ], + [ + [ + '{DAV:}getcontenttype' => 'text/plain' + ], + 'text/plain' + ], + [ + [ + ], + false + ], + ]; + } + + /** + * @dataProvider mimeTypeDataProvider + */ + public function testMimeType($davResponse, $apiResponse) { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir/file%25type', $this->logicalAnd($this->contains('{DAV:}resourcetype'), $this->contains('{DAV:}getcontenttype'))) + ->willReturn($davResponse); + + $this->assertEquals($apiResponse, $this->instance->getMimeType('/some%dir/file%type')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testMimeTypeException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->getMimeType('/some%dir/file%type'); + } + + public function permissionsDataProvider() { + return [ + ['CK', true, true, false, false], + ['W', false, true, false, false], + ['D', false, false, true, false], + ['R', false, false, false, true], + ['RDWCK', true, true, true, true], + ]; + } + + /** + * @dataProvider permissionsDataProvider + */ + public function testPermissions($perms, $creatable, $updatable, $deletable, $sharable) { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn([ + '{http://owncloud.org/ns}permissions' => $perms + ]); + + $path = 'some%dir'; + $this->assertEquals($creatable, $this->instance->isCreatable($path)); + $this->assertEquals($updatable, $this->instance->isUpdatable($path)); + $this->assertEquals($deletable, $this->instance->isDeletable($path)); + $this->assertEquals($sharable, $this->instance->isSharable($path)); + } + + public function testNoPermissionsDir() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn(['{DAV:}resourcetype' => $this->getResourceTypeResponse(true)]); + + // all perms given + $path = 'some%dir'; + $this->assertTrue($this->instance->isCreatable($path)); + $this->assertTrue($this->instance->isUpdatable($path)); + $this->assertTrue($this->instance->isDeletable($path)); + $this->assertTrue($this->instance->isSharable($path)); + + } + + public function testNoPermissionsFile() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn(['{DAV:}resourcetype' => $this->getResourceTypeResponse(false)]); + + // all perms given except create + $path = 'some%dir'; + $this->assertFalse($this->instance->isCreatable($path)); + $this->assertTrue($this->instance->isUpdatable($path)); + $this->assertTrue($this->instance->isDeletable($path)); + $this->assertTrue($this->instance->isSharable($path)); + + } + + public function testGetPermissionsUnexist() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{http://owncloud.org/ns}permissions')) + ->willReturn(false); + + // all perms given + $path = 'some%dir'; + $this->assertFalse($this->instance->isCreatable($path)); + $this->assertFalse($this->instance->isUpdatable($path)); + $this->assertFalse($this->instance->isDeletable($path)); + $this->assertFalse($this->instance->isSharable($path)); + $this->assertEquals(0, $this->instance->getPermissions($path)); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testGetPermissionsException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $path = 'some%dir'; + $this->instance->isSharable($path); + } + + public function testGetEtag() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getetag')) + ->willReturn([ + '{DAV:}getetag' => '"thisisanetagisntit"' + ]); + + $this->assertEquals('thisisanetagisntit', $this->instance->getETag('some%dir')); + } + + public function testGetEtagFallback() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getetag')) + ->willReturn([]); + + // unique id + $this->assertInternalType('string', $this->instance->getETag('some%dir')); + } + + public function testGetEtagUnexist() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getetag')) + ->willReturn(false); + + $this->assertNull($this->instance->getETag('some%dir')); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testGetEtagException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', $this->contains('{DAV:}getetag')) + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->getETag('some%dir'); + } + + public function hasUpdatedDataProvider() { + return [ + // etag did not change + [ + [ + '{DAV:}getetag' => '"oldetag"' + ], + [ + 'etag' => 'oldetag' + ], + false + ], + // etag changed + [ + [ + '{DAV:}getetag' => '"newetag"' + ], + [ + 'etag' => 'oldetag' + ], + true + ], + // etag did not change, share permissions did + [ + [ + '{DAV:}getetag' => '"oldetag"', + '{http://open-collaboration-services.org/ns}share-permissions' => 1 + ], + [ + 'etag' => 'oldetag', + 'permissions' => 31 + ], + true + ], + // etag did not change, share permissions did not + [ + [ + '{DAV:}getetag' => '"oldetag"', + '{http://open-collaboration-services.org/ns}share-permissions' => 1 + ], + [ + 'etag' => 'oldetag', + 'permissions' => 1 + ], + false + ], + // etag did not change, regular permissions did + [ + [ + '{DAV:}getetag' => '"oldetag"', + '{http://owncloud.org/ns}permissions' => 1 + ], + [ + 'etag' => 'oldetag', + 'permissions' => 31 + ], + true + ], + // etag did not change, regular permissions did not + [ + [ + '{DAV:}getetag' => '"oldetag"', + '{http://owncloud.org/ns}permissions' => 1 + ], + [ + 'etag' => 'oldetag', + 'permissions' => 1 + ], + false + ], + // no etag, fallback to last modified, unchanged case + [ + [ + '{DAV:}getlastmodified' => '2017-10-20T12:46:03+02:00' + ], + null, + false + ], + // no etag, fallback to last modified, older case + [ + [ + '{DAV:}getlastmodified' => '2017-10-20T12:40:03+02:00' + ], + null, + false + ], + // no etag, fallback to last modified, remote mtime higher case + [ + [ + '{DAV:}getlastmodified' => '2017-10-20T12:50:03+02:00' + ], + null, + true + ], + ]; + } + + /** + * @dataProvider hasUpdatedDataProvider + */ + public function testHasUpdated($davResponse, $cacheResponse, $expectedResult) { + $this->davClient->expects($this->once()) + ->method('propfind') + ->with('some%25dir', + $this->logicalAnd( + $this->contains('{DAV:}getetag'), + $this->contains('{DAV:}getlastmodified'), + $this->contains('{http://owncloud.org/ns}permissions'), + $this->contains('{http://open-collaboration-services.org/ns}share-permissions') + ) + ) + ->willReturn($davResponse); + + if ($cacheResponse !== null) { + $this->cache->expects($this->once()) + ->method('get') + ->with('some%dir') + ->willReturn($cacheResponse); + } else { + $this->cache->expects($this->never()) + ->method('get'); + } + + $this->assertEquals($expectedResult, $this->instance->hasUpdated('some%dir', 1508496363)); + } + + public function testHasUpdatedPathNotfound() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willReturn(false); + + $this->assertFalse($this->instance->hasUpdated('some%dir', 1508496363)); + } + + /** + * @expectedException OCP\Files\StorageNotAvailableException + */ + public function testHasUpdatedRootPathNotfound() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willReturn(false); + + $this->instance->hasUpdated('', 1508496363); + } + + /** + * @expectedException OCP\Files\StorageNotAvailableException + */ + public function testHasUpdatedRootPathMethodNotAllowed() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_METHOD_NOT_ALLOWED)); + + $this->instance->hasUpdated('', 1508496363); + } + + /** + * @expectedException OCP\Files\StorageNotAvailableException + */ + public function testHasUpdatedMethodNotAllowed() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_METHOD_NOT_ALLOWED)); + + $this->assertFalse($this->instance->hasUpdated('some%dir', 1508496363)); + } + + /** + * @expectedException \OCA\DAV\Connector\Sabre\Exception\Forbidden + */ + public function testHasUpdatedException() { + $this->davClient->expects($this->once()) + ->method('propfind') + ->willThrowException($this->createClientHttpException(Http::STATUS_FORBIDDEN)); + + $this->instance->hasUpdated('some%dir', 1508496363); + } +} + diff --git a/tests/lib/Http/Client/WebdavClientServiceTest.php b/tests/lib/Http/Client/WebdavClientServiceTest.php new file mode 100644 index 000000000000..73124afea86d --- /dev/null +++ b/tests/lib/Http/Client/WebdavClientServiceTest.php @@ -0,0 +1,101 @@ + + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @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 Test\Http\Client; + +use OCP\IConfig; +use OCP\ICertificateManager; +use OC\Http\Client\WebdavClientService; +use Sabre\DAV\Client; +use OCP\ITempManager; + +/** + * Class WebdavClientServiceTest + */ +class WebdavClientServiceTest extends \Test\TestCase { + /** + * @var ITempManager + */ + private $tempManager; + + public function setUp() { + parent::setUp(); + $this->tempManager = \OC::$server->getTempManager(); + } + + public function tearDown() { + $this->tempManager->clean(); + parent::tearDown(); + } + + public function testNewClient() { + $config = $this->createMock(IConfig::class); + $certificateManager = $this->createMock(ICertificateManager::class); + $certificateManager->method('getAbsoluteBundlePath') + ->willReturn($this->tempManager->getTemporaryFolder()); + + $clientService = new WebdavClientService($config, $certificateManager); + + $client = $clientService->newClient([ + 'baseUri' => 'https://davhost/davroot/', + 'userName' => 'davUser' + ]); + + $this->assertInstanceOf(Client::class, $client); + } + + public function testNewClientWithProxy() { + $config = $this->createMock(IConfig::class); + $config->expects($this->once()) + ->method('getSystemValue') + ->with('proxy', '') + ->willReturn('proxyhost'); + + $certificateManager = $this->createMock(ICertificateManager::class); + $certificateManager->method('getAbsoluteBundlePath') + ->willReturn($this->tempManager->getTemporaryFolder()); + + $clientService = new WebdavClientService($config, $certificateManager); + + $client = $clientService->newClient([ + 'baseUri' => 'https://davhost/davroot/', + 'userName' => 'davUser' + ]); + + $this->assertInstanceOf(Client::class, $client); + } + + public function testNewClientWithoutCertificate() { + $config = $this->createMock(IConfig::class); + $certificateManager = $this->createMock(ICertificateManager::class); + $certificateManager->method('getAbsoluteBundlePath') + ->willReturn($this->tempManager->getTemporaryFolder() . '/unexist'); + + $clientService = new WebdavClientService($config, $certificateManager); + + $client = $clientService->newClient([ + 'baseUri' => 'https://davhost/davroot/', + 'userName' => 'davUser' + ]); + + $this->assertInstanceOf(Client::class, $client); + } +} From 7e9b5ad416cc27f2aa0f4c1ad453bd611519a455 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 23 Oct 2017 13:48:52 +0200 Subject: [PATCH 6/6] Small tweaks for WebDavClientService and tests --- lib/private/Files/Storage/DAV.php | 8 ++-- ...entService.php => WebDavClientService.php} | 6 +-- lib/private/Server.php | 10 ++-- ...ntService.php => IWebDavClientService.php} | 4 +- tests/lib/Files/Storage/DavTest.php | 48 +++++++++---------- ...ceTest.php => WebDavClientServiceTest.php} | 12 ++--- 6 files changed, 44 insertions(+), 44 deletions(-) rename lib/private/Http/Client/{WebdavClientService.php => WebDavClientService.php} (95%) rename lib/public/Http/Client/{IWebdavClientService.php => IWebDavClientService.php} (95%) rename tests/lib/Http/Client/{WebdavClientServiceTest.php => WebDavClientServiceTest.php} (89%) diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index 0939b2941ef4..bed3181cdcdc 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -83,8 +83,8 @@ class DAV extends Common { /** @var \OCP\Http\Client\IClientService */ private $httpClientService; - /** @var \OCP\Http\Client\IWebdavClientService */ - private $webdavClientService; + /** @var \OCP\Http\Client\IWebDavClientService */ + private $webDavClientService; /** * @param array $params @@ -93,7 +93,7 @@ class DAV extends Common { public function __construct($params) { $this->statCache = new ArrayCache(); $this->httpClientService = \OC::$server->getHTTPClientService(); - $this->webdavClientService = \OC::$server->getWebdavClientService(); + $this->webDavClientService = \OC::$server->getWebDavClientService(); if (isset($params['host']) && isset($params['user']) && isset($params['password'])) { $host = $params['host']; //remove leading http[s], will be generated in createBaseUri() @@ -141,7 +141,7 @@ protected function init() { $settings['authType'] = $this->authType; } - $this->client = $this->webdavClientService->newClient($settings); + $this->client = $this->webDavClientService->newClient($settings); } /** diff --git a/lib/private/Http/Client/WebdavClientService.php b/lib/private/Http/Client/WebDavClientService.php similarity index 95% rename from lib/private/Http/Client/WebdavClientService.php rename to lib/private/Http/Client/WebDavClientService.php index ff07909d9f0d..84ad5aacf8f9 100644 --- a/lib/private/Http/Client/WebdavClientService.php +++ b/lib/private/Http/Client/WebDavClientService.php @@ -21,17 +21,17 @@ namespace OC\Http\Client; -use OCP\Http\Client\IWebdavClientService; +use OCP\Http\Client\IWebDavClientService; use OCP\IConfig; use OCP\ICertificateManager; use Sabre\DAV\Client; /** - * Class WebdavClientService + * Class WebDavClientService * * @package OC\Http */ -class WebdavClientService implements IWebdavClientService { +class WebDavClientService implements IWebDavClientService { /** @var IConfig */ private $config; /** @var ICertificateManager */ diff --git a/lib/private/Server.php b/lib/private/Server.php index 3717335f8b6f..c84edfe31f86 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -501,10 +501,10 @@ public function __construct($webRoot, \OC\Config $config) { new \OC\Security\CertificateManager($uid, new View(), $c->getConfig()) ); }); - $this->registerService('WebdavClientService', function (Server $c) { + $this->registerService('WebDavClientService', function (Server $c) { $user = \OC_User::getUser(); $uid = $user ? $user : null; - return new WebdavClientService( + return new WebDavClientService( $c->getConfig(), new \OC\Security\CertificateManager($uid, new View(), $c->getConfig()) ); @@ -1285,10 +1285,10 @@ public function getHTTPClientService() { /** * Returns an instance of the Webdav client service * - * @return \OCP\Http\Client\IWebdavClientService + * @return \OCP\Http\Client\IWebDavClientService */ - public function getWebdavClientService() { - return $this->query('WebdavClientService'); + public function getWebDavClientService() { + return $this->query('WebDavClientService'); } /** diff --git a/lib/public/Http/Client/IWebdavClientService.php b/lib/public/Http/Client/IWebDavClientService.php similarity index 95% rename from lib/public/Http/Client/IWebdavClientService.php rename to lib/public/Http/Client/IWebDavClientService.php index d6e8b48f47f3..950e55f52575 100644 --- a/lib/public/Http/Client/IWebdavClientService.php +++ b/lib/public/Http/Client/IWebDavClientService.php @@ -24,12 +24,12 @@ use Sabre\HTTP\Client; /** - * Interface IWebdavClientService + * Interface IWebDavClientService * * @package OCP\Http * @since 10.0.4 */ -interface IWebdavClientService { +interface IWebDavClientService { /** * Settings are provided through the 'settings' argument. The following * settings are supported: diff --git a/tests/lib/Files/Storage/DavTest.php b/tests/lib/Files/Storage/DavTest.php index 3ca57b9f3884..b430f5ba7d88 100644 --- a/tests/lib/Files/Storage/DavTest.php +++ b/tests/lib/Files/Storage/DavTest.php @@ -25,7 +25,7 @@ use OCP\Http\Client\IClientService; use OCP\Http\Client\IClient; use Sabre\DAV\Client; -use OCP\Http\Client\IWebdavClientService; +use OCP\Http\Client\IWebDavClientService; use Sabre\HTTP\ClientHttpException; use OCP\Lock\LockedException; use OCP\AppFramework\Http; @@ -50,37 +50,37 @@ class DavTest extends TestCase { /** - * @var DAV + * @var DAV | \PHPUnit_Framework_MockObject_MockObject */ private $instance; /** - * @var IClientService + * @var IClientService | \PHPUnit_Framework_MockObject_MockObject */ private $httpClientService; /** - * @var IWebdavClientService + * @var IWebDavClientService | \PHPUnit_Framework_MockObject_MockObject */ - private $webdavClientService; + private $webDavClientService; /** - * @var Client + * @var Client | \PHPUnit_Framework_MockObject_MockObject */ private $davClient; /** - * @var IClient + * @var IClient | \PHPUnit_Framework_MockObject_MockObject **/ private $httpClient; /** - * @var ITimeFactory + * @var ITimeFactory | \PHPUnit_Framework_MockObject_MockObject */ private $timeFactory; /** - * @var Cache + * @var Cache | \PHPUnit_Framework_MockObject_MockObject */ private $cache; @@ -90,8 +90,8 @@ protected function setUp() { $this->httpClientService = $this->createMock(IClientService::class); $this->overwriteService('HttpClientService', $this->httpClientService); - $this->webdavClientService = $this->createMock(IWebdavClientService::class); - $this->overwriteService('WebdavClientService', $this->webdavClientService); + $this->webDavClientService = $this->createMock(IWebDavClientService::class); + $this->overwriteService('WebDavClientService', $this->webDavClientService); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->overwriteService('TimeFactory', $this->timeFactory); @@ -100,7 +100,7 @@ protected function setUp() { $this->httpClientService->method('newClient')->willReturn($this->httpClient); $this->davClient = $this->createMock(Client::class); - $this->webdavClientService->method('newClient')->willReturn($this->davClient); + $this->webDavClientService->method('newClient')->willReturn($this->davClient); $this->instance = $this->getMockBuilder(\OC\Files\Storage\DAV::class) ->setConstructorArgs([[ @@ -119,7 +119,7 @@ protected function setUp() { protected function tearDown() { $this->restoreService('HttpClientService'); - $this->restoreService('WebdavClientService'); + $this->restoreService('WebDavClientService'); $this->restoreService('TimeFactory'); parent::tearDown(); } @@ -128,7 +128,7 @@ public function testId() { $this->assertEquals('webdav::davuser@davhost//davroot/', $this->instance->getId()); } - public function instantiateWebdavClientDataProvider() { + public function instantiateWebDavClientDataProvider() { return [ [false, 'http'], [true, 'https'], @@ -136,13 +136,13 @@ public function instantiateWebdavClientDataProvider() { } /** - * @dataProvider instantiateWebdavClientDataProvider + * @dataProvider instantiateWebDavClientDataProvider */ - public function testInstantiateWebdavClient($secure, $protocol) { - $this->restoreService('WebdavClientService'); - $this->webdavClientService = $this->createMock(IWebdavClientService::class); - $this->overwriteService('WebdavClientService', $this->webdavClientService); - $this->webdavClientService->expects($this->once()) + public function testInstantiateWebDavClient($secure, $protocol) { + $this->restoreService('WebDavClientService'); + $this->webDavClientService = $this->createMock(IWebDavClientService::class); + $this->overwriteService('WebDavClientService', $this->webDavClientService); + $this->webDavClientService->expects($this->once()) ->method('newClient') ->with([ 'baseUri' => $protocol . '://davhost/davroot/', @@ -187,7 +187,7 @@ public function invalidConfigDataProvider() { * @dataProvider invalidConfigDataProvider * @expectedException \InvalidArgumentException */ - public function testInstantiateWebdavClientInvalidConfig($params) { + public function testInstantiateWebDavClientInvalidConfig($params) { new \OC\Files\Storage\DAV($params); } @@ -1273,7 +1273,7 @@ public function testHasUpdatedPathNotfound() { } /** - * @expectedException OCP\Files\StorageNotAvailableException + * @expectedException \OCP\Files\StorageNotAvailableException */ public function testHasUpdatedRootPathNotfound() { $this->davClient->expects($this->once()) @@ -1284,7 +1284,7 @@ public function testHasUpdatedRootPathNotfound() { } /** - * @expectedException OCP\Files\StorageNotAvailableException + * @expectedException \OCP\Files\StorageNotAvailableException */ public function testHasUpdatedRootPathMethodNotAllowed() { $this->davClient->expects($this->once()) @@ -1295,7 +1295,7 @@ public function testHasUpdatedRootPathMethodNotAllowed() { } /** - * @expectedException OCP\Files\StorageNotAvailableException + * @expectedException \OCP\Files\StorageNotAvailableException */ public function testHasUpdatedMethodNotAllowed() { $this->davClient->expects($this->once()) diff --git a/tests/lib/Http/Client/WebdavClientServiceTest.php b/tests/lib/Http/Client/WebDavClientServiceTest.php similarity index 89% rename from tests/lib/Http/Client/WebdavClientServiceTest.php rename to tests/lib/Http/Client/WebDavClientServiceTest.php index 73124afea86d..460bbe9a4358 100644 --- a/tests/lib/Http/Client/WebdavClientServiceTest.php +++ b/tests/lib/Http/Client/WebDavClientServiceTest.php @@ -23,14 +23,14 @@ use OCP\IConfig; use OCP\ICertificateManager; -use OC\Http\Client\WebdavClientService; +use OC\Http\Client\WebDavClientService; use Sabre\DAV\Client; use OCP\ITempManager; /** - * Class WebdavClientServiceTest + * Class WebDavClientServiceTest */ -class WebdavClientServiceTest extends \Test\TestCase { +class WebDavClientServiceTest extends \Test\TestCase { /** * @var ITempManager */ @@ -52,7 +52,7 @@ public function testNewClient() { $certificateManager->method('getAbsoluteBundlePath') ->willReturn($this->tempManager->getTemporaryFolder()); - $clientService = new WebdavClientService($config, $certificateManager); + $clientService = new WebDavClientService($config, $certificateManager); $client = $clientService->newClient([ 'baseUri' => 'https://davhost/davroot/', @@ -73,7 +73,7 @@ public function testNewClientWithProxy() { $certificateManager->method('getAbsoluteBundlePath') ->willReturn($this->tempManager->getTemporaryFolder()); - $clientService = new WebdavClientService($config, $certificateManager); + $clientService = new WebDavClientService($config, $certificateManager); $client = $clientService->newClient([ 'baseUri' => 'https://davhost/davroot/', @@ -89,7 +89,7 @@ public function testNewClientWithoutCertificate() { $certificateManager->method('getAbsoluteBundlePath') ->willReturn($this->tempManager->getTemporaryFolder() . '/unexist'); - $clientService = new WebdavClientService($config, $certificateManager); + $clientService = new WebDavClientService($config, $certificateManager); $client = $clientService->newClient([ 'baseUri' => 'https://davhost/davroot/',