Merge pull request #25136 from nextcloud/cachejail-search-filter

do cachejail search filtering in sql
nextcloud · Mar 15, 2021 · e559afb · e559afb
2 parents f512705 + 6ecf33b
commit e559afb
Show file tree

Hide file tree

Showing 8 changed files with 238 additions and 23 deletions.
diff --git a/apps/files_sharing/lib/Cache.php b/apps/files_sharing/lib/Cache.php
@@ -89,6 +89,10 @@ protected function getRoot() {
 		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
@@ -517,4 +517,40 @@ public function testShareJailedStorage() {
 
 		$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
@@ -121,7 +121,7 @@ public function __construct(IStorage $storage) {
 		$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
@@ -166,6 +166,9 @@ private function getOperatorFieldAndValue(ISearchComparison $operator) {
 			$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 @@ private function validateComparison(ISearchComparison $operator) {
 			'mimetype' => 'string',
 			'mtime' => 'integer',
 			'name' => 'string',
+			'path' => 'string',
 			'size' => 'integer',
 			'tagname' => 'string',
 			'favorite' => 'boolean',
@@ -184,6 +188,7 @@ private function validateComparison(ISearchComparison $operator) {
 			'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
@@ -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 @@ protected function getSourcePath($path) {
 
 	/**
 	 * @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 @@ protected function formatCacheEntry($entry) {
 		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 @@ public function getStatus($file) {
 	}
 
 	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 @@ private function formatSearchResults($results) {
 	 * @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 @@ public function search($pattern) {
 	 * @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
@@ -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 @@ public function setNodeType($type) {
 	 */
 	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
@@ -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 @@ protected function setUp(): void {
 	}
 
 	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 @@ public function testSearchOutsideJail() {
 		$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 @@ public function testMoveBetweenJail() {
 		$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']);
+	}
 }