From 5158b437ce9bdf2890d4d1dcfb7fb50c5734b76f Mon Sep 17 00:00:00 2001 From: nfebe Date: Fri, 6 Feb 2026 00:41:24 +0100 Subject: [PATCH 1/2] fix(share): Set expiration time to end of day (23:59:59) Shares now expire at the end of the selected day instead of the beginning, allowing access throughout the entire expiration day. Also return actual stored time in API response instead of hardcoded 00:00:00 to prevent timezone-related display issues in the UI. Signed-off-by: nfebe --- .../lib/Controller/ShareAPIController.php | 2 +- lib/private/Share20/Manager.php | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 1db9c962a0384..08fedeb6bf653 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -231,7 +231,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'); } $currentUserPermissions = $recipientNode?->getPermissions() ?? Constants::PERMISSION_ALL; diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 4ec442d132b45..b22b7654ad51c 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -297,7 +297,7 @@ protected function validateExpirationDateInternal(IShare $share) { 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); @@ -316,8 +316,8 @@ protected function validateExpirationDateInternal(IShare $share) { 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); + $expirationDate->setTime(23, 59, 59); + $days = (int) $this->config->getAppValue('core', $configProp, (string) $defaultExpireDays); if ($days > $defaultExpireDays) { $days = $defaultExpireDays; } @@ -331,7 +331,7 @@ protected function validateExpirationDateInternal(IShare $share) { } $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); @@ -375,7 +375,7 @@ protected function validateExpirationDateLink(IShare $share) { 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); @@ -394,7 +394,7 @@ protected function validateExpirationDateLink(IShare $share) { 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()) { @@ -410,7 +410,7 @@ protected function validateExpirationDateLink(IShare $share) { } $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( From d3125403ff246e3e12432380b12b43adb6b95c2e Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 13 Aug 2026 14:05:17 +0200 Subject: [PATCH 2/2] test(sharing): resolve sharing timestamp issues in tests Signed-off-by: Ferdinand Thiessen --- apps/files_sharing/tests/ApiTest.php | 8 +-- .../Controller/ShareAPIControllerTest.php | 10 ++-- .../tests/SharesReminderJobTest.php | 9 ++- .../features/bootstrap/Sharing.php | 4 +- tests/lib/Share20/ManagerTest.php | 57 ++++++++++++------- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index a537540292e5f..f52fec5bc75eb 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1065,7 +1065,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()); @@ -1280,7 +1280,7 @@ public function testShareStorageMountPoint(): void { public 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())); @@ -1345,14 +1345,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 = \OC::$server->getURLGenerator()->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 4bb6c789da4a4..36c8729aa342f 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -765,7 +765,7 @@ public function dataGetShare() { $data[] = [$share, $expected]; // 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 = $this->createShare( 101, IShare::TYPE_LINK, @@ -799,7 +799,7 @@ public function dataGetShare() { '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, @@ -2089,7 +2089,7 @@ public function testCreateShareLinkPublicUploadFile(): void { $file = $this->createMock(File::class); $file->method('getId')->willReturn(42); $file->method('getStorage')->willReturn($storage); - + $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($file); $this->rootFolder->method('getById') @@ -4430,7 +4430,7 @@ public function dataFormatShare() { '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', @@ -4484,7 +4484,7 @@ public function dataFormatShare() { '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 d68a9d30182d3..0cfcaab4d27a6 100644 --- a/apps/files_sharing/tests/SharesReminderJobTest.php +++ b/apps/files_sharing/tests/SharesReminderJobTest.php @@ -98,12 +98,11 @@ public 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 6dc0e82cf0b30..dcc557ce5b0e8 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -308,7 +308,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) { @@ -607,7 +607,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/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 8fe63e23b812d..b6290e83ea7ea 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1153,7 +1153,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]); @@ -1188,7 +1188,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]); @@ -1239,7 +1239,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); @@ -1281,7 +1281,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); @@ -1325,7 +1325,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())); @@ -1365,7 +1365,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); @@ -1404,7 +1404,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; @@ -1433,7 +1433,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); @@ -1532,7 +1532,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]); @@ -1553,7 +1553,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]); @@ -1588,7 +1588,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); @@ -1617,7 +1617,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(); @@ -1654,7 +1654,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') @@ -1681,7 +1681,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(); @@ -1712,7 +1712,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(); @@ -1741,7 +1741,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())); @@ -1765,7 +1765,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); @@ -2425,7 +2425,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); @@ -2461,6 +2461,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()) @@ -2480,7 +2484,7 @@ public function testCreateShareUser(): void { public function testCreateShareGroup(): void { $manager = $this->createManagerMock() - ->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks']) + ->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2516,6 +2520,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()) @@ -2767,6 +2775,7 @@ public function testCreateShareHookError(): void { 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', + 'validateExpirationDateInternal', ]) ->getMock(); @@ -2803,6 +2812,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') @@ -2827,7 +2840,7 @@ public function testCreateShareHookError(): void { public function testCreateShareOfIncomingFederatedShare(): void { $manager = $this->createManagerMock() - ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2882,6 +2895,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())