From 046638abeb50ccfd07069ac3c4f9c9428c3f23ed Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 14 Jan 2021 19:03:39 +0100 Subject: [PATCH 1/5] do cachejail search filtering in sql Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 2 +- lib/private/Files/Cache/QuerySearchHelper.php | 2 + lib/private/Files/Cache/Wrapper/CacheJail.php | 60 ++++++++++++++++++- .../lib/Files/Cache/Wrapper/CacheJailTest.php | 37 ++++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 2513abd525..840523c189 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -121,7 +121,7 @@ class Cache implements ICache { $this->querySearchHelper = new QuerySearchHelper($this->mimetypeLoader); } - private function getQueryBuilder() { + protected function getQueryBuilder() { return new CacheQueryBuilder( $this->connection, \OC::$server->getSystemConfig(), diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 574b4c18d5..5e33ad235a 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -175,6 +175,7 @@ class QuerySearchHelper { 'mimetype' => 'string', 'mtime' => 'integer', 'name' => 'string', + 'path' => 'string', 'size' => 'integer', 'tagname' => 'string', 'favorite' => 'boolean', @@ -184,6 +185,7 @@ class QuerySearchHelper { 'mimetype' => ['eq', 'like'], 'mtime' => ['eq', 'gt', 'lt', 'gte', 'lte'], 'name' => ['eq', 'like'], + 'path' => ['eq', 'like'], 'size' => ['eq', 'gt', 'lt', 'gte', 'lte'], 'tagname' => ['eq', 'like'], 'favorite' => ['eq'], diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 6c1c17be02..27590fb741 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -29,8 +29,13 @@ namespace OC\Files\Cache\Wrapper; use OC\Files\Cache\Cache; +use OC\Files\Search\SearchBinaryOperator; +use OC\Files\Search\SearchComparison; use OC\Files\Search\SearchQuery; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Search\ISearchBinaryOperator; +use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchQuery; /** @@ -49,6 +54,8 @@ class CacheJail extends CacheWrapper { public function __construct($cache, $root) { parent::__construct($cache); $this->root = $root; + $this->connection = \OC::$server->getDatabaseConnection(); + $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); } protected function getRoot() { @@ -221,7 +228,26 @@ class CacheJail extends CacheWrapper { * @return array an array of file data */ public function search($pattern) { - $results = $this->getCache()->search($pattern); + // normalize pattern + $pattern = $this->normalize($pattern); + + if ($pattern === '%%') { + return []; + } + + $query = $this->getQueryBuilder(); + $query->selectFileCache() + ->whereStorageId() + ->andWhere($query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%'))) + ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); + + $result = $query->execute(); + $files = $result->fetchAll(); + $result->closeCursor(); + + $results = array_map(function (array $data) { + return self::cacheEntryFromData($data, $this->mimetypeLoader); + }, $files); return $this->formatSearchResults($results); } @@ -232,12 +258,40 @@ class CacheJail extends CacheWrapper { * @return array */ public function searchByMime($mimetype) { - $results = $this->getCache()->searchByMime($mimetype); + $mimeId = $this->mimetypeLoader->getId($mimetype); + + $query = $this->getQueryBuilder(); + $query->selectFileCache() + ->whereStorageId() + ->andWhere($query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%'))); + + if (strpos($mimetype, '/')) { + $query->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); + } else { + $query->andWhere($query->expr()->eq('mimepart', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); + } + + $result = $query->execute(); + $files = $result->fetchAll(); + $result->closeCursor(); + + $results = array_map(function (array $data) { + return self::cacheEntryFromData($data, $this->mimetypeLoader); + }, $files); return $this->formatSearchResults($results); } public function searchQuery(ISearchQuery $query) { - $simpleQuery = new SearchQuery($query->getSearchOperation(), 0, 0, $query->getOrder(), $query->getUser()); + $prefixFilter = new SearchComparison( + ISearchComparison::COMPARE_LIKE, + 'path', + $this->getRoot() . '/%' + ); + $operation = new SearchBinaryOperator( + ISearchBinaryOperator::OPERATOR_AND, + [$prefixFilter, $query->getSearchOperation()] + ); + $simpleQuery = new SearchQuery($operation, 0, 0, $query->getOrder(), $query->getUser()); $results = $this->getCache()->searchQuery($simpleQuery); $results = $this->formatSearchResults($results); diff --git a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php index f0943ba5a0..83173efd3b 100644 --- a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php +++ b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php @@ -9,6 +9,11 @@ namespace Test\Files\Cache\Wrapper; use OC\Files\Cache\Wrapper\CacheJail; +use OC\Files\Search\SearchComparison; +use OC\Files\Search\SearchQuery; +use OC\User\User; +use OCP\Files\Search\ISearchComparison; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\Files\Cache\CacheTest; /** @@ -46,6 +51,38 @@ class CacheJailTest extends CacheTest { $this->assertEquals('foobar', $result[0]['path']); } + public function testSearchMimeOutsideJail() { + $file1 = 'foo/foobar'; + $file2 = 'folder/foobar'; + $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; + + $this->sourceCache->put($file1, $data1); + $this->sourceCache->put($file2, $data1); + + $this->assertCount(2, $this->sourceCache->searchByMime('foo/folder')); + + $result = $this->cache->search('%foobar%'); + $this->assertCount(1, $result); + $this->assertEquals('foobar', $result[0]['path']); + } + + public function testSearchQueryOutsideJail() { + $file1 = 'foo/foobar'; + $file2 = 'folder/foobar'; + $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; + + $this->sourceCache->put($file1, $data1); + $this->sourceCache->put($file2, $data1); + + $user = new User('foo', null, $this->createMock(EventDispatcherInterface::class)); + $query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foobar'), 10, 0, [], $user); + $this->assertCount(2, $this->sourceCache->searchQuery($query)); + + $result = $this->cache->search('%foobar%'); + $this->assertCount(1, $result); + $this->assertEquals('foobar', $result[0]['path']); + } + public function testClearKeepEntriesOutsideJail() { $file1 = 'foo/foobar'; $file2 = 'foo/foobar/asd'; From eb56b1d0bd4ce523d65505a6f409b65d92f6c94c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Jan 2021 15:30:27 +0100 Subject: [PATCH 2/5] fix cachjail searching for root Signed-off-by: Robin Appelman --- lib/private/Files/Cache/QuerySearchHelper.php | 3 +++ lib/private/Files/Cache/Wrapper/CacheJail.php | 17 ++++++++++++++--- .../lib/Files/Cache/Wrapper/CacheJailTest.php | 19 +++++++++++++++++-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Cache/QuerySearchHelper.php b/lib/private/Files/Cache/QuerySearchHelper.php index 5e33ad235a..16b15aa6b3 100644 --- a/lib/private/Files/Cache/QuerySearchHelper.php +++ b/lib/private/Files/Cache/QuerySearchHelper.php @@ -166,6 +166,9 @@ class QuerySearchHelper { $field = 'tag.category'; } elseif ($field === 'fileid') { $field = 'file.fileid'; + } elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL) { + $field = 'path_hash'; + $value = md5((string)$value); } return [$field, $value, $type]; } diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 27590fb741..2f873a65cf 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -238,7 +238,10 @@ class CacheJail extends CacheWrapper { $query = $this->getQueryBuilder(); $query->selectFileCache() ->whereStorageId() - ->andWhere($query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%'))) + ->andWhere($query->expr()->orX( + $query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%')), + $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getRoot()))), + )) ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); $result = $query->execute(); @@ -263,7 +266,10 @@ class CacheJail extends CacheWrapper { $query = $this->getQueryBuilder(); $query->selectFileCache() ->whereStorageId() - ->andWhere($query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%'))); + ->andWhere($query->expr()->orX( + $query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%')), + $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getRoot()))), + )); if (strpos($mimetype, '/')) { $query->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT))); @@ -287,9 +293,14 @@ class CacheJail extends CacheWrapper { 'path', $this->getRoot() . '/%' ); + $rootFilter = new SearchComparison( + ISearchComparison::COMPARE_EQUAL, + 'path', + $this->getRoot() + ); $operation = new SearchBinaryOperator( ISearchBinaryOperator::OPERATOR_AND, - [$prefixFilter, $query->getSearchOperation()] + [new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [$prefixFilter, $rootFilter]) , $query->getSearchOperation()] ); $simpleQuery = new SearchQuery($operation, 0, 0, $query->getOrder(), $query->getUser()); $results = $this->getCache()->searchQuery($simpleQuery); diff --git a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php index 83173efd3b..de54333312 100644 --- a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php +++ b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php @@ -37,6 +37,7 @@ class CacheJailTest extends CacheTest { } public function testSearchOutsideJail() { + $this->storage->getScanner()->scan(''); $file1 = 'foo/foobar'; $file2 = 'folder/foobar'; $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; @@ -49,9 +50,18 @@ class CacheJailTest extends CacheTest { $result = $this->cache->search('%foobar%'); $this->assertCount(1, $result); $this->assertEquals('foobar', $result[0]['path']); + + $result = $this->cache->search('%foo%'); + $this->assertCount(2, $result); + usort($result, function ($a, $b) { + return $a['path'] <=> $b['path']; + }); + $this->assertEquals('', $result[0]['path']); + $this->assertEquals('foobar', $result[1]['path']); } public function testSearchMimeOutsideJail() { + $this->storage->getScanner()->scan(''); $file1 = 'foo/foobar'; $file2 = 'folder/foobar'; $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; @@ -67,6 +77,7 @@ class CacheJailTest extends CacheTest { } public function testSearchQueryOutsideJail() { + $this->storage->getScanner()->scan(''); $file1 = 'foo/foobar'; $file2 = 'folder/foobar'; $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; @@ -76,11 +87,15 @@ class CacheJailTest extends CacheTest { $user = new User('foo', null, $this->createMock(EventDispatcherInterface::class)); $query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foobar'), 10, 0, [], $user); - $this->assertCount(2, $this->sourceCache->searchQuery($query)); + $result = $this->cache->searchQuery($query); - $result = $this->cache->search('%foobar%'); $this->assertCount(1, $result); $this->assertEquals('foobar', $result[0]['path']); + + $query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foo'), 10, 0, [], $user); + $result = $this->cache->searchQuery($query); + $this->assertCount(1, $result); + $this->assertEquals('', $result[0]['path']); } public function testClearKeepEntriesOutsideJail() { From ec0985ba68069a6fb2bd3c9454be5d533a124881 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 25 Jan 2021 17:38:34 +0100 Subject: [PATCH 3/5] fix search in nested jails Signed-off-by: Robin Appelman --- apps/files_sharing/lib/Cache.php | 4 ++ apps/files_sharing/tests/CacheTest.php | 36 +++++++++++++ lib/private/Files/Cache/Wrapper/CacheJail.php | 54 ++++++++++++------- lib/private/Share20/Share.php | 5 +- .../lib/Files/Cache/Wrapper/CacheJailTest.php | 18 +++++++ 5 files changed, 96 insertions(+), 21 deletions(-) diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php index c3f31ac3e4..3a6ade5a2a 100644 --- a/apps/files_sharing/lib/Cache.php +++ b/apps/files_sharing/lib/Cache.php @@ -89,6 +89,10 @@ class Cache extends CacheJail { return $this->root; } + protected function getGetUnjailedRoot() { + return $this->sourceRootInfo->getPath(); + } + public function getCache() { if (is_null($this->cache)) { $sourceStorage = $this->storage->getSourceStorage(); diff --git a/apps/files_sharing/tests/CacheTest.php b/apps/files_sharing/tests/CacheTest.php index b55fba78d4..ba9f347e29 100644 --- a/apps/files_sharing/tests/CacheTest.php +++ b/apps/files_sharing/tests/CacheTest.php @@ -517,4 +517,40 @@ class CacheTest extends TestCase { $this->assertTrue($sourceStorage->getCache()->inCache('jail/sub/bar.txt')); } + + public function testSearchShareJailedStorage() { + $sourceStorage = new Temporary(); + $sourceStorage->mkdir('jail'); + $sourceStorage->mkdir('jail/sub'); + $sourceStorage->file_put_contents('jail/sub/foo.txt', 'foo'); + $jailedSource = new Jail([ + 'storage' => $sourceStorage, + 'root' => 'jail' + ]); + $sourceStorage->getScanner()->scan(''); + $this->registerMount(self::TEST_FILES_SHARING_API_USER1, $jailedSource, '/' . self::TEST_FILES_SHARING_API_USER1 . '/files/foo'); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + + $rootFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1); + $node = $rootFolder->get('foo/sub'); + $share = $this->shareManager->newShare(); + $share->setNode($node) + ->setShareType(IShare::TYPE_USER) + ->setSharedWith(self::TEST_FILES_SHARING_API_USER2) + ->setSharedBy(self::TEST_FILES_SHARING_API_USER1) + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + $share = $this->shareManager->createShare($share); + $share->setStatus(IShare::STATUS_ACCEPTED); + $this->shareManager->updateShare($share); + \OC_Util::tearDownFS(); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + + /** @var SharedStorage $sharedStorage */ + list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/sub'); + + $results = $sharedStorage->getCache()->search("foo.txt"); + $this->assertCount(1, $results); + } } diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 2f873a65cf..97700156d0 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -46,6 +46,7 @@ class CacheJail extends CacheWrapper { * @var string */ protected $root; + protected $unjailedRoot; /** * @param \OCP\Files\Cache\ICache $cache @@ -56,12 +57,27 @@ class CacheJail extends CacheWrapper { $this->root = $root; $this->connection = \OC::$server->getDatabaseConnection(); $this->mimetypeLoader = \OC::$server->getMimeTypeLoader(); + + if ($cache instanceof CacheJail) { + $this->unjailedRoot = $cache->getSourcePath($root); + } else { + $this->unjailedRoot = $root; + } } protected function getRoot() { return $this->root; } + /** + * Get the root path with any nested jails resolved + * + * @return string + */ + protected function getGetUnjailedRoot() { + return $this->unjailedRoot; + } + protected function getSourcePath($path) { if ($path === '') { return $this->getRoot(); @@ -72,16 +88,20 @@ class CacheJail extends CacheWrapper { /** * @param string $path + * @param null|string $root * @return null|string the jailed path or null if the path is outside the jail */ - protected function getJailedPath($path) { - if ($this->getRoot() === '') { + protected function getJailedPath(string $path, string $root = null) { + if ($root === null) { + $root = $this->getRoot(); + } + if ($root === '') { return $path; } - $rootLength = strlen($this->getRoot()) + 1; - if ($path === $this->getRoot()) { + $rootLength = strlen($root) + 1; + if ($path === $root) { return ''; - } elseif (substr($path, 0, $rootLength) === $this->getRoot() . '/') { + } elseif (substr($path, 0, $rootLength) === $root . '/') { return substr($path, $rootLength); } else { return null; @@ -99,11 +119,6 @@ class CacheJail extends CacheWrapper { return $entry; } - protected function filterCacheEntry($entry) { - $rootLength = strlen($this->getRoot()) + 1; - return $rootLength === 1 || ($entry['path'] === $this->getRoot()) || (substr($entry['path'], 0, $rootLength) === $this->getRoot() . '/'); - } - /** * get the stored metadata of a file or folder * @@ -216,9 +231,10 @@ class CacheJail extends CacheWrapper { } private function formatSearchResults($results) { - $results = array_filter($results, [$this, 'filterCacheEntry']); - $results = array_values($results); - return array_map([$this, 'formatCacheEntry'], $results); + return array_map(function($entry) { + $entry['path'] = $this->getJailedPath($entry['path'], $this->getGetUnjailedRoot()); + return $entry; + }, $results); } /** @@ -239,8 +255,8 @@ class CacheJail extends CacheWrapper { $query->selectFileCache() ->whereStorageId() ->andWhere($query->expr()->orX( - $query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%')), - $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getRoot()))), + $query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')), + $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))), )) ->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern))); @@ -267,8 +283,8 @@ class CacheJail extends CacheWrapper { $query->selectFileCache() ->whereStorageId() ->andWhere($query->expr()->orX( - $query->expr()->like('path', $query->createNamedParameter($this->getRoot() . '/%')), - $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getRoot()))), + $query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')), + $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))), )); if (strpos($mimetype, '/')) { @@ -291,12 +307,12 @@ class CacheJail extends CacheWrapper { $prefixFilter = new SearchComparison( ISearchComparison::COMPARE_LIKE, 'path', - $this->getRoot() . '/%' + $this->getGetUnjailedRoot() . '/%' ); $rootFilter = new SearchComparison( ISearchComparison::COMPARE_EQUAL, 'path', - $this->getRoot() + $this->getGetUnjailedRoot() ); $operation = new SearchBinaryOperator( ISearchBinaryOperator::OPERATOR_AND, diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 69f36edf4f..9b782d5a44 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -32,6 +32,7 @@ namespace OC\Share20; use OCP\Files\Cache\ICacheEntry; use OCP\Files\File; +use OCP\Files\FileInfo; use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; @@ -233,8 +234,8 @@ class Share implements \OCP\Share\IShare { */ public function getNodeType() { if ($this->nodeType === null) { - $node = $this->getNode(); - $this->nodeType = $node instanceof File ? 'file' : 'folder'; + $info = $this->getNodeCacheEntry(); + $this->nodeType = $info->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file'; } return $this->nodeType; diff --git a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php index de54333312..d9f7af1f03 100644 --- a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php +++ b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php @@ -182,4 +182,22 @@ class CacheJailTest extends CacheTest { $this->assertTrue($this->sourceCache->inCache('target/foo')); $this->assertTrue($this->sourceCache->inCache('target/foo/bar')); } + + public function testSearchNested() { + $this->storage->getScanner()->scan(''); + $file1 = 'foo'; + $file2 = 'foo/bar'; + $file3 = 'foo/bar/asd'; + $data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder']; + + $this->sourceCache->put($file1, $data1); + $this->sourceCache->put($file2, $data1); + $this->sourceCache->put($file3, $data1); + + $nested = new \OC\Files\Cache\Wrapper\CacheJail($this->cache, 'bar'); + + $result = $nested->search('%asd%'); + $this->assertCount(1, $result); + $this->assertEquals('asd', $result[0]['path']); + } } From c7278790738c346e25f4ad201806e4a7a9e81363 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 Jan 2021 15:48:04 +0100 Subject: [PATCH 4/5] adjust tests Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Wrapper/CacheJail.php | 2 +- lib/private/Share20/Share.php | 1 - tests/lib/Share20/LegacyHooksTest.php | 19 ++++++++++++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Cache/Wrapper/CacheJail.php b/lib/private/Files/Cache/Wrapper/CacheJail.php index 97700156d0..fe3124301b 100644 --- a/lib/private/Files/Cache/Wrapper/CacheJail.php +++ b/lib/private/Files/Cache/Wrapper/CacheJail.php @@ -231,7 +231,7 @@ class CacheJail extends CacheWrapper { } private function formatSearchResults($results) { - return array_map(function($entry) { + return array_map(function ($entry) { $entry['path'] = $this->getJailedPath($entry['path'], $this->getGetUnjailedRoot()); return $entry; }, $results); diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 9b782d5a44..a4fbf05141 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -31,7 +31,6 @@ namespace OC\Share20; use OCP\Files\Cache\ICacheEntry; -use OCP\Files\File; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; use OCP\Files\Node; diff --git a/tests/lib/Share20/LegacyHooksTest.php b/tests/lib/Share20/LegacyHooksTest.php index 66dbafe769..a615e26afb 100644 --- a/tests/lib/Share20/LegacyHooksTest.php +++ b/tests/lib/Share20/LegacyHooksTest.php @@ -26,6 +26,7 @@ namespace Test\Share20; use OC\Share20\LegacyHooks; use OC\Share20\Manager; use OCP\Constants; +use OCP\Files\Cache\ICacheEntry; use OCP\Files\File; use OCP\Share\IShare; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -55,6 +56,9 @@ class LegacyHooksTest extends TestCase { $path = $this->createMock(File::class); $path->method('getId')->willReturn(1); + $info = $this->createMock(ICacheEntry::class); + $info->method('getMimeType')->willReturn('text/plain'); + $share = $this->manager->newShare(); $share->setId(42) ->setProviderId('prov') @@ -62,7 +66,8 @@ class LegacyHooksTest extends TestCase { ->setSharedWith('awesomeUser') ->setSharedBy('sharedBy') ->setNode($path) - ->setTarget('myTarget'); + ->setTarget('myTarget') + ->setNodeCacheEntry($info); $hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre'])->getMock(); \OCP\Util::connectHook('OCP\Share', 'pre_unshare', $hookListner, 'pre'); @@ -92,6 +97,9 @@ class LegacyHooksTest extends TestCase { $path = $this->createMock(File::class); $path->method('getId')->willReturn(1); + $info = $this->createMock(ICacheEntry::class); + $info->method('getMimeType')->willReturn('text/plain'); + $share = $this->manager->newShare(); $share->setId(42) ->setProviderId('prov') @@ -99,7 +107,8 @@ class LegacyHooksTest extends TestCase { ->setSharedWith('awesomeUser') ->setSharedBy('sharedBy') ->setNode($path) - ->setTarget('myTarget'); + ->setTarget('myTarget') + ->setNodeCacheEntry($info); $hookListner = $this->getMockBuilder('Dummy')->setMethods(['post'])->getMock(); \OCP\Util::connectHook('OCP\Share', 'post_unshare', $hookListner, 'post'); @@ -143,6 +152,9 @@ class LegacyHooksTest extends TestCase { $path = $this->createMock(File::class); $path->method('getId')->willReturn(1); + $info = $this->createMock(ICacheEntry::class); + $info->method('getMimeType')->willReturn('text/plain'); + $share = $this->manager->newShare(); $share->setId(42) ->setProviderId('prov') @@ -150,7 +162,8 @@ class LegacyHooksTest extends TestCase { ->setSharedWith('awesomeUser') ->setSharedBy('sharedBy') ->setNode($path) - ->setTarget('myTarget'); + ->setTarget('myTarget') + ->setNodeCacheEntry($info); $hookListner = $this->getMockBuilder('Dummy')->setMethods(['postFromSelf'])->getMock(); \OCP\Util::connectHook('OCP\Share', 'post_unshareFromSelf', $hookListner, 'postFromSelf'); From e5ffb96c36f89113fcca8e0f5bdfdb98040186b6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 Jan 2021 16:07:26 +0100 Subject: [PATCH 5/5] only use share cacheentry when available Signed-off-by: Robin Appelman --- lib/private/Share20/Share.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index a4fbf05141..87f77701ee 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -30,6 +30,7 @@ namespace OC\Share20; +use OCP\Files\File; use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; use OCP\Files\IRootFolder; @@ -233,8 +234,13 @@ class Share implements \OCP\Share\IShare { */ public function getNodeType() { if ($this->nodeType === null) { - $info = $this->getNodeCacheEntry(); - $this->nodeType = $info->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file'; + if ($this->getNodeCacheEntry()) { + $info = $this->getNodeCacheEntry(); + $this->nodeType = $info->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file'; + } else { + $node = $this->getNode(); + $this->nodeType = $node instanceof File ? 'file' : 'folder'; + } } return $this->nodeType;