Skip to content

Commit 120aefb

Browse files
MorrisJobkeMorrisJobke
authored
Merge pull request #26133 from nextcloud/backport/25136/stable21
[stable21] do cachejail search filtering in sql
2 parents e09d59a + e5ffb96 commit 120aefb

File tree

8 files changed

+238
-23
lines changed

8 files changed

+238
-23
lines changed

‎apps/files_sharing/lib/Cache.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ protected function getRoot() {
8989
return $this->root;
9090
}
9191

92+
protected function getGetUnjailedRoot() {
93+
return $this->sourceRootInfo->getPath();
94+
}
95+
9296
public function getCache() {
9397
if (is_null($this->cache)) {
9498
$sourceStorage = $this->storage->getSourceStorage();

‎apps/files_sharing/tests/CacheTest.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,4 +517,40 @@ public function testShareJailedStorage() {
517517

518518
$this->assertTrue($sourceStorage->getCache()->inCache('jail/sub/bar.txt'));
519519
}
520+
521+
public function testSearchShareJailedStorage() {
522+
$sourceStorage = new Temporary();
523+
$sourceStorage->mkdir('jail');
524+
$sourceStorage->mkdir('jail/sub');
525+
$sourceStorage->file_put_contents('jail/sub/foo.txt', 'foo');
526+
$jailedSource = new Jail([
527+
'storage' => $sourceStorage,
528+
'root' => 'jail'
529+
]);
530+
$sourceStorage->getScanner()->scan('');
531+
$this->registerMount(self::TEST_FILES_SHARING_API_USER1, $jailedSource, '/' . self::TEST_FILES_SHARING_API_USER1 . '/files/foo');
532+
533+
self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
534+
535+
$rootFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1);
536+
$node = $rootFolder->get('foo/sub');
537+
$share = $this->shareManager->newShare();
538+
$share->setNode($node)
539+
->setShareType(IShare::TYPE_USER)
540+
->setSharedWith(self::TEST_FILES_SHARING_API_USER2)
541+
->setSharedBy(self::TEST_FILES_SHARING_API_USER1)
542+
->setPermissions(\OCP\Constants::PERMISSION_ALL);
543+
$share = $this->shareManager->createShare($share);
544+
$share->setStatus(IShare::STATUS_ACCEPTED);
545+
$this->shareManager->updateShare($share);
546+
\OC_Util::tearDownFS();
547+
548+
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
549+
550+
/** @var SharedStorage $sharedStorage */
551+
list($sharedStorage) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/sub');
552+
553+
$results = $sharedStorage->getCache()->search("foo.txt");
554+
$this->assertCount(1, $results);
555+
}
520556
}

‎lib/private/Files/Cache/Cache.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public function __construct(IStorage $storage) {
121121
$this->querySearchHelper = new QuerySearchHelper($this->mimetypeLoader);
122122
}
123123

