From 2d20ee2e4e1ec61cf39cffad85eafc1cc5585b70 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 | 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 8ed52a1de41..0f1ad3bd5d5 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -236,7 +236,7 @@ class ShareAPIController extends OCSController { $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 1d72ec63cb0..586a1ff9c0d 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -302,7 +302,7 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { } $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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { 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 @@ class Manager implements IManager { } $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 ce0d97b6e5803e47ea095b2cf219e766e4f7dc2f 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 --- apps/files_sharing/tests/ApiTest.php | 8 +-- .../Controller/ShareAPIControllerTest.php | 8 +-- .../tests/SharesReminderJobTest.php | 9 ++- .../features/bootstrap/Sharing.php | 4 +- tests/lib/Share20/ManagerTest.php | 57 ++++++++++++------- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 713af803ab2..e0340b3ce00 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1052,7 +1052,7 @@ class ApiTest extends TestCase { $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 @@ class ApiTest extends TestCase { 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 @@ class ApiTest extends TestCase { $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 2fe2281891a..28caea90cea 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -773,7 +773,7 @@ class ShareAPIControllerTest extends TestCase { $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 @@ class ShareAPIControllerTest extends TestCase { '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 @@ class ShareAPIControllerTest extends TestCase { '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 @@ class ShareAPIControllerTest extends TestCase { '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 475d418f1e2..641d9a5295f 100644 --- a/apps/files_sharing/tests/SharesReminderJobTest.php +++ b/apps/files_sharing/tests/SharesReminderJobTest.php @@ -99,12 +99,11 @@ class SharesReminderJobTest extends \Test\TestCase { $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 bd12f1f2455..72f6902af16 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -310,7 +310,7 @@ trait Sharing { $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 @@ trait Sharing { } 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 2afb5e6c3b3..ba8dfe1442f 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1199,7 +1199,7 @@ class ManagerTest extends \Test\TestCase { } $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 @@ class ManagerTest extends \Test\TestCase { } $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { ]); $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 @@ class ManagerTest extends \Test\TestCase { ]); $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', + 'validateExpirationDateInternal', ]) ->getMock(); @@ -2841,6 +2850,10 @@ class ManagerTest extends \Test\TestCase { $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 @@ class ManagerTest extends \Test\TestCase { 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 @@ class ManagerTest extends \Test\TestCase { $manager->expects($this->once()) ->method('pathCreateChecks') ->with($path); + $manager->expects($this->once()) + ->method('validateExpirationDateInternal') + ->with($share) + ->willReturnArgument(0); $this->defaultProvider ->expects($this->once())