From bd268c35f523dc861f315fc476f2416eaa605b5e Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 22 Aug 2014 15:59:44 +0200 Subject: [PATCH] generateTarget() will always find a unique target --- lib/private/share/helper.php | 117 ++++++++++++----------------------- lib/private/share/share.php | 59 ++++++++++++------ 2 files changed, 80 insertions(+), 96 deletions(-) diff --git a/lib/private/share/helper.php b/lib/private/share/helper.php index 46e3c28048..0c679b2bda 100644 --- a/lib/private/share/helper.php +++ b/lib/private/share/helper.php @@ -35,8 +35,7 @@ class Helper extends \OC\Share\Constants { * @throws \Exception * @return string Item target */ - public static function generateTarget($itemType, $itemSource, $shareType, $shareWith, $uidOwner, - $suggestedTarget = null, $groupParent = null) { + public static function generateTarget($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $suggestedTarget = null, $groupParent = null) { $backend = \OC\Share\Share::getBackend($itemType); if ($shareType == self::SHARE_TYPE_LINK) { if (isset($suggestedTarget)) { @@ -58,87 +57,51 @@ class Helper extends \OC\Share\Constants { } else { $userAndGroups = false; } - $exclude = null; - // Backend has 3 opportunities to generate a unique target - for ($i = 0; $i < 2; $i++) { - // Check if suggested target exists first - if ($i == 0 && isset($suggestedTarget)) { - $target = $suggestedTarget; + $exclude = array(); + // Find similar targets to improve backend's chances to generate a unqiue target + if ($userAndGroups) { + if ($column == 'file_target') { + $checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`' + . ' WHERE `item_type` IN (\'file\', \'folder\')' + . ' AND `share_type` IN (?,?,?)' + . ' AND `share_with` IN (\'' . implode('\',\'', $userAndGroups) . '\')'); + $result = $checkTargets->execute(array(self::SHARE_TYPE_USER, self::SHARE_TYPE_GROUP, + self::$shareTypeGroupUserUnique)); } else { - if ($shareType == self::SHARE_TYPE_GROUP) { - $target = $backend->generateTarget($itemSource, false, $exclude); - } else { - $target = $backend->generateTarget($itemSource, $shareWith, $exclude); - } - if (is_array($exclude) && in_array($target, $exclude)) { - break; - } + $checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`' + . ' WHERE `item_type` = ? AND `share_type` IN (?,?,?)' + . ' AND `share_with` IN (\'' . implode('\',\'', $userAndGroups) . '\')'); + $result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_USER, + self::SHARE_TYPE_GROUP, self::$shareTypeGroupUserUnique)); } - // Check if target already exists - $checkTarget = \OC\Share\Share::getItems($itemType, $target, $shareType, $shareWith); - if (!empty($checkTarget)) { - foreach ($checkTarget as $item) { - // Skip item if it is the group parent row - if (isset($groupParent) && $item['id'] == $groupParent) { - if (count($checkTarget) == 1) { - return $target; - } else { - continue; - } - } - if ($item['uid_owner'] == $uidOwner) { - if ($itemType == 'file' || $itemType == 'folder') { - $meta = \OC\Files\Filesystem::getFileInfo($itemSource); - if ($item['file_source'] == $meta['fileid']) { - return $target; - } - } else if ($item['item_source'] == $itemSource) { - return $target; - } - } - } - if (!isset($exclude)) { - $exclude = array(); - } - // Find similar targets to improve backend's chances to generate a unqiue target - if ($userAndGroups) { - if ($column == 'file_target') { - $checkTargets = \OC_DB::prepare('SELECT `'.$column.'` FROM `*PREFIX*share`' - .' WHERE `item_type` IN (\'file\', \'folder\')' - .' AND `share_type` IN (?,?,?)' - .' AND `share_with` IN (\''.implode('\',\'', $userAndGroups).'\')'); - $result = $checkTargets->execute(array(self::SHARE_TYPE_USER, self::SHARE_TYPE_GROUP, - self::$shareTypeGroupUserUnique)); - } else { - $checkTargets = \OC_DB::prepare('SELECT `'.$column.'` FROM `*PREFIX*share`' - .' WHERE `item_type` = ? AND `share_type` IN (?,?,?)' - .' AND `share_with` IN (\''.implode('\',\'', $userAndGroups).'\')'); - $result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_USER, - self::SHARE_TYPE_GROUP, self::$shareTypeGroupUserUnique)); - } - } else { - if ($column == 'file_target') { - $checkTargets = \OC_DB::prepare('SELECT `'.$column.'` FROM `*PREFIX*share`' - .' WHERE `item_type` IN (\'file\', \'folder\')' - .' AND `share_type` = ? AND `share_with` = ?'); - $result = $checkTargets->execute(array(self::SHARE_TYPE_GROUP, $shareWith)); - } else { - $checkTargets = \OC_DB::prepare('SELECT `'.$column.'` FROM `*PREFIX*share`' - .' WHERE `item_type` = ? AND `share_type` = ? AND `share_with` = ?'); - $result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_GROUP, $shareWith)); - } - } - while ($row = $result->fetchRow()) { - $exclude[] = $row[$column]; - } + } else { + if ($column == 'file_target') { + $checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`' + . ' WHERE `item_type` IN (\'file\', \'folder\')' + . ' AND `share_type` = ? AND `share_with` = ?'); + $result = $checkTargets->execute(array(self::SHARE_TYPE_GROUP, $shareWith)); } else { - return $target; + $checkTargets = \OC_DB::prepare('SELECT `' . $column . '` FROM `*PREFIX*share`' + . ' WHERE `item_type` = ? AND `share_type` = ? AND `share_with` = ?'); + $result = $checkTargets->execute(array($itemType, self::SHARE_TYPE_GROUP, $shareWith)); } } + while ($row = $result->fetchRow()) { + $exclude[] = $row[$column]; + } + + // Check if suggested target exists first + if (!isset($suggestedTarget)) { + $suggestedTarget = $itemSource; + } + if ($shareType == self::SHARE_TYPE_GROUP) { + $target = $backend->generateTarget($suggestedTarget, false, $exclude); + } else { + $target = $backend->generateTarget($suggestedTarget, $shareWith, $exclude); + } + + return $target; } - $message = 'Sharing backend registered for '.$itemType.' did not generate a unique target for '.$itemSource; - \OC_Log::write('OCP\Share', $message, \OC_Log::ERROR); - throw new \Exception($message); } /** diff --git a/lib/private/share/share.php b/lib/private/share/share.php index 08d9e7955b..c98373455d 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -296,7 +296,7 @@ class Share extends \OC\Share\Constants { // first check if there is a db entry for the specific user $query = \OC_DB::prepare( - 'SELECT `file_target`, `permissions`, `expiration` + 'SELECT `file_target`, `item_target`, `permissions`, `expiration` FROM `*PREFIX*share` WHERE @@ -1294,13 +1294,17 @@ class Share extends \OC\Share\Constants { $queryArgs = array_merge($queryArgs, $collectionTypes); } } + + if ($shareType == self::$shareTypeUserAndGroups) { + // Make sure the unique user target is returned if it exists, + // unique targets should follow the group share in the database + // If the limit is not 1, the filtering can be done later + $where .= ' ORDER BY `*PREFIX*share`.`id` DESC'; + } else { + $where .= ' ORDER BY `*PREFIX*share`.`id` ASC'; + } + if ($limit != -1 && !$includeCollections) { - if ($shareType == self::$shareTypeUserAndGroups) { - // Make sure the unique user target is returned if it exists, - // unique targets should follow the group share in the database - // If the limit is not 1, the filtering can be done later - $where .= ' ORDER BY `*PREFIX*share`.`id` ASC'; - } // The limit must be at least 3, because filtering needs to be done if ($limit < 3) { $queryLimit = 3; @@ -1309,7 +1313,6 @@ class Share extends \OC\Share\Constants { } } else { $queryLimit = null; - $where .= ' ORDER BY `*PREFIX*share`.`id` ASC'; } $select = self::createSelectStatement($format, $fileDependent, $uidOwner); $root = strlen($root); @@ -1338,10 +1341,10 @@ class Share extends \OC\Share\Constants { } } else if (!isset($uidOwner)) { // Check if the same target already exists - if (isset($targets[$row[$column]])) { + if (isset($targets[$row['id']])) { // Check if the same owner shared with the user twice // through a group and user share - this is allowed - $id = $targets[$row[$column]]; + $id = $targets[$row['id']]; if (isset($items[$id]) && $items[$id]['uid_owner'] == $row['uid_owner']) { // Switch to group share type to ensure resharing conditions aren't bypassed if ($items[$id]['share_type'] != self::SHARE_TYPE_GROUP) { @@ -1357,12 +1360,12 @@ class Share extends \OC\Share\Constants { unset($items[$id]); $id = $row['id']; } - // Combine the permissions for the item $items[$id]['permissions'] |= (int)$row['permissions']; - continue; + } - } else { - $targets[$row[$column]] = $row['id']; + continue; + } elseif (!empty($row['parent'])) { + $targets[$row['parent']] = $row['id']; } } // Remove root from file source paths if retrieving own shared items @@ -1417,10 +1420,13 @@ class Share extends \OC\Share\Constants { $row['displayname_owner'] = \OCP\User::getDisplayName($row['uid_owner']); } - $items[$row['id']] = $row; + if ($row['permissions'] > 0) { + $items[$row['id']] = $row; + } } + // group items if we are looking for items shared with the current user if (isset($shareWith) && $shareWith === \OCP\User::getUser()) { $items = self::groupItems($items, $itemType); } @@ -1646,7 +1652,8 @@ class Share extends \OC\Share\Constants { } foreach ($users as $user) { - $sourceExists = self::getItemSharedWithBySource($itemType, $itemSource, self::FORMAT_NONE, null, true, $user); + $sourceId = ($itemType === 'file' || $itemType === 'folder') ? $fileSource : $itemSource; + $sourceExists = self::getItemSharedWithBySource($itemType, $sourceId, self::FORMAT_NONE, null, true, $user); $shareType = ($isGroupShare) ? self::$shareTypeGroupUserUnique : $shareType; @@ -1679,9 +1686,23 @@ class Share extends \OC\Share\Constants { } } else { - // move to the next user if it is neither a unique group share - // nor a single share without adding it to the share table - continue; + + // group share which doesn't exists until now, check if we need a unique target for this user + + $itemTarget = Helper::generateTarget($itemType, $itemSource, self::SHARE_TYPE_USER, $user, + $uidOwner, $suggestedItemTarget, $parent); + + // do we also need a file target + if (isset($fileSource)) { + $fileTarget = Helper::generateTarget('file', $filePath, self::SHARE_TYPE_USER, $user, + $uidOwner, $suggestedFileTarget, $parent); + } else { + $fileTarget = null; + } + + if ($itemTarget === $groupItemTarget) { + continue; + } } $queriesToExecute[] = array(