Skip to content

Commit 728644d

Browse files
AltahrimAltahrim
authored
Merge pull request #59731 from nextcloud/jtr/refactor-share-dry-exp-dat-validation
refactor(share): DRY up expiration date validation and fix dispatchEvent() log message
2 parents 46a8117 + af076d5 commit 728644d

1 file changed

Lines changed: 62 additions & 90 deletions

File tree

‎lib/private/Share20/Manager.php‎

Lines changed: 62 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -278,33 +278,28 @@ protected function generalCreateChecks(IShare $share, bool $isUpdate = false): v
278278
}
279279

280280
/**
281-
* Validate if the expiration date fits the system settings
281+
* Validate if the expiration date fits the provided share policy settings.
282282
*
283283
* @param IShare $share The share to validate the expiration date of
284+
* @param bool $isEnforced Whether an expiration date is mandatory and capped
285+
* @param bool $defaultExpireDate Whether a default expiration date should be applied
286+
* @param int $defaultExpireDays Maximum/default number of expiration days
287+
* @param string $configProp Config key used to retrieve the admin-configured default days
284288
* @return IShare The modified share object
285289
* @throws GenericShareException
286290
* @throws \InvalidArgumentException
287291
* @throws \Exception
288292
*/
289-
protected function validateExpirationDateInternal(IShare $share): IShare {
290-
$isRemote = $share->getShareType() === IShare::TYPE_REMOTE || $share->getShareType() === IShare::TYPE_REMOTE_GROUP;
291-
293+
protected function validateExpirationDate(
294+
IShare $share,
295+
bool $isEnforced,
296+
bool $defaultExpireDate,
297+
int $defaultExpireDays,
298+
string $configProp,
299+
): IShare {
292300
$expirationDate = $share->getExpirationDate();
293301

294-
if ($isRemote) {
295-
$defaultExpireDate = $this->shareApiRemoteDefaultExpireDate();
296-
$defaultExpireDays = $this->shareApiRemoteDefaultExpireDays();
297-
$configProp = 'remote_defaultExpDays';
298-
$isEnforced = $this->shareApiRemoteDefaultExpireDateEnforced();
299-
} else {
300-
$defaultExpireDate = $this->shareApiInternalDefaultExpireDate();
301-
$defaultExpireDays = $this->shareApiInternalDefaultExpireDays();
302-
$configProp = 'internal_defaultExpDays';
303-
$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
304-
}
305-
306-
// If $expirationDate is falsy, noExpirationDate is true and expiration not enforced
307-
// Then skip expiration date validation as null is accepted
302+
// If no-expiration is allowed and expiration is not enforced, null is accepted.
308303
if (!$share->getNoExpirationDate() || $isEnforced) {
309304
if ($expirationDate !== null) {
310305
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
@@ -317,7 +312,7 @@ protected function validateExpirationDateInternal(IShare $share): IShare {
317312
}
318313
}
319314

320-
// If expiredate is empty set a default one if there is a default
315+
// If expire date is empty, set a default one for new shares if configured.
321316
$fullId = null;
322317
try {
323318
$fullId = $share->getFullId();
@@ -328,14 +323,15 @@ protected function validateExpirationDateInternal(IShare $share): IShare {
328323
if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
329324
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
330325
$expirationDate->setTime(23, 59, 59);
326+
331327
$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
332328
if ($days > $defaultExpireDays) {
333329
$days = $defaultExpireDays;
334330
}
335331
$expirationDate->add(new \DateInterval('P' . $days . 'D'));
336332
}
337333

338-
// If we enforce the expiration date check that is does not exceed
334+
// If expiration is enforced, it must be present and must not exceed the maximum.
339335
if ($isEnforced) {
340336
if (empty($expirationDate)) {
341337
throw new \InvalidArgumentException($this->l->t('Expiration date is enforced'));
@@ -345,7 +341,14 @@ protected function validateExpirationDateInternal(IShare $share): IShare {
345341
$date->setTime(23, 59, 59);
346342
$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
347343
if ($date < $expirationDate) {
348-
throw new GenericShareException($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), code: 404);
344+
throw new GenericShareException(
345+
$this->l->n(
346+
'Cannot set expiration date more than %n day in the future',
347+
'Cannot set expiration date more than %n days in the future',
348+
$defaultExpireDays
349+
),
350+
code: 404,
351+
);
349352
}
350353
}
351354
}
@@ -377,78 +380,47 @@ protected function validateExpirationDateInternal(IShare $share): IShare {
377380
* @throws \InvalidArgumentException
378381
* @throws \Exception
379382
*/
380-
protected function validateExpirationDateLink(IShare $share): IShare {
381-
$expirationDate = $share->getExpirationDate();
382-
$isEnforced = $this->shareApiLinkDefaultExpireDateEnforced();
383-
384-
// If $expirationDate is falsy, noExpirationDate is true and expiration not enforced
385-
// Then skip expiration date validation as null is accepted
386-
if (!($share->getNoExpirationDate() && !$isEnforced)) {
387-
if ($expirationDate !== null) {
388-
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
389-
$expirationDate->setTime(23, 59, 59);
390-
391-
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
392-
$date->setTime(0, 0, 0);
393-
if ($date >= $expirationDate) {
394-
throw new GenericShareException($this->l->t('Expiration date is in the past'), code: 404);
395-
}
396-
}
397-
398-
// If expiredate is empty set a default one if there is a default
399-
$fullId = null;
400-
try {
401-
$fullId = $share->getFullId();
402-
} catch (\UnexpectedValueException $e) {
403-
// This is a new share
404-
}
405-
406-
if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
407-
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
408-
$expirationDate->setTime(23, 59, 59);
409-
410-
$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());
411-
if ($days > $this->shareApiLinkDefaultExpireDays()) {
412-
$days = $this->shareApiLinkDefaultExpireDays();
413-
}
414-
$expirationDate->add(new \DateInterval('P' . $days . 'D'));
415-
}
416-
417-
// If we enforce the expiration date check that is does not exceed
418-
if ($isEnforced) {
419-
if (empty($expirationDate)) {
420-
throw new \InvalidArgumentException($this->l->t('Expiration date is enforced'));
421-
}
422-
423-
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
424-
$date->setTime(23, 59, 59);
425-
$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
426-
if ($date < $expirationDate) {
427-
throw new GenericShareException(
428-
$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()),
429-
code: 404,
430-
);
431-
}
432-
}
433-
434-
}
435-
436-
$accepted = true;
437-
$message = '';
438-
Util::emitHook('\OC\Share', 'verifyExpirationDate', [
439-
'expirationDate' => &$expirationDate,
440-
'accepted' => &$accepted,
441-
'message' => &$message,
442-
'passwordSet' => $share->getPassword() !== null,
443-
]);
383+
protected function validateExpirationDateInternal(IShare $share): IShare {
384+
$isRemote = $share->getShareType() === IShare::TYPE_REMOTE || $share->getShareType() === IShare::TYPE_REMOTE_GROUP;
444385

445-
if (!$accepted) {
446-
throw new \Exception($message);
386+
if ($isRemote) {
387+
$defaultExpireDate = $this->shareApiRemoteDefaultExpireDate();
388+
$defaultExpireDays = $this->shareApiRemoteDefaultExpireDays();
389+
$configProp = 'remote_defaultExpDays';
390+
$isEnforced = $this->shareApiRemoteDefaultExpireDateEnforced();
391+
} else {
392+
$defaultExpireDate = $this->shareApiInternalDefaultExpireDate();
393+
$defaultExpireDays = $this->shareApiInternalDefaultExpireDays();
394+
$configProp = 'internal_defaultExpDays';
395+
$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
447396
}
448397

449-
$share->setExpirationDate($expirationDate);
398+
return $this->validateExpirationDate(
399+
$share,
400+
$isEnforced,
401+
$defaultExpireDate,
402+
$defaultExpireDays,
403+
$configProp,
404+
);
405+
}
450406

451-
return $share;
407+
/**
408+
* Validate if the expiration date fits the system settings
409+
*
410+
* @param IShare $share The share to validate the expiration date of
411+
* @return IShare The modified share object
412+
* @throws GenericShareException
413+
* @throws \InvalidArgumentException
414+
* @throws \Exception
415+
*/
416+
protected function validateExpirationDateLink(IShare $share): IShare {
417+
return $this->validateExpirationDate(
418+
$share,
419+
$this->shareApiLinkDefaultExpireDateEnforced(),
420+
$this->shareApiLinkDefaultExpireDate(),
421+
$this->shareApiLinkDefaultExpireDays(),
422+
'link_defaultExpDays',
423+
);
452424
}
453425

454426
/**
@@ -1990,7 +1962,7 @@ private function dispatchEvent(Event $event, string $name): void {
19901962
try {
19911963
$this->dispatcher->dispatchTyped($event);
19921964
} catch (\Exception $e) {
1993-
$this->logger->error("Error while sending ' . $name . ' event", ['exception' => $e]);
1965+
$this->logger->error("Error while sending '$name' event", ['exception' => $e]);
19941966
}
19951967
}
19961968

0 commit comments

Comments
 (0)