summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMorris Jobke <hey@morrisjobke.de>2021-03-16 16:59:29 +0100
committerGitHub <noreply@github.com>2021-03-16 16:59:29 +0100
commit120aefb79505b671ea99f85cc381e05765c3ca15 (patch)
tree13058255f25b800060c71edaf29158f8316aece0
parente09d59aa8184c7ea01e80d61eb2850642f97b53b (diff)
parente5ffb96c36f89113fcca8e0f5bdfdb98040186b6 (diff)
downloadnextcloud-server-120aefb79505b671ea99f85cc381e05765c3ca15.tar.gz
nextcloud-server-120aefb79505b671ea99f85cc381e05765c3ca15.zip
Merge pull request #26133 from nextcloud/backport/25136/stable21
[stable21] do cachejail search filtering in sql
-rw-r--r--apps/files_sharing/lib/Cache.php4
-rw-r--r--apps/files_sharing/tests/CacheTest.php36
-rw-r--r--lib/private/Files/Cache/Cache.php2
-rw-r--r--lib/private/Files/Cache/QuerySearchHelper.php5
-rw-r--r--lib/private/Files/Cache/Wrapper/CacheJail.php113
-rw-r--r--lib/private/Share20/Share.php12
-rw-r--r--tests/lib/Files/Cache/Wrapper/CacheJailTest.php70
-rw-r--r--tests/lib/Share20/LegacyHooksTest.php19
8 files changed, 238 insertions, 23 deletions
diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php
index c3f31ac3e4f..3a6ade5a2ac 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 b55fba78d42..ba9f347e291 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/Cache.php b/lib/private/Files/Cache/Cache.php
index 2513abd525f..840523c1890 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 574b4c18d5d..16b15aa6b3b 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];
}
@@ -175,6 +178,7 @@ class QuerySearchHelper {
'mimetype' => 'string',
'mtime' => 'integer',
'name' => 'string',
+ 'path' => 'string',
'size' => 'integer',
'tagname' => 'string',
'favorite' => 'boolean',
@@ -184,6 +188,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 6c1c17be028..fe3124301ba 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;
/**
@@ -41,6 +46,7 @@ class CacheJail extends CacheWrapper {
* @var string
*/
protected $root;
+ protected $unjailedRoot;
/**
* @param \OCP\Files\Cache\ICache $cache
@@ -49,12 +55,29 @@ 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();
+
+ 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();
@@ -65,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;
@@ -92,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
*
@@ -209,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);
}
/**
@@ -221,7 +244,29 @@ 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()->orX(
+ $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)));
+
+ $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 +277,48 @@ 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()->orX(
+ $query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')),
+ $query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))),
+ ));
+
+ 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->getGetUnjailedRoot() . '/%'
+ );
+ $rootFilter = new SearchComparison(
+ ISearchComparison::COMPARE_EQUAL,
+ 'path',
+ $this->getGetUnjailedRoot()
+ );
+ $operation = new SearchBinaryOperator(
+ ISearchBinaryOperator::OPERATOR_AND,
+ [new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [$prefixFilter, $rootFilter]) , $query->getSearchOperation()]
+ );
+ $simpleQuery = new SearchQuery($operation, 0, 0, $query->getOrder(), $query->getUser());
$results = $this->getCache()->searchQuery($simpleQuery);
$results = $this->formatSearchResults($results);
diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php
index 69f36edf4f3..87f77701ee2 100644
--- a/lib/private/Share20/Share.php
+++ b/lib/private/Share20/Share.php
@@ -30,8 +30,9 @@
namespace OC\Share20;
-use OCP\Files\Cache\ICacheEntry;
use OCP\Files\File;
+use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\FileInfo;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
@@ -233,8 +234,13 @@ class Share implements \OCP\Share\IShare {
*/
public function getNodeType() {
if ($this->nodeType === null) {
- $node = $this->getNode();
- $this->nodeType = $node instanceof File ? 'file' : 'folder';
+ 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;
diff --git a/tests/lib/Files/Cache/Wrapper/CacheJailTest.php b/tests/lib/Files/Cache/Wrapper/CacheJailTest.php
index f0943ba5a03..d9f7af1f034 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;
/**
@@ -32,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'];
@@ -44,6 +50,52 @@ 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'];
+
+ $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() {
+ $this->storage->getScanner()->scan('');
+ $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);
+ $result = $this->cache->searchQuery($query);
+
+ $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() {
@@ -130,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']);
+ }
}
diff --git a/tests/lib/Share20/LegacyHooksTest.php b/tests/lib/Share20/LegacyHooksTest.php
index 66dbafe7691..a615e26afb0 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');