diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 8ed52a1de41d6..0f1ad3bd5d557 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -236,7 +236,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra $expiration = $share->getExpirationDate(); if ($expiration !== null) { $expiration->setTimezone($this->dateTimeZone->getTimeZone()); - $result['expiration'] = $expiration->format('Y-m-d 00:00:00'); + $result['expiration'] = $expiration->format('Y-m-d H:i:s'); } if ($share->getShareType() === IShare::TYPE_USER) { diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 713af803ab2e8..e0340b3ce0015 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1052,7 +1052,7 @@ public function testUpdateShareExpireDate(): void { $share1 = $this->shareManager->getShareById($share1->getFullId()); // date should be changed - $dateWithinRange->setTime(0, 0, 0); + $dateWithinRange->setTime(23, 59, 59); $dateWithinRange->setTimezone(new \DateTimeZone(date_default_timezone_get())); $this->assertEquals($dateWithinRange, $share1->getExpirationDate()); @@ -1263,7 +1263,7 @@ public function testShareStorageMountPoint(): void { public static function datesProvider() { $date = new \DateTime(); - $date->setTime(0, 0); + $date->setTime(23, 59, 59); $date->add(new \DateInterval('P5D')); $date->setTimezone(new \DateTimeZone(date_default_timezone_get())); @@ -1325,14 +1325,14 @@ public function testCreatePublicLinkExpireDateValid(): void { $data = $result->getData(); $this->assertTrue(is_string($data['token'])); - $this->assertEquals($date->format('Y-m-d 00:00:00'), $data['expiration']); + $this->assertEquals($date->format('Y-m-d 23:59:59'), $data['expiration']); // check for correct link $url = Server::get(IURLGenerator::class)->getAbsoluteURL('/index.php/s/' . $data['token']); $this->assertEquals($url, $data['url']); $share = $this->shareManager->getShareById('ocinternal:' . $data['id']); - $date->setTime(0, 0, 0); + $date->setTime(23, 59, 59); $this->assertEquals($date, $share->getExpirationDate()); $this->shareManager->deleteShare($share); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 2fe2281891a38..28caea90ceae8 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -773,7 +773,7 @@ public static function dataGetShare(): array { $data['Folder shared with group'] = [$share, $expected, true]; // File shared by link with Expire - $expire = \DateTime::createFromFormat('Y-m-d h:i:s', '2000-01-02 01:02:03'); + $expire = \DateTime::createFromFormat('Y-m-d H:i:s', '2000-01-02 23:59:59'); $share = [ '101', IShare::TYPE_LINK, @@ -807,7 +807,7 @@ public static function dataGetShare(): array { 'file_target' => 'target', 'file_parent' => 3, 'token' => 'token', - 'expiration' => '2000-01-02 00:00:00', + 'expiration' => '2000-01-02 23:59:59', 'permissions' => 4, 'attributes' => null, 'stime' => 5, @@ -4504,7 +4504,7 @@ public static function dataFormatShare(): array { 'permissions' => 1, 'stime' => 946684862, 'parent' => null, - 'expiration' => '2001-02-03 00:00:00', + 'expiration' => '2001-02-03 04:05:06', 'token' => null, 'uid_file_owner' => 'owner', 'displayname_file_owner' => 'owner', @@ -4554,7 +4554,7 @@ public static function dataFormatShare(): array { 'permissions' => 1, 'stime' => 946684862, 'parent' => null, - 'expiration' => '2001-02-03 00:00:00', + 'expiration' => '2001-02-03 04:05:06', 'token' => null, 'uid_file_owner' => 'owner', 'displayname_file_owner' => 'owner', diff --git a/apps/files_sharing/tests/SharesReminderJobTest.php b/apps/files_sharing/tests/SharesReminderJobTest.php index 475d418f1e2fe..641d9a5295fca 100644 --- a/apps/files_sharing/tests/SharesReminderJobTest.php +++ b/apps/files_sharing/tests/SharesReminderJobTest.php @@ -99,12 +99,11 @@ public static function dataSharesReminder() { $someMail = 'test@test.com'; $noExpirationDate = null; $today = new \DateTime(); - // For expiration dates, the time is always automatically set to zero by ShareAPIController - $today->setTime(0, 0); - $nearFuture = new \DateTime(); - $nearFuture->setTimestamp($today->getTimestamp() + 86400 * 1); + // Expiration dates are set to end of day (23:59:59) by the Share Manager + $today->setTime(23, 59, 59); + $nearFuture = clone $today; $farFuture = new \DateTime(); - $farFuture->setTimestamp($today->getTimestamp() + 86400 * 2); + $farFuture->setTimestamp($today->getTimestamp() + 86400 * 1); $permissionRead = Constants::PERMISSION_READ; $permissionCreate = $permissionRead | Constants::PERMISSION_CREATE; $permissionUpdate = $permissionRead | Constants::PERMISSION_UPDATE; diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index bd12f1f2455a6..72f6902af167e 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -310,7 +310,7 @@ public function isFieldInResponse($field, $contentExpected) { $data = simplexml_load_string($this->response->getBody())->data[0]; if ((string)$field == 'expiration') { if (!empty($contentExpected)) { - $contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 00:00:00'; + $contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 23:59:59'; } } if (count($data->element) > 0) { @@ -611,7 +611,7 @@ private function assertFieldIsInReturnedShare(string $field, string $contentExpe } if ($field === 'expiration' && !empty($contentExpected)) { - $contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 00:00:00'; + $contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 23:59:59'; } if ($contentExpected === 'A_NUMBER') { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 1d72ec63cb0ff..586a1ff9c0d88 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -302,7 +302,7 @@ protected function validateExpirationDateInternal(IShare $share): IShare { if (!$share->getNoExpirationDate() || $isEnforced) { if ($expirationDate !== null) { $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); + $expirationDate->setTime(23, 59, 59); $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $date->setTime(0, 0, 0); @@ -321,7 +321,7 @@ protected function validateExpirationDateInternal(IShare $share): IShare { if ($fullId === null && $expirationDate === null && $defaultExpireDate) { $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); + $expirationDate->setTime(23, 59, 59); $days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays); if ($days > $defaultExpireDays) { $days = $defaultExpireDays; @@ -336,7 +336,7 @@ protected function validateExpirationDateInternal(IShare $share): IShare { } $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); + $date->setTime(23, 59, 59); $date->add(new \DateInterval('P' . $defaultExpireDays . 'D')); if ($date < $expirationDate) { 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); @@ -380,7 +380,7 @@ protected function validateExpirationDateLink(IShare $share): IShare { if (!($share->getNoExpirationDate() && !$isEnforced)) { if ($expirationDate !== null) { $expirationDate->setTimezone($this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); + $expirationDate->setTime(23, 59, 59); $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); $date->setTime(0, 0, 0); @@ -399,7 +399,7 @@ protected function validateExpirationDateLink(IShare $share): IShare { if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) { $expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $expirationDate->setTime(0, 0, 0); + $expirationDate->setTime(23, 59, 59); $days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays()); if ($days > $this->shareApiLinkDefaultExpireDays()) { @@ -415,7 +415,7 @@ protected function validateExpirationDateLink(IShare $share): IShare { } $date = new \DateTime('now', $this->dateTimeZone->getTimeZone()); - $date->setTime(0, 0, 0); + $date->setTime(23, 59, 59); $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); if ($date < $expirationDate) { throw new GenericShareException( diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 2afb5e6c3b3b7..ba8dfe1442f6b 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1199,7 +1199,7 @@ public function testValidateExpirationDateInternalEnforceButNotSetNewShare($shar } $expected = new \DateTime('now', $this->timezone); - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $expected->add(new \DateInterval('P3D')); self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); @@ -1232,7 +1232,7 @@ public function testValidateExpirationDateInternalEnforceRelaxedDefaultButNotSet } $expected = new \DateTime('now', $this->timezone); - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $expected->add(new \DateInterval('P1D')); self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]); @@ -1279,7 +1279,7 @@ public function testValidateExpirationDateInternalEnforceValid($shareType): void $future->setTime(1, 2, 3); $expected = clone $future; - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1319,7 +1319,7 @@ public function testValidateExpirationDateInternalNoDefault($shareType): void { $date->setTime(1, 2, 3); $expected = clone $date; - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1359,7 +1359,7 @@ public function testValidateExpirationDateInternalNoDateDefault($shareType): voi $share->setShareType($shareType); $expected = new \DateTime('now', $this->timezone); - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $expected->add(new \DateInterval('P3D')); $expected->setTimezone(new \DateTimeZone(date_default_timezone_get())); @@ -1397,7 +1397,7 @@ public function testValidateExpirationDateInternalDefault($shareType): void { $future->setTime(1, 2, 3); $expected = clone $future; - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1434,7 +1434,7 @@ public function testValidateExpirationDateInternalDefault($shareType): void { public function testValidateExpirationDateInternalHookModification($shareType): void { $nextWeek = new \DateTime('now', $this->timezone); $nextWeek->add(new \DateInterval('P7D')); - $nextWeek->setTime(0, 0, 0); + $nextWeek->setTime(23, 59, 59); $save = clone $nextWeek; @@ -1461,7 +1461,7 @@ public function testValidateExpirationDateInternalHookException($shareType): voi $nextWeek = new \DateTime(); $nextWeek->add(new \DateInterval('P7D')); - $nextWeek->setTime(0, 0, 0); + $nextWeek->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1562,7 +1562,7 @@ public function testValidateExpirationDateEnforceButNotSetNewShare(): void { ]); $expected = new \DateTime('now', $this->timezone); - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $expected->add(new \DateInterval('P3D')); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]); @@ -1587,7 +1587,7 @@ public function testValidateExpirationDateEnforceRelaxedDefaultButNotSetNewShare ]); $expected = new \DateTime('now', $this->timezone); - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $expected->add(new \DateInterval('P1D')); self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]); @@ -1626,7 +1626,7 @@ public function testValidateExpirationDateEnforceValid(): void { $future->setTime(1, 2, 3); $expected = clone $future; - $expected->setTime(0, 0, 0); + $expected->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setExpirationDate($future); @@ -1659,7 +1659,7 @@ public function testValidateExpirationDateNoDefault(): void { $date->setTime(1, 2, 3); $expected = clone $date; - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $expected->setTimezone(new \DateTimeZone(date_default_timezone_get())); $share = $this->manager->newShare(); @@ -1696,7 +1696,7 @@ public function testValidateExpirationDateNoDateDefault(): void { $expected = new \DateTime('now', $this->timezone); $expected->add(new \DateInterval('P3D')); - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $expected->setTimezone(new \DateTimeZone(date_default_timezone_get())); $this->config->method('getAppValue') @@ -1728,7 +1728,7 @@ public function testValidateExpirationDateDefault(): void { $future->setTime(1, 2, 3); $expected = clone $future; - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $expected->setTimezone(new \DateTimeZone(date_default_timezone_get())); $share = $this->manager->newShare(); @@ -1764,7 +1764,7 @@ public function testValidateExpirationNegativeOffsetTimezone(): void { $expected = clone $future; $expected->setTimezone($this->timezone); - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $expected->setTimezone(new \DateTimeZone(date_default_timezone_get())); $share = $this->manager->newShare(); @@ -1798,7 +1798,7 @@ public function testValidateExpirationDateHookModification(): void { $nextWeek->add(new \DateInterval('P7D')); $save = clone $nextWeek; - $save->setTime(0, 0); + $save->setTime(23, 59, 59); $save->sub(new \DateInterval('P2D')); $save->setTimezone(new \DateTimeZone(date_default_timezone_get())); @@ -1822,7 +1822,7 @@ public function testValidateExpirationDateHookException(): void { $nextWeek = new \DateTime(); $nextWeek->add(new \DateInterval('P7D')); - $nextWeek->setTime(0, 0, 0); + $nextWeek->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setExpirationDate($nextWeek); @@ -2486,7 +2486,7 @@ public function testCanShare($expected, $sharingEnabled, $disabledForUser): void public function testCreateShareUser(): void { /** @var Manager&MockObject $manager */ $manager = $this->createManagerMock() - ->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2521,6 +2521,10 @@ public function testCreateShareUser(): void { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once()) @@ -2540,7 +2544,7 @@ public function testCreateShareUser(): void { public function testCreateShareGroup(): void { $manager = $this->createManagerMock() - ->onlyMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks']) + ->onlyMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2575,6 +2579,10 @@ public function testCreateShareGroup(): void { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once()) @@ -2806,6 +2814,7 @@ public function testCreateShareHookError(): void { 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', + 'validateExpirationDateInternal', ]) ->getMock(); @@ -2841,6 +2850,10 @@ public function testCreateShareHookError(): void { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $share->expects($this->once()) ->method('setShareOwner') @@ -2865,7 +2878,7 @@ public function testCreateShareHookError(): void { public function testCreateShareOfIncomingFederatedShare(): void { $manager = $this->createManagerMock() - ->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2919,6 +2932,10 @@ public function testCreateShareOfIncomingFederatedShare(): void { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once())