upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-08 08:16:43 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

fix Folder->getById() when a single storage is mounted multiple times

Browse source 
Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
Robin Appelman 2021-07-28 17:03:33 +02:00 committed by backportbot[bot]
parent d53a6dac31
commit 4c954e7dd1
2 changed files with 59 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
lib/private/Files/Node/Folder.php
View file

@ -40,7 +40,6 @@ use OC\Files\Storage\Storage;
use OCA\Files_Sharing\SharedStorage;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\Config\ICachedMountInfo;
use OCP\Files\FileInfo;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\NotFoundException;
@ -372,17 +371,28 @@ class Folder extends Node implements \OCP\Files\Folder {
$user = null;
}
$mountsContainingFile = $mountCache->getMountsForFileId((int)$id, $user);
// when a user has access trough the same storage trough multiple paths
// (such as an external storage that is both mounted for a user and shared to the user)
// the mount cache will only hold a single entry for the storage
// this can lead to issues as the different ways the user has access to a storage can have different permissions
//
// so instead of using the cached entries directly, we instead filter the current mounts by the rootid of the cache entry
$mountRootIds = array_map(function ($mount) {
return $mount->getRootId();
}, $mountsContainingFile);
$mountRootPaths = array_map(function ($mount) {
return $mount->getRootInternalPath();
}, $mountsContainingFile);
$mountRoots = array_combine($mountRootIds, $mountRootPaths);
$mounts = $this->root->getMountsIn($this->path);
$mounts[] = $this->root->getMount($this->path);
/** @var IMountPoint[] $folderMounts */
$folderMounts = array_combine(array_map(function (IMountPoint $mountPoint) {
return $mountPoint->getMountPoint();
}, $mounts), $mounts);
/** @var ICachedMountInfo[] $mountsContainingFile */
$mountsContainingFile = array_values(array_filter($mountsContainingFile, function (ICachedMountInfo $cachedMountInfo) use ($folderMounts) {
return isset($folderMounts[$cachedMountInfo->getMountPoint()]);
}));
$mountsContainingFile = array_filter($mounts, function ($mount) use ($mountRoots) {
return isset($mountRoots[$mount->getStorageRootId()]);
});
if (count($mountsContainingFile) === 0) {
if ($user === $this->getAppDataDirectoryName()) {
@ -391,18 +401,18 @@ class Folder extends Node implements \OCP\Files\Folder {
return [];
}
$nodes = array_map(function (ICachedMountInfo $cachedMountInfo) use ($folderMounts, $id) {
$mount = $folderMounts[$cachedMountInfo->getMountPoint()];
$nodes = array_map(function (IMountPoint $mount) use ($id, $mountRoots) {
$rootInternalPath = $mountRoots[$mount->getStorageRootId()];
$cacheEntry = $mount->getStorage()->getCache()->get((int)$id);
if (!$cacheEntry) {
return null;
}
// cache jails will hide the "true" internal path
$internalPath = ltrim($cachedMountInfo->getRootInternalPath() . '/' . $cacheEntry->getPath(), '/');
$pathRelativeToMount = substr($internalPath, strlen($cachedMountInfo->getRootInternalPath()));
$internalPath = ltrim($rootInternalPath . '/' . $cacheEntry->getPath(), '/');
$pathRelativeToMount = substr($internalPath, strlen($rootInternalPath));
$pathRelativeToMount = ltrim($pathRelativeToMount, '/');
$absolutePath = rtrim($cachedMountInfo->getMountPoint() . $pathRelativeToMount, '/');
$absolutePath = rtrim($mount->getMountPoint() . $pathRelativeToMount, '/');
return $this->root->createNode($absolutePath, new \OC\Files\FileInfo(
$absolutePath, $mount->getStorage(), $cacheEntry->getPath(), $cacheEntry, $mount,
\OC::$server->getUserManager()->get($mount->getStorage()->getOwner($pathRelativeToMount))
@ -411,9 +421,13 @@ class Folder extends Node implements \OCP\Files\Folder {
$nodes = array_filter($nodes);
return array_filter($nodes, function (Node $node) {
$folders = array_filter($nodes, function (Node $node) {
return $this->getRelativePath($node->getPath());
});
usort($folders, function ($a, $b) {
return $b->getPath() <=> $a->getPath();
});
return $folders;
}
protected function getAppDataDirectoryName(): string {

98
tests/lib/Files/Node/FolderTest.php
View file

@ -99,8 +99,7 @@ class FolderTest extends NodeTest {
->method('getUser')
->willReturn($this->user);
$root->expects($this->once())
->method('get')
$root->method('get')
->with('/bar/foo/asd');
$node = new Folder($root, $view, '/bar/foo');
@ -122,8 +121,7 @@ class FolderTest extends NodeTest {
$child = new Folder($root, $view, '/bar/foo/asd');
$root->expects($this->once())
->method('get')
$root->method('get')
->with('/bar/foo/asd')
->willReturn($child);
@ -144,8 +142,7 @@ class FolderTest extends NodeTest {
->method('getUser')
->willReturn($this->user);
$root->expects($this->once())
->method('get')
$root->method('get')
->with('/bar/foo/asd')
->will($this->throwException(new NotFoundException()));
@ -166,13 +163,11 @@ class FolderTest extends NodeTest {
->method('getUser')
->willReturn($this->user);
$view->expects($this->once())
->method('getFileInfo')
$view->method('getFileInfo')
->with('/bar/foo')
->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_ALL]));
$view->expects($this->once())
->method('mkdir')
$view->method('mkdir')
->with('/bar/foo/asd')
->willReturn(true);
@ -194,12 +189,10 @@ class FolderTest extends NodeTest {
$root = $this->getMockBuilder(Root::class)
->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager])
->getMock();
$root->expects($this->any())
->method('getUser')
$root->method('getUser')
->willReturn($this->user);
$view->expects($this->once())
->method('getFileInfo')
$view->method('getFileInfo')
->with('/bar/foo')
->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_READ]));
@ -220,13 +213,11 @@ class FolderTest extends NodeTest {
->method('getUser')
->willReturn($this->user);
$view->expects($this->once())
->method('getFileInfo')
$view->method('getFileInfo')
->with('/bar/foo')
->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_ALL]));
$view->expects($this->once())
->method('touch')
$view->method('touch')
->with('/bar/foo/asd')
->willReturn(true);
@ -248,12 +239,10 @@ class FolderTest extends NodeTest {
$root = $this->getMockBuilder(Root::class)
->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager])
->getMock();
$root->expects($this->any())
->method('getUser')
$root->method('getUser')
->willReturn($this->user);
$view->expects($this->once())
->method('getFileInfo')
$view->method('getFileInfo')
->with('/bar/foo')
->willReturn($this->getFileInfo(['permissions' => \OCP\Constants::PERMISSION_READ]));
@ -270,12 +259,10 @@ class FolderTest extends NodeTest {
$root = $this->getMockBuilder(Root::class)
->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager])
->getMock();
$root->expects($this->any())
->method('getUser')
$root->method('getUser')
->willReturn($this->user);
$view->expects($this->once())
->method('free_space')
$view->method('free_space')
->with('/bar/foo')
->willReturn(100);
@ -502,8 +489,7 @@ class FolderTest extends NodeTest {
$fileInfo = new CacheEntry(['path' => 'foo/qwerty', 'mimetype' => 'text/plain'], null);
$storage->expects($this->once())
->method('getCache')
$storage->method('getCache')
->willReturn($cache);
$this->userMountCache->expects($this->any())
@ -518,18 +504,15 @@ class FolderTest extends NodeTest {
''
)]);
$cache->expects($this->once())
->method('get')
$cache->method('get')
->with(1)
->willReturn($fileInfo);
$root->expects($this->once())
->method('getMountsIn')
$root->method('getMountsIn')
->with('/bar/foo')
->willReturn([]);
$root->expects($this->once())
->method('getMount')
$root->method('getMount')
->with('/bar/foo')
->willReturn($mount);
@ -556,8 +539,7 @@ class FolderTest extends NodeTest {
$fileInfo = new CacheEntry(['path' => '', 'mimetype' => 'text/plain'], null);
$storage->expects($this->once())
->method('getCache')
$storage->method('getCache')
->willReturn($cache);
$this->userMountCache->expects($this->any())
@ -572,13 +554,11 @@ class FolderTest extends NodeTest {
''
)]);
$cache->expects($this->once())
->method('get')
$cache->method('get')
->with(1)
->willReturn($fileInfo);
$root->expects($this->once())
->method('getMount')
$root->method('getMount')
->with('/bar')
->willReturn($mount);
@ -605,8 +585,7 @@ class FolderTest extends NodeTest {
$fileInfo = new CacheEntry(['path' => 'foobar', 'mimetype' => 'text/plain'], null);
$storage->expects($this->once())
->method('getCache')
$storage->method('getCache')
->willReturn($cache);
$this->userMountCache->expects($this->any())
@ -621,18 +600,15 @@ class FolderTest extends NodeTest {
''
)]);
$cache->expects($this->once())
->method('get')
$cache->method('get')
->with(1)
->willReturn($fileInfo);
$root->expects($this->once())
->method('getMountsIn')
$root->method('getMountsIn')
->with('/bar/foo')
->willReturn([]);
$root->expects($this->once())
->method('getMount')
$root->method('getMount')
->with('/bar/foo')
->willReturn($mount);
@ -659,12 +635,10 @@ class FolderTest extends NodeTest {
$fileInfo = new CacheEntry(['path' => 'foo/qwerty', 'mimetype' => 'text/plain'], null);
$storage->expects($this->exactly(2))
->method('getCache')
$storage->method('getCache')
->willReturn($cache);
$this->userMountCache->expects($this->any())
->method('getMountsForFileId')
$this->userMountCache->method('getMountsForFileId')
->with(1)
->willReturn([
new CachedMountInfo(
@ -675,32 +649,20 @@ class FolderTest extends NodeTest {
1,
''
),
new CachedMountInfo(
$this->user,
1,
0,
'/bar/foo/asd/',
1,
''
),
]);
$storage->expects($this->any())
->method('getCache')
$storage->method('getCache')
->willReturn($cache);
$cache->expects($this->any())
->method('get')
$cache->method('get')
->with(1)
->willReturn($fileInfo);
$root->expects($this->any())
->method('getMountsIn')
$root->method('getMountsIn')
->with('/bar/foo')
->willReturn([$mount2]);
$root->expects($this->once())
->method('getMount')
$root->method('getMount')
->with('/bar/foo')
->willReturn($mount1);