From f91bc059e221149bf68443ec637f570ebe157915 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 022e33d6783db..47b98383d5868 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -234,7 +234,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 a8c7b827265f2..3f4ca814cb0a3 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -300,7 +300,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); @@ -319,7 +319,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; @@ -334,7 +334,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); @@ -378,7 +378,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); @@ -397,7 +397,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()) { @@ -413,7 +413,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 0535b103acb351c329529168bc3c9722f5cb8592 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 | 8 +-- .../Controller/ShareAPIControllerTest.php | 12 ++-- .../tests/SharesReminderJobTest.php | 9 ++- autotest.sh | 2 +- .../features/bootstrap/Sharing.php | 4 +- tests/lib/Share20/ManagerTest.php | 57 ++++++++++++------- 6 files changed, 54 insertions(+), 38 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 960f29224bb9e..ec98cf4154eca 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1091,7 +1091,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()); @@ -1306,7 +1306,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())); @@ -1371,14 +1371,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 65186f0b57151..98056e3e6950e 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -774,9 +774,9 @@ 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'); - $share = $this->createShare( - 101, + $expire = \DateTime::createFromFormat('Y-m-d H:i:s', '2000-01-02 23:59:59'); + $share = [ + '101', IShare::TYPE_LINK, null, 'initiatorId', @@ -808,7 +808,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, @@ -4481,7 +4481,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', @@ -4535,7 +4535,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 ce468e279ecc1..49bd921b21b90 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/autotest.sh b/autotest.sh index 32a844a670de8..a30931414c619 100755 --- a/autotest.sh +++ b/autotest.sh @@ -309,7 +309,7 @@ function execute_tests { if [ ! -z "$USEDOCKER" ] ; then echo "Fire up the postgres docker" DOCKER_CONTAINER_ID=$(docker run -e POSTGRES_DB="$DATABASENAME" -e POSTGRES_USER="$DATABASEUSER" -e POSTGRES_PASSWORD=owncloud -d postgres) - DATABASEHOST=$(docker inspect --format="{{.NetworkSettings.IPAddress}}" "$DOCKER_CONTAINER_ID") + DATABASEHOST=$(docker inspect --format="{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}" "$DOCKER_CONTAINER_ID") echo "Waiting for Postgres initialisation ..." diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index d93f114f27bdb..2cb377de1f005 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -309,7 +309,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) { @@ -610,7 +610,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 e0abc86306fbf..c316110ee3857 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1165,7 +1165,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]); @@ -1198,7 +1198,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]); @@ -1245,7 +1245,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); @@ -1285,7 +1285,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())); @@ -1363,7 +1363,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); @@ -1400,7 +1400,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; @@ -1427,7 +1427,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); @@ -1528,7 +1528,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]); @@ -1592,7 +1592,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); @@ -1625,7 +1625,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(); @@ -1662,7 +1662,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') @@ -1694,7 +1694,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(); @@ -1730,7 +1730,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(); @@ -1764,7 +1764,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())); @@ -1788,7 +1788,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); @@ -2452,7 +2452,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); @@ -2488,6 +2488,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()) @@ -2507,7 +2511,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); @@ -2543,6 +2547,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()) @@ -2776,6 +2784,7 @@ public function testCreateShareHookError(): void { 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', + 'validateExpirationDateInternal', ]) ->getMock(); @@ -2812,6 +2821,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') @@ -2836,7 +2849,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); @@ -2891,6 +2904,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())