Skip to content

Commit af076d5

Browse files
joshtrichardsjoshtrichards
authored
refactor(share): extract shared expiration date validation
Both `validateExpirationDateInternal()` and `validateExpirationDateLink()` implemented the same algorithm for: - skipping validation when no expiration date is allowed - rejecting past expiration dates - applying a default expiration date for new shares - enforcing the maximum allowed expiration date - running the `verifyExpirationDate` hook This change keeps the existing behavior but centralizes the logic so future fixes only need to be made in one place. This commit also fixes a tiny unrelated quoting bug in `dispatchEvent()` that caused garbled error-log output in `dispatchEvent()`. Signed-off-by: Josh <josh.t.richards@gmail.com>
1 parent 63a1a08 commit af076d5

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
@@ -276,33 +276,28 @@ protected function generalCreateChecks(IShare $share, bool $isUpdate = false): v
276276
}
277277

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

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

318-
// If expiredate is empty set a default one if there is a default
313+
// If expire date is empty, set a default one for new shares if configured.
319314
$fullId = null;
320315
try {
321316
$fullId = $share->getFullId();
@@ -326,14 +321,15 @@ protected function validateExpirationDateInternal(IShare $share): IShare {
326321
if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
327322
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
328323
$expirationDate->setTime(23, 59, 59);
324+
329325
$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
330326
if ($days > $defaultExpireDays) {
331327
$days = $defaultExpireDays;
332328
}
333329
$expirationDate->add(new \DateInterval('P' . $days . 'D'));
334330
}
335331

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

443-
if (!$accepted) {
444-
throw new \Exception($message);
384+
if ($isRemote) {
385+
$defaultExpireDate = $this->shareApiRemoteDefaultExpireDate();
386+
$defaultExpireDays = $this->shareApiRemoteDefaultExpireDays();
387+
$configProp = 'remote_defaultExpDays';
388+
$isEnforced = $this->shareApiRemoteDefaultExpireDateEnforced();
389+
} else {
390+
$defaultExpireDate = $this->shareApiInternalDefaultExpireDate();
391+
$defaultExpireDays = $this->shareApiInternalDefaultExpireDays();
392+
$configProp = 'internal_defaultExpDays';
393+
$isEnforced = $this->shareApiInternalDefaultExpireDateEnforced();
445394
}
446395

447-
$share->setExpirationDate($expirationDate);
396+
return $this->validateExpirationDate(
397+
$share,
398+
$isEnforced,
399+
$defaultExpireDate,
400+
$defaultExpireDays,
401+
$configProp,
402+
);
403+
}
448404

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

452424
/**
@@ -1981,7 +1953,7 @@ private function dispatchEvent(Event $event, string $name): void {
19811953
try {
19821954
$this->dispatcher->dispatchTyped($event);
19831955
} catch (\Exception $e) {
1984-
$this->logger->error("Error while sending ' . $name . ' event", ['exception' => $e]);
1956+
$this->logger->error("Error while sending '$name' event", ['exception' => $e]);
19851957
}
19861958
}
19871959

0 commit comments

Comments
 (0)