From f8a5812f9846fe9799eacd2f2231af3707016e36 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Apr 2020 15:41:20 +0200 Subject: [PATCH 1/2] Try to use the display name of file transfers Signed-off-by: Joas Schilling --- .../lib/Service/OwnershipTransferService.php | 28 ++++++++++++++----- lib/private/Files/Filesystem.php | 2 +- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index e1b26c6449..3415a2fd9e 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -36,6 +36,7 @@ use OCA\Files\Exception\TransferOwnershipException; use OCP\Encryption\IManager as IEncryptionManager; use OCP\Files\FileInfo; use OCP\Files\IHomeStorage; +use OCP\Files\InvalidPathException; use OCP\Files\Mount\IMountManager; use OCP\IUser; use OCP\Share\IManager as IShareManager; @@ -94,18 +95,31 @@ class OwnershipTransferService { throw new TransferOwnershipException("The target user is not ready to accept files. The user has at least to have logged in once.", 2); } - if ($move) { - $finalTarget = "$destinationUid/files/"; - } else { - $date = date('Y-m-d H-i-s'); - $finalTarget = "$destinationUid/files/transferred from $sourceUid on $date"; - } - // setup filesystem Filesystem::initMountPoints($sourceUid); Filesystem::initMountPoints($destinationUid); $view = new View(); + + if ($move) { + $finalTarget = "$destinationUid/files/"; + } else { + $date = date('Y-m-d H-i-s'); + + // Remove some characters which are prone to cause errors + $cleanUserName = str_replace(['\\', '/', ':', '.', '?', '#', '\'', '"'], '-', $sourceUser->getDisplayName()); + // Replace multiple dashes with one dash + $cleanUserName = preg_replace('/-{2,}/s', '-', $cleanUserName); + $cleanUserName = $cleanUserName ?: $sourceUid; + + $finalTarget = "$destinationUid/files/transferred from $cleanUserName on $date"; + try { + $view->verifyPath(dirname($finalTarget), basename($finalTarget)); + } catch (InvalidPathException $e) { + $finalTarget = "$destinationUid/files/transferred from $sourceUid on $date"; + } + } + if (!($view->is_dir($sourcePath) || $view->is_file($sourcePath))) { throw new TransferOwnershipException("Unknown path provided: $path", 1); } diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index 8fddbf7120..44da2e6f20 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -828,7 +828,7 @@ class Filesystem { $patterns = [ '/\\\\/s', // no windows style slashes '/\/\.(\/\.)?\//s', // remove '/./' - '/\/{2,}/s', // remove squence of slashes + '/\/{2,}/s', // remove sequence of slashes '/\/\.$/s', // remove trailing /. ]; From 3ea37d0d10a855438307cb69f5f9c7c452ceba95 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 8 Apr 2020 16:09:03 +0200 Subject: [PATCH 2/2] Show the displayname instead of the user id Signed-off-by: Joas Schilling --- apps/files/lib/Notification/Notifier.php | 53 +++++++++++++++--------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php index 529eb7cbc7..0ce5287b11 100644 --- a/apps/files/lib/Notification/Notifier.php +++ b/apps/files/lib/Notification/Notifier.php @@ -32,6 +32,8 @@ use OCA\Files\Db\TransferOwnershipMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Notification\IAction; use OCP\Notification\IDismissableNotifier; @@ -50,22 +52,22 @@ class Notifier implements INotifier, IDismissableNotifier { private $mapper; /** @var IManager */ private $notificationManager; + /** @var IUserManager */ + private $userManager; /** @var ITimeFactory */ private $timeFactory; - /** - * @param IFactory $l10nFactory - * @param IURLGenerator $urlGenerator - */ public function __construct(IFactory $l10nFactory, IURLGenerator $urlGenerator, TransferOwnershipMapper $mapper, IManager $notificationManager, + IUserManager $userManager, ITimeFactory $timeFactory) { $this->l10nFactory = $l10nFactory; $this->urlGenerator = $urlGenerator; $this->mapper = $mapper; $this->notificationManager = $notificationManager; + $this->userManager = $userManager; $this->timeFactory = $timeFactory; } @@ -138,6 +140,7 @@ class Notifier implements INotifier, IDismissableNotifier { IAction::TYPE_DELETE ); + $sourceUser = $this->getUser($param['sourceUser']); $notification->addParsedAction($approveAction) ->addParsedAction($disapproveAction) ->setRichSubject( @@ -145,11 +148,11 @@ class Notifier implements INotifier, IDismissableNotifier { [ 'user' => [ 'type' => 'user', - 'id' => $param['sourceUser'], - 'name' => $param['sourceUser'], + 'id' => $sourceUser->getUID(), + 'name' => $sourceUser->getDisplayName(), ], ]) - ->setParsedSubject(str_replace('{user}', $param['sourceUser'], $l->t('Incoming ownership transfer from {user}'))) + ->setParsedSubject(str_replace('{user}', $sourceUser->getDisplayName(), $l->t('Incoming ownership transfer from {user}'))) ->setRichMessage( $l->t("Do you want to accept {path}?\n\nNote: The transfer process after accepting may take up to 1 hour."), [ @@ -168,6 +171,7 @@ class Notifier implements INotifier, IDismissableNotifier { $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); + $targetUser = $this->getUser($param['targetUser']); $notification->setRichSubject($l->t('Ownership transfer failed')) ->setParsedSubject($l->t('Ownership transfer failed')) @@ -181,11 +185,11 @@ class Notifier implements INotifier, IDismissableNotifier { ], 'user' => [ 'type' => 'user', - 'id' => $param['targetUser'], - 'name' => $param['targetUser'], + 'id' => $targetUser->getUID(), + 'name' => $targetUser->getDisplayName(), ], ]) - ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your ownership transfer of {path} to {user} failed.'))); + ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $targetUser->getDisplayName()], $l->t('Your ownership transfer of {path} to {user} failed.'))); return $notification; } @@ -193,6 +197,7 @@ class Notifier implements INotifier, IDismissableNotifier { $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); + $sourceUser = $this->getUser($param['sourceUser']); $notification->setRichSubject($l->t('Ownership transfer failed')) ->setParsedSubject($l->t('Ownership transfer failed')) @@ -206,11 +211,11 @@ class Notifier implements INotifier, IDismissableNotifier { ], 'user' => [ 'type' => 'user', - 'id' => $param['sourceUser'], - 'name' => $param['sourceUser'], + 'id' => $sourceUser->getUID(), + 'name' => $sourceUser->getDisplayName(), ], ]) - ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The ownership transfer of {path} from {user} failed.'))); + ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $sourceUser->getDisplayName()], $l->t('The ownership transfer of {path} from {user} failed.'))); return $notification; } @@ -219,6 +224,7 @@ class Notifier implements INotifier, IDismissableNotifier { $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); + $targetUser = $this->getUser($param['targetUser']); $notification->setRichSubject($l->t('Ownership transfer done')) ->setParsedSubject($l->t('Ownership transfer done')) @@ -232,11 +238,11 @@ class Notifier implements INotifier, IDismissableNotifier { ], 'user' => [ 'type' => 'user', - 'id' => $param['targetUser'], - 'name' => $param['targetUser'], + 'id' => $targetUser->getUID(), + 'name' => $targetUser->getDisplayName(), ], ]) - ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your ownership transfer of {path} to {user} has completed.'))); + ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $targetUser->getDisplayName()], $l->t('Your ownership transfer of {path} to {user} has completed.'))); return $notification; } @@ -245,6 +251,7 @@ class Notifier implements INotifier, IDismissableNotifier { $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); + $sourceUser = $this->getUser($param['sourceUser']); $notification->setRichSubject($l->t('Ownership transfer done')) ->setParsedSubject($l->t('Ownership transfer done')) @@ -258,11 +265,11 @@ class Notifier implements INotifier, IDismissableNotifier { ], 'user' => [ 'type' => 'user', - 'id' => $param['sourceUser'], - 'name' => $param['sourceUser'], + 'id' => $sourceUser->getUID(), + 'name' => $sourceUser->getDisplayName(), ], ]) - ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The ownership transfer of {path} from {user} has completed.'))); + ->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $sourceUser->getDisplayName()], $l->t('The ownership transfer of {path} from {user} has completed.'))); return $notification; } @@ -293,4 +300,12 @@ class Notifier implements INotifier, IDismissableNotifier { $this->mapper->delete($transferOwnership); } + + protected function getUser(string $userId): IUser { + $user = $this->userManager->get($userId); + if ($user instanceof IUser) { + return $user; + } + throw new \InvalidArgumentException('User not found'); + } }