124-
private function getQueryBuilder() {
124+
protected function getQueryBuilder() {
125125
return new CacheQueryBuilder(
126126
$this->connection,
127127
\OC::$server->getSystemConfig(),

‎lib/private/Files/Cache/QuerySearchHelper.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ private function getOperatorFieldAndValue(ISearchComparison $operator) {
166166
$field = 'tag.category';
167167
} elseif ($field === 'fileid') {
168168
$field = 'file.fileid';
169+
} elseif ($field === 'path' && $type === ISearchComparison::COMPARE_EQUAL) {
170+
$field = 'path_hash';
171+
$value = md5((string)$value);
169172
}
170173
return [$field, $value, $type];
171174
}
@@ -175,6 +178,7 @@ private function validateComparison(ISearchComparison $operator) {
175178
'mimetype' => 'string',
176179
'mtime' => 'integer',
177180
'name' => 'string',
181+
'path' => 'string',
178182
'size' => 'integer',
179183
'tagname' => 'string',
180184
'favorite' => 'boolean',
@@ -184,6 +188,7 @@ private function validateComparison(ISearchComparison $operator) {
184188
'mimetype' => ['eq', 'like'],
185189
'mtime' => ['eq', 'gt', 'lt', 'gte', 'lte'],
186190
'name' => ['eq', 'like'],
191+
'path' => ['eq', 'like'],
187192
'size' => ['eq', 'gt', 'lt', 'gte', 'lte'],
188193
'tagname' => ['eq', 'like'],
189194
'favorite' => ['eq'],

‎lib/private/Files/Cache/Wrapper/CacheJail.php‎

Lines changed: 97 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,13 @@
2929
namespace OC\Files\Cache\Wrapper;
3030

3131
use OC\Files\Cache\Cache;
32+
use OC\Files\Search\SearchBinaryOperator;
33+
use OC\Files\Search\SearchComparison;
3234
use OC\Files\Search\SearchQuery;
35+
use OCP\DB\QueryBuilder\IQueryBuilder;
3336
use OCP\Files\Cache\ICacheEntry;
37+
use OCP\Files\Search\ISearchBinaryOperator;
38+
use OCP\Files\Search\ISearchComparison;
3439
use OCP\Files\Search\ISearchQuery;
3540

3641
/**
@@ -41,6 +46,7 @@ class CacheJail extends CacheWrapper {
4146
* @var string
4247
*/
4348
protected $root;
49+
protected $unjailedRoot;
4450

4551
/**
4652
* @param \OCP\Files\Cache\ICache $cache
@@ -49,12 +55,29 @@ class CacheJail extends CacheWrapper {
4955
public function __construct($cache, $root) {
5056
parent::__construct($cache);
5157
$this->root = $root;
58+
$this->connection = \OC::$server->getDatabaseConnection();
59+
$this->mimetypeLoader = \OC::$server->getMimeTypeLoader();
60+
61+
if ($cache instanceof CacheJail) {
62+
$this->unjailedRoot = $cache->getSourcePath($root);
63+
} else {
64+
$this->unjailedRoot = $root;
65+
}
5266
}
5367

5468
protected function getRoot() {
5569
return $this->root;
5670
}
5771

72+
/**
73+
* Get the root path with any nested jails resolved
74+
*
75+
* @return string
76+
*/
77+
protected function getGetUnjailedRoot() {
78+
return $this->unjailedRoot;
79+
}
80+
5881
protected function getSourcePath($path) {
5982
if ($path === '') {
6083
return $this->getRoot();
@@ -65,16 +88,20 @@ protected function getSourcePath($path) {
6588

6689
/**
6790
* @param string $path
91+
* @param null|string $root
6892
* @return null|string the jailed path or null if the path is outside the jail
6993
*/
70-
protected function getJailedPath($path) {
71-
if ($this->getRoot() === '') {
94+
protected function getJailedPath(string $path, string $root = null) {
95+
if ($root === null) {
96+
$root = $this->getRoot();
97+
}
98+
if ($root === '') {
7299
return $path;
73100
}
74-
$rootLength = strlen($this->getRoot()) + 1;
75-
if ($path === $this->getRoot()) {
101+
$rootLength = strlen($root) + 1;
102+
if ($path === $root) {
76103
return '';
77-
} elseif (substr($path, 0, $rootLength) === $this->getRoot() . '/') {
104+
} elseif (substr($path, 0, $rootLength) === $root . '/') {
78105
return substr($path, $rootLength);
79106
} else {
80107
return null;
@@ -92,11 +119,6 @@ protected function formatCacheEntry($entry) {
92119
return $entry;
93120
}
94121

95-
protected function filterCacheEntry($entry) {
96-
$rootLength = strlen($this->getRoot()) + 1;
97-
return $rootLength === 1 || ($entry['path'] === $this->getRoot()) || (substr($entry['path'], 0, $rootLength) === $this->getRoot() . '/');
98-
}
99-
100122
/**
101123
* get the stored metadata of a file or folder
102124
*
@@ -209,9 +231,10 @@ public function getStatus($file) {
209231
}
210232

211233
private function formatSearchResults($results) {
212-
$results = array_filter($results, [$this, 'filterCacheEntry']);
213-
$results = array_values($results);
214-
return array_map([$this, 'formatCacheEntry'], $results);
234+
return array_map(function ($entry) {
235+
$entry['path'] = $this->getJailedPath($entry['path'], $this->getGetUnjailedRoot());
236+
return $entry;
237+
}, $results);
215238
}
216239

217240
/**
@@ -221,7 +244,29 @@ private function formatSearchResults($results) {
221244
* @return array an array of file data
222245
*/
223246
public function search($pattern) {
224-
$results = $this->getCache()->search($pattern);
247+
// normalize pattern
248+
$pattern = $this->normalize($pattern);
249+
250+
if ($pattern === '%%') {
251+
return [];
252+
}
253+
254+
$query = $this->getQueryBuilder();
255+
$query->selectFileCache()
256+
->whereStorageId()
257+
->andWhere($query->expr()->orX(
258+
$query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')),
259+
$query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))),
260+
))
261+
->andWhere($query->expr()->iLike('name', $query->createNamedParameter($pattern)));
262+
263+
$result = $query->execute();
264+
$files = $result->fetchAll();
265+
$result->closeCursor();
266+
267+
$results = array_map(function (array $data) {
268+
return self::cacheEntryFromData($data, $this->mimetypeLoader);
269+
}, $files);
225270
return $this->formatSearchResults($results);
226271
}
227272

@@ -232,12 +277,48 @@ public function search($pattern) {
232277
* @return array
233278
*/
234279
public function searchByMime($mimetype) {
235-
$results = $this->getCache()->searchByMime($mimetype);
280+
$mimeId = $this->mimetypeLoader->getId($mimetype);
281+
282+
$query = $this->getQueryBuilder();
283+
$query->selectFileCache()
284+
->whereStorageId()
285+
->andWhere($query->expr()->orX(
286+
$query->expr()->like('path', $query->createNamedParameter($this->getGetUnjailedRoot() . '/%')),
287+
$query->expr()->eq('path_hash', $query->createNamedParameter(md5($this->getGetUnjailedRoot()))),
288+
));
289+
290+
if (strpos($mimetype, '/')) {
291+
$query->andWhere($query->expr()->eq('mimetype', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT)));
292+
} else {
293+
$query->andWhere($query->expr()->eq('mimepart', $query->createNamedParameter($mimeId, IQueryBuilder::PARAM_INT)));
294+
}
295+
296+
$result = $query->execute();
297+
$files = $result->fetchAll();
298+
$result->closeCursor();
299+
300+
$results = array_map(function (array $data) {
301+
return self::cacheEntryFromData($data, $this->mimetypeLoader);
302+
}, $files);
236303
return $this->formatSearchResults($results);
237304
}
238305

239306
public function searchQuery(ISearchQuery $query) {
240-
$simpleQuery = new SearchQuery($query->getSearchOperation(), 0, 0, $query->getOrder(), $query->getUser());
307+
$prefixFilter = new SearchComparison(
308+
ISearchComparison::COMPARE_LIKE,
309+
'path',
310+
$this->getGetUnjailedRoot() . '/%'
311+
);
312+
$rootFilter = new SearchComparison(
313+
ISearchComparison::COMPARE_EQUAL,
314+
'path',
315+
$this->getGetUnjailedRoot()
316+
);
317+
$operation = new SearchBinaryOperator(
318+
ISearchBinaryOperator::OPERATOR_AND,
319+
[new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_OR, [$prefixFilter, $rootFilter]) , $query->getSearchOperation()]
320+
);
321+
$simpleQuery = new SearchQuery($operation, 0, 0, $query->getOrder(), $query->getUser());
241322
$results = $this->getCache()->searchQuery($simpleQuery);
242323
$results = $this->formatSearchResults($results);
243324

‎lib/private/Share20/Share.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030

3131
namespace OC\Share20;
3232

33-
use OCP\Files\Cache\ICacheEntry;
3433
use OCP\Files\File;
34+
use OCP\Files\Cache\ICacheEntry;
35+
use OCP\Files\FileInfo;
3536
use OCP\Files\IRootFolder;
3637
use OCP\Files\Node;
3738
use OCP\Files\NotFoundException;
@@ -233,8 +234,13 @@ public function setNodeType($type) {
233234
*/
234235
public function getNodeType() {
235236
if ($this->nodeType === null) {
236-
$node = $this->getNode();
237-
$this->nodeType = $node instanceof File ? 'file' : 'folder';
237+
if ($this->getNodeCacheEntry()) {
238+
$info = $this->getNodeCacheEntry();
239+
$this->nodeType = $info->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file';
240+
} else {
241+
$node = $this->getNode();
242+
$this->nodeType = $node instanceof File ? 'file' : 'folder';
243+
}
238244
}
239245

240246
return $this->nodeType;

‎tests/lib/Files/Cache/Wrapper/CacheJailTest.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
namespace Test\Files\Cache\Wrapper;
1010

1111
use OC\Files\Cache\Wrapper\CacheJail;
12+
use OC\Files\Search\SearchComparison;
13+
use OC\Files\Search\SearchQuery;
14+
use OC\User\User;
15+
use OCP\Files\Search\ISearchComparison;
16+
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
1217
use Test\Files\Cache\CacheTest;
1318

1419
/**
@@ -32,6 +37,7 @@ protected function setUp(): void {
3237
}
3338

3439
public function testSearchOutsideJail() {
40+
$this->storage->getScanner()->scan('');
3541
$file1 = 'foo/foobar';
3642
$file2 = 'folder/foobar';
3743
$data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder'];
@@ -44,6 +50,52 @@ public function testSearchOutsideJail() {
4450
$result = $this->cache->search('%foobar%');
4551
$this->assertCount(1, $result);
4652
$this->assertEquals('foobar', $result[0]['path']);
53+
54+
$result = $this->cache->search('%foo%');
55+
$this->assertCount(2, $result);
56+
usort($result, function ($a, $b) {
57+
return $a['path'] <=> $b['path'];
58+
});
59+
$this->assertEquals('', $result[0]['path']);
60+
$this->assertEquals('foobar', $result[1]['path']);
61+
}
62+
63+
public function testSearchMimeOutsideJail() {
64+
$this->storage->getScanner()->scan('');
65+
$file1 = 'foo/foobar';
66+
$file2 = 'folder/foobar';
67+
$data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder'];
68+
69+
$this->sourceCache->put($file1, $data1);
70+
$this->sourceCache->put($file2, $data1);
71+
72+
$this->assertCount(2, $this->sourceCache->searchByMime('foo/folder'));
73+
74+
$result = $this->cache->search('%foobar%');
75+
$this->assertCount(1, $result);
76+
$this->assertEquals('foobar', $result[0]['path']);
77+
}
78+
79+
public function testSearchQueryOutsideJail() {
80+
$this->storage->getScanner()->scan('');
81+
$file1 = 'foo/foobar';
82+
$file2 = 'folder/foobar';
83+
$data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder'];
84+
85+
$this->sourceCache->put($file1, $data1);
86+
$this->sourceCache->put($file2, $data1);
87+
88+
$user = new User('foo', null, $this->createMock(EventDispatcherInterface::class));
89+
$query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foobar'), 10, 0, [], $user);
90+
$result = $this->cache->searchQuery($query);
91+
92+
$this->assertCount(1, $result);
93+
$this->assertEquals('foobar', $result[0]['path']);
94+
95+
$query = new SearchQuery(new SearchComparison(ISearchComparison::COMPARE_EQUAL, 'name', 'foo'), 10, 0, [], $user);
96+
$result = $this->cache->searchQuery($query);
97+
$this->assertCount(1, $result);
98+
$this->assertEquals('', $result[0]['path']);
4799
}
48100

49101
public function testClearKeepEntriesOutsideJail() {
@@ -130,4 +182,22 @@ public function testMoveBetweenJail() {
130182
$this->assertTrue($this->sourceCache->inCache('target/foo'));
131183
$this->assertTrue($this->sourceCache->inCache('target/foo/bar'));
132184
}
185+
186+
public function testSearchNested() {
187+
$this->storage->getScanner()->scan('');
188+
$file1 = 'foo';
189+
$file2 = 'foo/bar';
190+
$file3 = 'foo/bar/asd';
191+
$data1 = ['size' => 100, 'mtime' => 50, 'mimetype' => 'foo/folder'];
192+
193+
$this->sourceCache->put($file1, $data1);
194+
$this->sourceCache->put($file2, $data1);
195+
$this->sourceCache->put($file3, $data1);
196+
197+
$nested = new \OC\Files\Cache\Wrapper\CacheJail($this->cache, 'bar');
198+
199+
$result = $nested->search('%asd%');
200+
$this->assertCount(1, $result);
201+
$this->assertEquals('asd', $result[0]['path']);
202+
}
133203
}

0 commit comments

Comments
 (0)