From 8ef8bf46689a6a5421d52861734429830b72e884 Mon Sep 17 00:00:00 2001 From: fenn-cs Date: Wed, 20 Mar 2024 16:41:26 +0100 Subject: [PATCH] fix(shareManager): Respect empty `expireDate` in server If `expireDate` is an empty string and not `null` then the server should not set a default. Signed-off-by: fenn-cs --- .../lib/Controller/ShareAPIController.php | 19 ++-- apps/files_sharing/openapi.json | 3 +- lib/private/Share20/Manager.php | 40 +++++--- lib/private/Share20/Share.php | 98 ++++++++----------- lib/public/Share/IShare.php | 24 ++++- 5 files changed, 104 insertions(+), 80 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 1bdcee11c45ee..4307154aa7aa6 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -571,7 +571,7 @@ public function createShare( string $publicUpload = 'false', string $password = '', ?string $sendPasswordByTalk = null, - string $expireDate = '', + ?string $expireDate = null, string $note = '', string $label = '', ?string $attributes = null @@ -646,12 +646,17 @@ public function createShare( } //Expire date - if ($expireDate !== '') { - try { - $expireDateTime = $this->parseDate($expireDate); - $share->setExpirationDate($expireDateTime); - } catch (\Exception $e) { - throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD')); + if ($expireDate !== null) { + if ($expireDate !== '') { + try { + $expireDateTime = $this->parseDate($expireDate); + $share->setExpirationDate($expireDateTime); + } catch (\Exception $e) { + throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD')); + } + } else { + // Client sent empty string for expire date. Do not overwrite. + $share->setOverwriteFalsyExpirationDate(false); } } diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 6808993af3fa6..51ee52fed9b32 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -1783,8 +1783,7 @@ "in": "query", "description": "Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.", "schema": { - "type": "string", - "default": "" + "type": "string" } }, { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 53dbf65ccc755..99d7ba441ae3c 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -389,7 +389,25 @@ protected function validateExpirationDateInternal(IShare $share) { $expirationDate = $share->getExpirationDate(); - if ($expirationDate !== null) { + if ($isRemote) { + $defaultExpireDate = $this->shareApiRemoteDefaultExpireDate(); + $defaultExpireDays = $this->shareApiRemoteDefaultExpireDays(); + $configProp = 'remote_defaultExpDays'; + $isEnforced = $this->shareApiRemoteDefaultExpireDateEnforced(); + } else { + $defaultExpireDate = $this->shareApiInternalDefaultExpireDate(); + $defaultExpireDays = $this->shareApiInternalDefaultExpireDays(); + $configProp = 'internal_defaultExpDays'; + $isEnforced = $this->shareApiInternalDefaultExpireDateEnforced(); + } + + // If $expirationDate is falsy, overwrite flag is false, no expiration is set + if(empty($expirationDate) && !$share->getOverwriteFalsyExpirationDate() && !$isEnforced) { + $share->setExpirationDate(null); + return $share; + } + + if ($expirationDate != null) { $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); $expirationDate->setTime(0, 0, 0); @@ -409,17 +427,6 @@ protected function validateExpirationDateInternal(IShare $share) { // This is a new share } - if ($isRemote) { - $defaultExpireDate = $this->shareApiRemoteDefaultExpireDate(); - $defaultExpireDays = $this->shareApiRemoteDefaultExpireDays(); - $configProp = 'remote_defaultExpDays'; - $isEnforced = $this->shareApiRemoteDefaultExpireDateEnforced(); - } else { - $defaultExpireDate = $this->shareApiInternalDefaultExpireDate(); - $defaultExpireDays = $this->shareApiInternalDefaultExpireDays(); - $configProp = 'internal_defaultExpDays'; - $isEnforced = $this->shareApiInternalDefaultExpireDateEnforced(); - } if ($fullId === null && $expirationDate === null && $defaultExpireDate) { $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $expirationDate->setTime(0, 0, 0); @@ -474,6 +481,13 @@ protected function validateExpirationDateInternal(IShare $share) { */ protected function validateExpirationDateLink(IShare $share) { $expirationDate = $share->getExpirationDate(); + $isEnforced = $this->shareApiLinkDefaultExpireDateEnforced(); + + // If $expirationDate is falsy, overwrite flag is false, no expiration is set + if(empty($expirationDate) && !$share->getOverwriteFalsyExpirationDate() && !$isEnforced) { + $share->setExpirationDate(null); + return $share; + } if ($expirationDate !== null) { $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); @@ -507,7 +521,7 @@ protected function validateExpirationDateLink(IShare $share) { } // If we enforce the expiration date check that is does not exceed - if ($this->shareApiLinkDefaultExpireDateEnforced()) { + if ($isEnforced) { if ($expirationDate === null) { throw new \InvalidArgumentException('Expiration date is enforced'); } diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index e1d8818216b7a..4f32d1ad35b61 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -41,61 +41,34 @@ use OCP\Share\IShare; class Share implements IShare { - /** @var string */ - private $id; - /** @var string */ - private $providerId; - /** @var Node */ - private $node; - /** @var int */ - private $fileId; - /** @var string */ - private $nodeType; - /** @var int */ - private $shareType; - /** @var string */ - private $sharedWith; - /** @var string */ - private $sharedWithDisplayName; - /** @var string */ - private $sharedWithAvatar; - /** @var string */ - private $sharedBy; - /** @var string */ - private $shareOwner; - /** @var int */ - private $permissions; - /** @var IAttributes */ - private $attributes; - /** @var int */ - private $status; - /** @var string */ - private $note = ''; - /** @var \DateTime */ - private $expireDate; - /** @var string */ - private $password; - private ?\DateTimeInterface $passwordExpirationTime = null; - /** @var bool */ - private $sendPasswordByTalk = false; - /** @var string */ - private $token; - /** @var int */ - private $parent; - /** @var string */ - private $target; - /** @var \DateTime */ - private $shareTime; - /** @var bool */ - private $mailSend; - /** @var string */ - private $label = ''; - - /** @var ICacheEntry|null */ - private $nodeCacheEntry; - - /** @var bool */ - private $hideDownload = false; + private string $id; + private string $providerId; + private Node $node; + private int $fileId; + private string $nodeType; + private int $shareType; + private string $sharedWith; + private string $sharedWithDisplayName; + private string $sharedWithAvatar; + private string $sharedBy; + private string $shareOwner; + private int $permissions; + private IAttributes $attributes; + private int $status; + private string $note = ''; + private \DateTime $expireDate; + private bool $overwriteFalsyExpirationDate = true; + private string $password; + private ?\DateTimeInterface $passwordExpirationTime = null; + private bool $sendPasswordByTalk = false; + private string $token; + private int $parent; + private string $target; + private \DateTime $shareTime; + private bool $mailSend; + private string $label = ''; + private ?ICacheEntry $nodeCacheEntry = null; + private bool $hideDownload = false; public function __construct( private IRootFolder $rootFolder, @@ -421,6 +394,21 @@ public function getExpirationDate() { return $this->expireDate; } + /** + * @inheritdoc + */ + public function setOverwriteFalsyExpirationDate(bool $overwriteFalsyExpirationDate) { + $this->overwriteFalsyExpirationDate = $overwriteFalsyExpirationDate; + return $this; + } + + /** + * @inheritdoc + */ + public function getOverwriteFalsyExpirationDate(): bool { + return $this->overwriteFalsyExpirationDate; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 0961631ea92f5..432e7e7c2d54b 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -385,20 +385,38 @@ public function getNote(); /** * Set the expiration date * - * @param null|\DateTime $expireDate + * @param \DateTime|null $expireDate * @return \OCP\Share\IShare The modified object * @since 9.0.0 */ - public function setExpirationDate($expireDate); + public function setExpirationDate(\DateTime|null $expireDate); /** * Get the expiration date * - * @return null|\DateTime + * @return \DateTime|null * @since 9.0.0 */ public function getExpirationDate(); + /** + * Set overwrite flag for falsy expiry date vavlues + * + * @param bool $overwriteFalsyExpirationDate + * @return \OCP\Share\IShare The modified object + * @since 27.0.0 + */ + public function setOverwriteFalsyExpirationDate(bool $overwriteFalsyExpirationDate); + + + /** + * Get value of overwrite falsy expiry date flag + * + * @return bool + * @since 27.0.0 + */ + public function getOverwriteFalsyExpirationDate(); + /** * Is the share expired ? *