upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-28 04:32:30 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 25

perf: Perform share path validation early

Browse source 
Signed-off-by: Carl Schwan <carlschwan@kde.org>
This commit is contained in:
Carl Schwan 2026-02-26 10:36:10 +01:00
parent 3e8f02308a
commit c3a15af538
No known key found for this signature in database
GPG key ID: 02325448204E452A
2 changed files with 83 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
lib/private/Share20/Manager.php
View file

@ -14,6 +14,7 @@ use OC\Share20\Exception\ProviderException;
use OCA\Files_Sharing\AppInfo\Application;
use OCA\Files_Sharing\SharedStorage;
use OCA\ShareByMail\ShareByMailProvider;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\File;
@ -1072,7 +1073,7 @@ class Manager implements IManager {
// Figure out which users has some shares with which providers
$qb = $this->connection->getQueryBuilder();
$qb->select('uid_initiator', 'share_type')
$qb->select('uid_initiator', 'share_type', 'uid_owner', 'file_source')
->from('share')
->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter($shareTypes, IQueryBuilder::PARAM_INT_ARRAY)))
@ -1094,47 +1095,52 @@ class Manager implements IManager {
$qb->orderBy('id');
$cursor = $qb->executeQuery();
/** @var array<string, list<array{IShare::TYPE_*, Node}>> $rawShare */
$rawShares = [];
while ($data = $cursor->fetch()) {
if (!isset($rawShares[$data['uid_initiator']])) {
$rawShares[$data['uid_initiator']] = [];
}
if (!in_array($data['share_type'], $rawShares[$data['uid_initiator']], true)) {
$rawShares[$data['uid_initiator']][] = $data['share_type'];
if ($node instanceof Folder) {
if ($data['file_source'] === null || $data['uid_owner'] === null) {
/* Ignore share of non-existing node */
continue;
}
// for federated shares the owner can be a remote user, in this
// case we use the initiator
if ($this->userManager->userExists($data['uid_owner'])) {
$userFolder = $this->rootFolder->getUserFolder($data['uid_owner']);
} else {
$userFolder = $this->rootFolder->getUserFolder($data['uid_initiator']);
}
$sharedNode = $userFolder->getFirstNodeById((int)$data['file_source']);
if (!$sharedNode) {
continue;
}
if ($node->getRelativePath($sharedNode->getPath()) !== null) {
$rawShares[$data['uid_initiator']][] = [(int)$data['share_type'], $sharedNode];
}
} elseif ($node instanceof File) {
$rawShares[$data['uid_initiator']][] = [(int)$data['share_type'], $node];
}
}
}
$cursor->closeCursor();
foreach ($rawShares as $userId => $shareTypes) {
foreach ($shareTypes as $shareType) {
foreach ($rawShares as $userId => $shareInfos) {
foreach ($shareInfos as $shareInfo) {
[$shareType, $sharedNode] = $shareInfo;
try {
$provider = $this->factory->getProviderForType($shareType);
} catch (ProviderException) {
continue;
}
if ($node instanceof Folder) {
/* We need to get all shares by this user to get subshares */
$shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0);
foreach ($shares as $share) {
try {
$path = $share->getNode()->getPath();
} catch (NotFoundException) {
/* Ignore share of non-existing node */
continue;
}
if ($node->getRelativePath($path) !== null) {
/* If relative path is not null it means the shared node is the same or in a subfolder */
$reshareRecords[] = $share;
}
}
} else {
$shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0);
foreach ($shares as $child) {
$reshareRecords[] = $child;
}
$shares = $provider->getSharesBy($userId, $shareType, $sharedNode, false, -1, 0);
foreach ($shares as $child) {
$reshareRecords[] = $child;
}
}
}

62
tests/lib/Share20/ManagerTest.php
View file

@ -499,6 +499,7 @@ class ManagerTest extends \Test\TestCase {
->getMock();
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(42);
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn(IShare::TYPE_USER);
@ -527,6 +528,13 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->exactly(1))->method('updateShare')->with($reShare);
$this->userManager->method('userExists')->willReturn(true);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder);
$userFolder->method('getFirstNodeById')
->with(42)
->willReturn($file);
$qb = $this->createMock(IQueryBuilder::class);
$result = $this->createMock(IResult::class);
$qb->method('select')
@ -543,7 +551,7 @@ class ManagerTest extends \Test\TestCase {
->willReturn($qb);
$result->method('fetch')
->willReturnOnConsecutiveCalls(
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42],
false,
);
@ -584,14 +592,18 @@ class ManagerTest extends \Test\TestCase {
$reShareInOtherFolder->method('getNode')->willReturn($otherFolder);
$this->defaultProvider->method('getSharesBy')
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $reShareInSubFolder, $reShareInOtherFolder) {
->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($folder, $subFolder, $reShare, $reShareInSubFolder, $reShareInOtherFolder) {
if ($shareType === IShare::TYPE_USER) {
return match($userId) {
'userB' => [$reShare,$reShareInSubFolder,$reShareInOtherFolder],
};
} else {
return [];
if ($userId === 'userB') {
if ($node === $folder) {
return [$reShare];
}
if ($node === $subFolder) {
return [$reShareInSubFolder];
}
}
}
$this->fail();
});
$manager->method('generalCreateChecks')->willThrowException(new GenericShareException());
@ -606,6 +618,18 @@ class ManagerTest extends \Test\TestCase {
$this->assertEquals($expected, $share);
});
$this->userManager->method('userExists')->willReturn(true);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder);
$userFolder->method('getFirstNodeById')
->willReturnCallback(function ($id) use ($subFolder, $otherFolder, $folder) {
return match ($id) {
41 => $subFolder,
42 => $otherFolder,
43 => $folder,
};
});
$qb = $this->createMock(IQueryBuilder::class);
$result = $this->createMock(IResult::class);
$qb->method('select')
@ -622,7 +646,9 @@ class ManagerTest extends \Test\TestCase {
->willReturn($qb);
$result->method('fetch')
->willReturnOnConsecutiveCalls(
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 41],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 43],
false,
);
@ -654,6 +680,13 @@ class ManagerTest extends \Test\TestCase {
/* No share is promoted because generalCreateChecks does not throw */
$manager->expects($this->never())->method('updateShare');
$this->userManager->method('userExists')->willReturn(true);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder);
$userFolder->method('getFirstNodeById')
->with(42)
->willReturn($folder);
$qb = $this->createMock(IQueryBuilder::class);
$result = $this->createMock(IResult::class);
$qb->method('select')
@ -670,7 +703,7 @@ class ManagerTest extends \Test\TestCase {
->willReturn($qb);
$result->method('fetch')
->willReturnOnConsecutiveCalls(
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42],
false,
);
@ -729,6 +762,13 @@ class ManagerTest extends \Test\TestCase {
$manager->method('getSharedWith')->willReturn([]);
$this->userManager->method('userExists')->willReturn(true);
$userFolder = $this->createMock(Folder::class);
$this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder);
$userFolder->method('getFirstNodeById')
->with(42)
->willReturn($folder);
$calls = [
$reShare1,
$reShare2,
@ -756,8 +796,8 @@ class ManagerTest extends \Test\TestCase {
->willReturn($qb);
$result->method('fetch')
->willReturnOnConsecutiveCalls(
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER],
['uid_initiator' => 'userC', 'share_type' => IShare::TYPE_USER],
['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42],
['uid_initiator' => 'userC', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42],
false,
);