From 9f9c2d1bf1ec75e56e52e03b07e927e0672332d4 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 [skip ci] --- .../lib/Controller/ShareAPIController.php | 2 +- lib/private/Share20/Manager.php | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 247a86361201c..5de3794015071 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -225,7 +225,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/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index c7a42bbbf02e9..af82cd7fefd50 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -327,7 +327,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); @@ -347,7 +347,7 @@ protected function validateExpirationDateInternal(IShare $share) { 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; @@ -362,7 +362,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) { $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); @@ -407,7 +407,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); @@ -427,7 +427,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()) { @@ -443,7 +443,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) { $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()); From 75c43e2a76efbfc329b9db7d17daf784be5cab94 Mon Sep 17 00:00:00 2001 From: nfebe Date: Fri, 6 Feb 2026 00:46:22 +0100 Subject: [PATCH 2/2] test(share): Update expiration date tests for end-of-day time Update expected values in ManagerTest to reflect the new behavior where share expiration dates are set to 23:59:59 instead of 00:00:00. Signed-off-by: nfebe Signed-off-by: Carl Schwan --- apps/files_sharing/tests/ApiTest.php | 10 ++-- .../Controller/ShareAPIControllerTest.php | 10 ++-- .../features/bootstrap/Sharing.php | 6 +- tests/lib/Share20/ManagerTest.php | 57 ++++++++++++------- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 1dc52d9f58674..87c85f92bd1bc 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1077,7 +1077,7 @@ public function testUpdateShareExpireDate() { $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()); @@ -1292,7 +1292,7 @@ public function testShareStorageMountPoint() { 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())); @@ -1357,14 +1357,14 @@ public function testCreatePublicLinkExpireDateValid() { $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); + $share = $this->shareManager->getShareById('ocinternal:' . $data['id']); + $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 02da6098d283d..d0996909ebe06 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -711,7 +711,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, @@ -745,7 +745,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, @@ -2019,7 +2019,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') @@ -4290,7 +4290,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', @@ -4343,7 +4343,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/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index c887bc25e9873..e809279002bc8 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -330,8 +330,8 @@ public function createShare($user, 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"; + if (!empty($contentExpected)) { + $contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 23:59:59'; } } if (count($data->element) > 0) { @@ -630,7 +630,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 e69afda4ada9c..931f8c2f98ca2 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1164,7 +1164,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]); @@ -1199,7 +1199,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]); @@ -1250,7 +1250,7 @@ public function testValidateExpirationDateInternalEnforceValid($shareType) { $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); @@ -1292,7 +1292,7 @@ public function testValidateExpirationDateInternalNoDefault($shareType) { $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); @@ -1336,7 +1336,7 @@ public function testValidateExpirationDateInternalNoDateDefault($shareType) { $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())); @@ -1376,7 +1376,7 @@ public function testValidateExpirationDateInternalDefault($shareType) { $future->setTime(1, 2, 3); $expected = clone $future; - $expected->setTime(0, 0); + $expected->setTime(23, 59, 59); $share = $this->manager->newShare(); $share->setShareType($shareType); @@ -1415,7 +1415,7 @@ public function testValidateExpirationDateInternalDefault($shareType) { public function testValidateExpirationDateInternalHookModification($shareType) { $nextWeek = new \DateTime('now', $this->timezone); $nextWeek->add(new \DateInterval('P7D')); - $nextWeek->setTime(0, 0, 0); + $nextWeek->setTime(23, 59, 59); $save = clone $nextWeek; @@ -1444,7 +1444,7 @@ public function testValidateExpirationDateInternalHookException($shareType) { $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); @@ -1543,7 +1543,7 @@ public function testValidateExpirationDateEnforceButNotSetNewShare() { ]); $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]); @@ -1564,7 +1564,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]); @@ -1599,7 +1599,7 @@ public function testValidateExpirationDateEnforceValid() { $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); @@ -1628,7 +1628,7 @@ public function testValidateExpirationDateNoDefault() { $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(); @@ -1665,7 +1665,7 @@ public function testValidateExpirationDateNoDateDefault() { $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') @@ -1692,7 +1692,7 @@ public function testValidateExpirationDateDefault() { $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(); @@ -1723,7 +1723,7 @@ public function testValidateExpirationNegativeOffsetTimezone() { $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(); @@ -1752,7 +1752,7 @@ public function testValidateExpirationDateHookModification() { $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())); @@ -1776,7 +1776,7 @@ public function testValidateExpirationDateHookException() { $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); @@ -2436,7 +2436,7 @@ public function testCanShare($expected, $sharingEnabled, $disabledForUser) { 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); @@ -2472,6 +2472,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()) @@ -2491,7 +2495,7 @@ public function testCreateShareUser(): void { public function testCreateShareGroup() { $manager = $this->createManagerMock() - ->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks']) + ->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2527,6 +2531,10 @@ public function testCreateShareGroup() { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once()) @@ -2778,6 +2786,7 @@ public function testCreateShareHookError() { 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', + 'validateExpirationDateInternal', ]) ->getMock(); @@ -2814,6 +2823,10 @@ public function testCreateShareHookError() { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $share->expects($this->once()) ->method('setShareOwner') @@ -2838,7 +2851,7 @@ public function testCreateShareHookError() { public function testCreateShareOfIncomingFederatedShare() { $manager = $this->createManagerMock() - ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks']) + ->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal']) ->getMock(); $shareOwner = $this->createMock(IUser::class); @@ -2893,6 +2906,10 @@ public function testCreateShareOfIncomingFederatedShare() { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once())