From 59a5b4e0bddbc29d4e16a930972be34873a73e0c 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 | 23 ++- apps/files_sharing/openapi.json | 4 +- lib/private/Share20/Manager.php | 167 ++++++++++-------- lib/private/Share20/Share.php | 19 +- lib/public/Share/IShare.php | 24 ++- 5 files changed, 144 insertions(+), 93 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 1bdcee11c45ee..b9f89109304ce 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -549,7 +549,8 @@ public function deleteShare(string $id): DataResponse { * @param string $publicUpload If public uploading is allowed * @param string $password Password for the share * @param string|null $sendPasswordByTalk Send the password for the share over Talk - * @param string $expireDate Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used. + * @param ?string $expireDate The expiry date of the share in the user's timezone (UTC) at 00:00. + * If $expireDate is not supplied or set to `null`, the system default will be used. * @param string $note Note for the share * @param string $label Label for the share (only used in link and email) * @param string|null $attributes Additional attributes for the share @@ -571,7 +572,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 +647,18 @@ 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. + // Set noExpirationDate to true so overwrite is prevented. + $share->setNoExpirationDate(true); } } diff --git a/apps/files_sharing/openapi.json b/apps/files_sharing/openapi.json index 6808993af3fa6..f5a8ead3eba3b 100644 --- a/apps/files_sharing/openapi.json +++ b/apps/files_sharing/openapi.json @@ -1781,10 +1781,10 @@ { "name": "expireDate", "in": "query", - "description": "Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.", + "description": "The expiry date of the share in the user's timezone (UTC) at 00:00. If $expireDate is not supplied or set to `null`, the system default will be used.", "schema": { "type": "string", - "default": "" + "nullable": true } }, { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 4f097cdc3e280..4b44edd8b0138 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -348,26 +348,6 @@ protected function validateExpirationDateInternal(IShare $share) { $expirationDate = $share->getExpirationDate(); - if ($expirationDate !== null) { - $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); - - $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); - if ($date >= $expirationDate) { - $message = $this->l->t('Expiration date is in the past'); - throw new GenericShareException($message, $message, 404); - } - } - - // If expiredate is empty set a default one if there is a default - $fullId = null; - try { - $fullId = $share->getFullId(); - } catch (\UnexpectedValueException $e) { - // This is a new share - } - if ($isRemote) { $defaultExpireDate = $this->shareApiRemoteDefaultExpireDate(); $defaultExpireDays = $this->shareApiRemoteDefaultExpireDays(); @@ -379,28 +359,53 @@ protected function validateExpirationDateInternal(IShare $share) { $configProp = 'internal_defaultExpDays'; $isEnforced = $this->shareApiInternalDefaultExpireDateEnforced(); } - if ($fullId === null && $expirationDate === null && $defaultExpireDate) { - $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); - $days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays); - if ($days > $defaultExpireDays) { - $days = $defaultExpireDays; + + // If $expirationDate is falsy, noExpirationDate is true and expiration not enforced + // Then skip expiration date validation as null is accepted + if(!($share->getNoExpirationDate() && !$isEnforced)) { + if ($expirationDate != null) { + $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); + $expirationDate->setTime(0, 0, 0); + + $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $date->setTime(0, 0, 0); + if ($date >= $expirationDate) { + $message = $this->l->t('Expiration date is in the past'); + throw new GenericShareException($message, $message, 404); + } } - $expirationDate->add(new \DateInterval('P' . $days . 'D')); - } - // If we enforce the expiration date check that is does not exceed - if ($isEnforced) { - if ($expirationDate === null) { - throw new \InvalidArgumentException('Expiration date is enforced'); + // If expiredate is empty set a default one if there is a default + $fullId = null; + try { + $fullId = $share->getFullId(); + } catch (\UnexpectedValueException $e) { + // This is a new share } - $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); - $date->add(new \DateInterval('P' . $defaultExpireDays . 'D')); - if ($date < $expirationDate) { - $message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $defaultExpireDays); - throw new GenericShareException($message, $message, 404); + if ($fullId === null && $expirationDate === null && $defaultExpireDate) { + $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $expirationDate->setTime(0, 0, 0); + $days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays); + if ($days > $defaultExpireDays) { + $days = $defaultExpireDays; + } + $expirationDate->add(new \DateInterval('P' . $days . 'D')); + } + + // If we enforce the expiration date check that is does not exceed + if ($isEnforced) { + if (empty($expirationDate)) { + throw new \InvalidArgumentException('Expiration date is enforced'); + } + + $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $date->setTime(0, 0, 0); + $date->add(new \DateInterval('P' . $defaultExpireDays . 'D')); + if ($date < $expirationDate) { + $message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $defaultExpireDays); + throw new GenericShareException($message, $message, 404); + } } } @@ -433,51 +438,57 @@ protected function validateExpirationDateInternal(IShare $share) { */ protected function validateExpirationDateLink(IShare $share) { $expirationDate = $share->getExpirationDate(); - - if ($expirationDate !== null) { - $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); - - $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); - if ($date >= $expirationDate) { - $message = $this->l->t('Expiration date is in the past'); - throw new GenericShareException($message, $message, 404); + $isEnforced = $this->shareApiLinkDefaultExpireDateEnforced(); + + // If $expirationDate is falsy, noExpirationDate is true and expiration not enforced + // Then skip expiration date validation as null is accepted + if(!($share->getNoExpirationDate() && !$isEnforced)) { + if ($expirationDate !== null) { + $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); + $expirationDate->setTime(0, 0, 0); + + $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $date->setTime(0, 0, 0); + if ($date >= $expirationDate) { + $message = $this->l->t('Expiration date is in the past'); + throw new GenericShareException($message, $message, 404); + } } - } - - // If expiredate is empty set a default one if there is a default - $fullId = null; - try { - $fullId = $share->getFullId(); - } catch (\UnexpectedValueException $e) { - // This is a new share - } - if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) { - $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); - - $days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays()); - if ($days > $this->shareApiLinkDefaultExpireDays()) { - $days = $this->shareApiLinkDefaultExpireDays(); + // If expiredate is empty set a default one if there is a default + $fullId = null; + try { + $fullId = $share->getFullId(); + } catch (\UnexpectedValueException $e) { + // This is a new share + } + + if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) { + $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $expirationDate->setTime(0, 0, 0); + + $days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays()); + if ($days > $this->shareApiLinkDefaultExpireDays()) { + $days = $this->shareApiLinkDefaultExpireDays(); + } + $expirationDate->add(new \DateInterval('P' . $days . 'D')); } - $expirationDate->add(new \DateInterval('P' . $days . 'D')); - } - - // If we enforce the expiration date check that is does not exceed - if ($this->shareApiLinkDefaultExpireDateEnforced()) { - if ($expirationDate === null) { - throw new \InvalidArgumentException('Expiration date is enforced'); + + // If we enforce the expiration date check that is does not exceed + if ($isEnforced) { + if (empty($expirationDate)) { + throw new \InvalidArgumentException('Expiration date is enforced'); + } + + $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); + $date->setTime(0, 0, 0); + $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); + if ($date < $expirationDate) { + $message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $this->shareApiLinkDefaultExpireDays()); + throw new GenericShareException($message, $message, 404); + } } - $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); - $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); - if ($date < $expirationDate) { - $message = $this->l->n('Cannot set expiration date more than %n day in the future', 'Cannot set expiration date more than %n days in the future', $this->shareApiLinkDefaultExpireDays()); - throw new GenericShareException($message, $message, 404); - } } $accepted = true; diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index e1d8818216b7a..0bc85712647c7 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -90,13 +90,13 @@ class Share implements IShare { private $mailSend; /** @var string */ private $label = ''; - /** @var ICacheEntry|null */ private $nodeCacheEntry; - /** @var bool */ private $hideDownload = false; + private bool $noExpirationDate = false; + public function __construct( private IRootFolder $rootFolder, private IUserManager $userManager, @@ -421,6 +421,21 @@ public function getExpirationDate() { return $this->expireDate; } + /** + * @inheritdoc + */ + public function setNoExpirationDate(bool $noExpirationDate) { + $this->noExpirationDate = $noExpirationDate; + return $this; + } + + /** + * @inheritdoc + */ + public function getNoExpirationDate(): bool { + return $this->noExpirationDate; + } + /** * @inheritdoc */ diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index 0961631ea92f5..0f4f9561eeb65 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 $noExpirationDate + * @return \OCP\Share\IShare The modified object + * @since 30.0.0 + */ + public function setNoExpirationDate(bool $noExpirationDate); + + + /** + * Get value of overwrite falsy expiry date flag + * + * @return bool + * @since 30.0.0 + */ + public function getNoExpirationDate(); + /** * Is the share expired ? *