Skip to content

Commit e6a6001

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #58712 from nextcloud/backport/58128/stable29
[stable29] fix(share): Set expiration time to end of day (23:59:59)
2 parents 53039f7 + 6d42f9f commit e6a6001

6 files changed

Lines changed: 57 additions & 40 deletions

File tree

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ protected function formatShare(IShare $share, ?Node $recipientNode = null): arra
225225
$expiration = $share->getExpirationDate();
226226
if ($expiration !== null) {
227227
$expiration->setTimezone($this->dateTimeZone->getTimeZone());
228-
$result['expiration'] = $expiration->format('Y-m-d 00:00:00');
228+
$result['expiration'] = $expiration->format('Y-m-d H:i:s');
229229
}
230230

231231
$currentUserPermissions = $recipientNode?->getPermissions() ?? Constants::PERMISSION_ALL;

‎apps/files_sharing/tests/ApiTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ public function testUpdateShareExpireDate() {
10771077
$share1 = $this->shareManager->getShareById($share1->getFullId());
10781078

10791079
// date should be changed
1080-
$dateWithinRange->setTime(0, 0, 0);
1080+
$dateWithinRange->setTime(23, 59, 59);
10811081
$dateWithinRange->setTimezone(new \DateTimeZone(date_default_timezone_get()));
10821082
$this->assertEquals($dateWithinRange, $share1->getExpirationDate());
10831083

@@ -1292,7 +1292,7 @@ public function testShareStorageMountPoint() {
12921292

12931293
public function datesProvider() {
12941294
$date = new \DateTime();
1295-
$date->setTime(0, 0);
1295+
$date->setTime(23, 59, 59);
12961296
$date->add(new \DateInterval('P5D'));
12971297
$date->setTimezone(new \DateTimeZone(date_default_timezone_get()));
12981298

@@ -1357,14 +1357,14 @@ public function testCreatePublicLinkExpireDateValid() {
13571357

13581358
$data = $result->getData();
13591359
$this->assertTrue(is_string($data['token']));
1360-
$this->assertEquals($date->format('Y-m-d 00:00:00'), $data['expiration']);
1360+
$this->assertEquals($date->format('Y-m-d 23:59:59'), $data['expiration']);
13611361

13621362
// check for correct link
13631363
$url = \OC::$server->getURLGenerator()->getAbsoluteURL('/index.php/s/' . $data['token']);
13641364
$this->assertEquals($url, $data['url']);
13651365

1366-
$share = $this->shareManager->getShareById('ocinternal:'.$data['id']);
1367-
$date->setTime(0, 0, 0);
1366+
$share = $this->shareManager->getShareById('ocinternal:' . $data['id']);
1367+
$date->setTime(23, 59, 59);
13681368
$this->assertEquals($date, $share->getExpirationDate());
13691369

13701370
$this->shareManager->deleteShare($share);

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ public function dataGetShare() {
711711
$data[] = [$share, $expected];
712712

713713
// File shared by link with Expire
714-
$expire = \DateTime::createFromFormat('Y-m-d h:i:s', '2000-01-02 01:02:03');
714+
$expire = \DateTime::createFromFormat('Y-m-d H:i:s', '2000-01-02 23:59:59');
715715
$share = $this->createShare(
716716
101,
717717
IShare::TYPE_LINK,
@@ -745,7 +745,7 @@ public function dataGetShare() {
745745
'file_target' => 'target',
746746
'file_parent' => 3,
747747
'token' => 'token',
748-
'expiration' => '2000-01-02 00:00:00',
748+
'expiration' => '2000-01-02 23:59:59',
749749
'permissions' => 4,
750750
'attributes' => null,
751751
'stime' => 5,
@@ -2019,7 +2019,7 @@ public function testCreateShareLinkPublicUploadFile(): void {
20192019
$file = $this->createMock(File::class);
20202020
$file->method('getId')->willReturn(42);
20212021
$file->method('getStorage')->willReturn($storage);
2022-
2022+
20232023
$this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf();
20242024
$this->rootFolder->method('get')->with('valid-path')->willReturn($file);
20252025
$this->rootFolder->method('getById')
@@ -4290,7 +4290,7 @@ public function dataFormatShare() {
42904290
'permissions' => 1,
42914291
'stime' => 946684862,
42924292
'parent' => null,
4293-
'expiration' => '2001-02-03 00:00:00',
4293+
'expiration' => '2001-02-03 04:05:06',
42944294
'token' => null,
42954295
'uid_file_owner' => 'owner',
42964296
'displayname_file_owner' => 'owner',
@@ -4343,7 +4343,7 @@ public function dataFormatShare() {
43434343
'permissions' => 1,
43444344
'stime' => 946684862,
43454345
'parent' => null,
4346-
'expiration' => '2001-02-03 00:00:00',
4346+
'expiration' => '2001-02-03 04:05:06',
43474347
'token' => null,
43484348
'uid_file_owner' => 'owner',
43494349
'displayname_file_owner' => 'owner',

‎build/integration/features/bootstrap/Sharing.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,8 @@ public function createShare($user,
330330
public function isFieldInResponse($field, $contentExpected) {
331331
$data = simplexml_load_string($this->response->getBody())->data[0];
332332
if ((string)$field == 'expiration') {
333-
if(!empty($contentExpected)) {
334-
$contentExpected = date('Y-m-d', strtotime($contentExpected)) . " 00:00:00";
333+
if (!empty($contentExpected)) {
334+
$contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 23:59:59';
335335
}
336336
}
337337
if (count($data->element) > 0) {
@@ -630,7 +630,7 @@ private function assertFieldIsInReturnedShare(string $field, string $contentExpe
630630
}
631631

632632
if ($field === 'expiration' && !empty($contentExpected)) {
633-
$contentExpected = date('Y-m-d', strtotime($contentExpected)) . " 00:00:00";
633+
$contentExpected = date('Y-m-d', strtotime($contentExpected)) . ' 23:59:59';
634634
}
635635

636636
if ($contentExpected === 'A_NUMBER') {

‎lib/private/Share20/Manager.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ protected function validateExpirationDateInternal(IShare $share) {
327327
if(!$share->getNoExpirationDate() || $isEnforced) {
328328
if ($expirationDate !== null) {
329329
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
330-
$expirationDate->setTime(0, 0, 0);
330+
$expirationDate->setTime(23, 59, 59);
331331

332332
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
333333
$date->setTime(0, 0, 0);
@@ -347,7 +347,7 @@ protected function validateExpirationDateInternal(IShare $share) {
347347

348348
if ($fullId === null && $expirationDate === null && $defaultExpireDate) {
349349
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
350-
$expirationDate->setTime(0, 0, 0);
350+
$expirationDate->setTime(23, 59, 59);
351351
$days = (int)$this->config->getAppValue('core', $configProp, (string)$defaultExpireDays);
352352
if ($days > $defaultExpireDays) {
353353
$days = $defaultExpireDays;
@@ -362,7 +362,7 @@ protected function validateExpirationDateInternal(IShare $share) {
362362
}
363363

364364
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
365-
$date->setTime(0, 0, 0);
365+
$date->setTime(23, 59, 59);
366366
$date->add(new \DateInterval('P' . $defaultExpireDays . 'D'));
367367
if ($date < $expirationDate) {
368368
$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) {
407407
if(!($share->getNoExpirationDate() && !$isEnforced)) {
408408
if ($expirationDate !== null) {
409409
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());
410-
$expirationDate->setTime(0, 0, 0);
410+
$expirationDate->setTime(23, 59, 59);
411411

412412
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
413413
$date->setTime(0, 0, 0);
@@ -427,7 +427,7 @@ protected function validateExpirationDateLink(IShare $share) {
427427

428428
if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
429429
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
430-
$expirationDate->setTime(0, 0, 0);
430+
$expirationDate->setTime(23, 59, 59);
431431

432432
$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());
433433
if ($days > $this->shareApiLinkDefaultExpireDays()) {
@@ -443,7 +443,7 @@ protected function validateExpirationDateLink(IShare $share) {
443443
}
444444

445445
$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());
446-
$date->setTime(0, 0, 0);
446+
$date->setTime(23, 59, 59);
447447
$date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D'));
448448
if ($date < $expirationDate) {
449449
$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());

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,7 +1164,7 @@ public function testValidateExpirationDateInternalEnforceButNotSetNewShare($shar
11641164
}
11651165

11661166
$expected = new \DateTime('now', $this->timezone);
1167-
$expected->setTime(0, 0, 0);
1167+
$expected->setTime(23, 59, 59);
11681168
$expected->add(new \DateInterval('P3D'));
11691169

11701170
self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]);
@@ -1199,7 +1199,7 @@ public function testValidateExpirationDateInternalEnforceRelaxedDefaultButNotSet
11991199
}
12001200

12011201
$expected = new \DateTime('now', $this->timezone);
1202-
$expected->setTime(0, 0, 0);
1202+
$expected->setTime(23, 59, 59);
12031203
$expected->add(new \DateInterval('P1D'));
12041204

12051205
self::invokePrivate($this->manager, 'validateExpirationDateInternal', [$share]);
@@ -1250,7 +1250,7 @@ public function testValidateExpirationDateInternalEnforceValid($shareType) {
12501250
$future->setTime(1, 2, 3);
12511251

12521252
$expected = clone $future;
1253-
$expected->setTime(0, 0, 0);
1253+
$expected->setTime(23, 59, 59);
12541254

12551255
$share = $this->manager->newShare();
12561256
$share->setShareType($shareType);
@@ -1292,7 +1292,7 @@ public function testValidateExpirationDateInternalNoDefault($shareType) {
12921292
$date->setTime(1, 2, 3);
12931293

12941294
$expected = clone $date;
1295-
$expected->setTime(0, 0, 0);
1295+
$expected->setTime(23, 59, 59);
12961296

12971297
$share = $this->manager->newShare();
12981298
$share->setShareType($shareType);
@@ -1336,7 +1336,7 @@ public function testValidateExpirationDateInternalNoDateDefault($shareType) {
13361336
$share->setShareType($shareType);
13371337

13381338
$expected = new \DateTime('now', $this->timezone);
1339-
$expected->setTime(0, 0);
1339+
$expected->setTime(23, 59, 59);
13401340
$expected->add(new \DateInterval('P3D'));
13411341
$expected->setTimezone(new \DateTimeZone(date_default_timezone_get()));
13421342

@@ -1376,7 +1376,7 @@ public function testValidateExpirationDateInternalDefault($shareType) {
13761376
$future->setTime(1, 2, 3);
13771377

13781378
$expected = clone $future;
1379-
$expected->setTime(0, 0);
1379+
$expected->setTime(23, 59, 59);
13801380

13811381
$share = $this->manager->newShare();
13821382
$share->setShareType($shareType);
@@ -1415,7 +1415,7 @@ public function testValidateExpirationDateInternalDefault($shareType) {
14151415
public function testValidateExpirationDateInternalHookModification($shareType) {
14161416
$nextWeek = new \DateTime('now', $this->timezone);
14171417
$nextWeek->add(new \DateInterval('P7D'));
1418-
$nextWeek->setTime(0, 0, 0);
1418+
$nextWeek->setTime(23, 59, 59);
14191419

14201420
$save = clone $nextWeek;
14211421

@@ -1444,7 +1444,7 @@ public function testValidateExpirationDateInternalHookException($shareType) {
14441444

14451445
$nextWeek = new \DateTime();
14461446
$nextWeek->add(new \DateInterval('P7D'));
1447-
$nextWeek->setTime(0, 0, 0);
1447+
$nextWeek->setTime(23, 59, 59);
14481448

14491449
$share = $this->manager->newShare();
14501450
$share->setShareType($shareType);
@@ -1543,7 +1543,7 @@ public function testValidateExpirationDateEnforceButNotSetNewShare() {
15431543
]);
15441544

15451545
$expected = new \DateTime('now', $this->timezone);
1546-
$expected->setTime(0, 0, 0);
1546+
$expected->setTime(23, 59, 59);
15471547
$expected->add(new \DateInterval('P3D'));
15481548

15491549
self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
@@ -1564,7 +1564,7 @@ public function testValidateExpirationDateEnforceRelaxedDefaultButNotSetNewShare
15641564
]);
15651565

15661566
$expected = new \DateTime('now', $this->timezone);
1567-
$expected->setTime(0, 0, 0);
1567+
$expected->setTime(23, 59, 59);
15681568
$expected->add(new \DateInterval('P1D'));
15691569

15701570
self::invokePrivate($this->manager, 'validateExpirationDateLink', [$share]);
@@ -1599,7 +1599,7 @@ public function testValidateExpirationDateEnforceValid() {
15991599
$future->setTime(1, 2, 3);
16001600

16011601
$expected = clone $future;
1602-
$expected->setTime(0, 0, 0);
1602+
$expected->setTime(23, 59, 59);
16031603

16041604
$share = $this->manager->newShare();
16051605
$share->setExpirationDate($future);
@@ -1628,7 +1628,7 @@ public function testValidateExpirationDateNoDefault() {
16281628
$date->setTime(1, 2, 3);
16291629

16301630
$expected = clone $date;
1631-
$expected->setTime(0, 0);
1631+
$expected->setTime(23, 59, 59);
16321632
$expected->setTimezone(new \DateTimeZone(date_default_timezone_get()));
16331633

16341634
$share = $this->manager->newShare();
@@ -1665,7 +1665,7 @@ public function testValidateExpirationDateNoDateDefault() {
16651665

16661666
$expected = new \DateTime('now', $this->timezone);
16671667
$expected->add(new \DateInterval('P3D'));
1668-
$expected->setTime(0, 0);
1668+
$expected->setTime(23, 59, 59);
16691669
$expected->setTimezone(new \DateTimeZone(date_default_timezone_get()));
16701670

16711671
$this->config->method('getAppValue')
@@ -1692,7 +1692,7 @@ public function testValidateExpirationDateDefault() {
16921692
$future->setTime(1, 2, 3);
16931693

16941694
$expected = clone $future;
1695-
$expected->setTime(0, 0);
1695+
$expected->setTime(23, 59, 59);
16961696
$expected->setTimezone(new \DateTimeZone(date_default_timezone_get()));
16971697

16981698
$share = $this->manager->newShare();
@@ -1723,7 +1723,7 @@ public function testValidateExpirationNegativeOffsetTimezone() {
17231723

17241724
$expected = clone $future;
17251725
$expected->setTimezone($this->timezone);
1726-
$expected->setTime(0, 0);
1726+
$expected->setTime(23, 59, 59);
17271727
$expected->setTimezone(new \DateTimeZone(date_default_timezone_get()));
17281728

17291729
$share = $this->manager->newShare();
@@ -1752,7 +1752,7 @@ public function testValidateExpirationDateHookModification() {
17521752
$nextWeek->add(new \DateInterval('P7D'));
17531753

17541754
$save = clone $nextWeek;
1755-
$save->setTime(0, 0);
1755+
$save->setTime(23, 59, 59);
17561756
$save->sub(new \DateInterval('P2D'));
17571757
$save->setTimezone(new \DateTimeZone(date_default_timezone_get()));
17581758

@@ -1776,7 +1776,7 @@ public function testValidateExpirationDateHookException() {
17761776

17771777
$nextWeek = new \DateTime();
17781778
$nextWeek->add(new \DateInterval('P7D'));
1779-
$nextWeek->setTime(0, 0, 0);
1779+
$nextWeek->setTime(23, 59, 59);
17801780

17811781
$share = $this->manager->newShare();
17821782
$share->setExpirationDate($nextWeek);
@@ -2436,7 +2436,7 @@ public function testCanShare($expected, $sharingEnabled, $disabledForUser) {
24362436
public function testCreateShareUser(): void {
24372437
/** @var Manager|MockObject $manager */
24382438
$manager = $this->createManagerMock()
2439-
->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
2439+
->onlyMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal'])
24402440
->getMock();
24412441

24422442
$shareOwner = $this->createMock(IUser::class);
@@ -2472,6 +2472,10 @@ public function testCreateShareUser(): void {
24722472
$manager->expects($this->once())
24732473
->method('pathCreateChecks')
24742474
->with($path);
2475+
$manager->expects($this->once())
2476+
->method('validateExpirationDateInternal')
2477+
->with($share)
2478+
->willReturnArgument(0);
24752479

24762480
$this->defaultProvider
24772481
->expects($this->once())
@@ -2491,7 +2495,7 @@ public function testCreateShareUser(): void {
24912495

24922496
public function testCreateShareGroup() {
24932497
$manager = $this->createManagerMock()
2494-
->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks'])
2498+
->setMethods(['canShare', 'generalCreateChecks', 'groupCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal'])
24952499
->getMock();
24962500

24972501
$shareOwner = $this->createMock(IUser::class);
@@ -2527,6 +2531,10 @@ public function testCreateShareGroup() {
25272531
$manager->expects($this->once())
25282532
->method('pathCreateChecks')
25292533
->with($path);
2534+
$manager->expects($this->once())
2535+
->method('validateExpirationDateInternal')
2536+
->with($share)
2537+
->willReturnArgument(0);
25302538

25312539
$this->defaultProvider
25322540
->expects($this->once())
@@ -2778,6 +2786,7 @@ public function testCreateShareHookError() {
27782786
'generalCreateChecks',
27792787
'userCreateChecks',
27802788
'pathCreateChecks',
2789+
'validateExpirationDateInternal',
27812790
])
27822791
->getMock();
27832792

@@ -2814,6 +2823,10 @@ public function testCreateShareHookError() {
28142823
$manager->expects($this->once())
28152824
->method('pathCreateChecks')
28162825
->with($path);
2826+
$manager->expects($this->once())
2827+
->method('validateExpirationDateInternal')
2828+
->with($share)
2829+
->willReturnArgument(0);
28172830

28182831
$share->expects($this->once())
28192832
->method('setShareOwner')
@@ -2838,7 +2851,7 @@ public function testCreateShareHookError() {
28382851

28392852
public function testCreateShareOfIncomingFederatedShare() {
28402853
$manager = $this->createManagerMock()
2841-
->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
2854+
->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks', 'validateExpirationDateInternal'])
28422855
->getMock();
28432856

28442857
$shareOwner = $this->createMock(IUser::class);
@@ -2893,6 +2906,10 @@ public function testCreateShareOfIncomingFederatedShare() {
28932906
$manager->expects($this->once())
28942907
->method('pathCreateChecks')
28952908
->with($path);
2909+
$manager->expects($this->once())
2910+
->method('validateExpirationDateInternal')
2911+
->with($share)
2912+
->willReturnArgument(0);
28962913

28972914
$this->defaultProvider
28982915
->expects($this->once())

0 commit comments

Comments
 (0)