Skip to content

[stable28] Respect empty expiryDate value in server - #45482

Closed
backportbot[bot] wants to merge 3 commits into
stable28from
backport/44485/stable28
Closed

[stable28] Respect empty expiryDate value in server#45482
backportbot[bot] wants to merge 3 commits into
stable28from
backport/44485/stable28

Conversation

@backportbot

@backportbotbackportbotBot commented May 23, 2024•
edited by nfebe
Loading

Copy link
Copy Markdown

Backport of #44485

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Amend HEAD commit to remove the line stating to skip CI

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot
backportbotBot requested review from artonge and nfebeMay 23, 2024 14:42
@backportbotbackportbotBot added this to the Nextcloud 28.0.6 milestone May 23, 2024
@nfebe
nfebe marked this pull request as ready for review May 23, 2024 15:27
@nfebe
nfebe requested a review from provokateurin as a code ownerMay 23, 2024 15:28
@nfebe
nfebe enabled auto-merge May 23, 2024 15:28
github-advanced-security[bot]
github-advanced-securityAI found potential problems May 23, 2024
View reviewed changes
Comment threadlib/private/Share20/Share.php
* @inheritdoc
*/
public function setNoExpirationDate(bool $noExpirationDate) {
$this->noExpirationDate = $noExpirationDate;

Check failure

Code scanning / Psalm

UndefinedThisPropertyAssignment

Instance property OC\Share20\Share::$noExpirationDate is not defined
Comment threadlib/private/Share20/Share.php
* @inheritdoc
*/
public function getNoExpirationDate(): bool {
return $this->noExpirationDate;

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Share::$noExpirationDate is not defined
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php
* @param string $password Password for the share
* @param string|null $sendPasswordByTalk Send the password for the share over Talk
* @param string $expireDate Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.
* @param ?string $expireDate The expiry date of the share in the user's timezone (UTC) at 00:00.

Check failure

Code scanning / Psalm

MismatchingDocblockParamType

Parameter $expireDate has wrong type 'null|string', should be 'string'
Comment threadlib/private/Share20/Manager.php
$this->knownUserService = $knownUserService;
$this->shareDisableChecker = $shareDisableChecker;
$this->dateTimeZone = $dateTimeZone;
$this->legacyHooks = new LegacyHooks($this->dispatcher);

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dispatcher is not defined
Comment threadlib/private/Share20/Manager.php
// Then skip expiration date validation as null is accepted
if(!($share->getNoExpirationDate() && !$isEnforced)) {
if ($expirationDate != null) {
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dateTimeZone is not defined
Comment threadlib/private/Share20/Manager.php
throw new \InvalidArgumentException('Expiration date is enforced');
}

$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dateTimeZone is not defined
Comment threadlib/private/Share20/Manager.php
// Then skip expiration date validation as null is accepted
if(!($share->getNoExpirationDate() && !$isEnforced)) {
if ($expirationDate !== null) {
$expirationDate->setTimezone($this->dateTimeZone->getTimeZone());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dateTimeZone is not defined
Comment threadlib/private/Share20/Manager.php
}

if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dateTimeZone is not defined
Comment threadlib/private/Share20/Manager.php
$expirationDate = new \DateTime('now', $this->dateTimeZone->getTimeZone());
$expirationDate->setTime(0, 0, 0);

$days = (int)$this->config->getAppValue('core', 'link_defaultExpDays', (string)$this->shareApiLinkDefaultExpireDays());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$config is not defined
Comment threadlib/private/Share20/Manager.php
throw new \InvalidArgumentException('Expiration date is enforced');
}

$date = new \DateTime('now', $this->dateTimeZone->getTimeZone());

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch

Instance property OC\Share20\Manager::$dateTimeZone is not defined
@AndyScherzinger
AndyScherzingerforce-pushed the backport/44485/stable28 branch from bcd8482 to a81833aCompareMay 27, 2024 10:05
Antreesy
Antreesy reviewed May 31, 2024
View reviewed changes
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php
nfebe added 2 commits May 31, 2024 11:15
@nfebe
fix(shareManager): Respect empty expireDate in server
89e8d3e
If `expireDate` is an empty string and not `null` then the server should not set a default.
Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@nfebe
refactor(shareManager): Use constructor property promotion
3856111
Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@nfebe
nfebeforce-pushed the backport/44485/stable28 branch from a81833a to 88b84b8CompareMay 31, 2024 10:16
@nfebe
nfebe requested a review from AntreesyMay 31, 2024 10:16
@nfebe
test(Sharing): Integration test for no expiration set date for share
4a583bc
- Verify that explicitly sending empty `expireDate` param to server overwrite default
and sets not expiry date, if non is enforced.
- Update tests to avoid converting empty string to date.
Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
@nfebe
nfebeforce-pushed the backport/44485/stable28 branch from 88b84b8 to 4a583bcCompareMay 31, 2024 10:17
Antreesy
Antreesy reviewed May 31, 2024
View reviewed changes
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php
* @param string $password Password for the share
* @param string|null $sendPasswordByTalk Send the password for the share over Talk
* @param string $expireDate Expiry date of the share using user timezone at 00:00. It means date in UTC timezone will be used.
* @param ?string $expireDate The expiry date of the share in the user's timezone (UTC) at 00:00.

@AntreesyAntreesyMay 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L612 missing?

?string $expireDate = null,

And L793?

if ($expireDate !== null) {
if ($expireDate !== '') {

Comment threadlib/private/Share20/Manager.php
private LegacyHooks $legacyHooks;

public function __construct(
LoggerInterface $logger,

@AntreesyAntreesyMay 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LoggerInterface $logger,
privateLoggerInterface $logger,

and further?

Comment threadlib/private/Share20/Share.php

/** @var bool */
private $hideDownload = false;

@AntreesyAntreesyMay 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private bool $noExpirationDate = false;

@nfebe

nfebe commented May 31, 2024

Copy link
Copy Markdown
Contributor

Closing in favor of #45604

stable29 had similar backport issues, fixed there and backported via stable29

@nfebenfebe closed this May 31, 2024
auto-merge was automatically disabled May 31, 2024 13:09

Pull request was closed

@solracsf
solracsf deleted the backport/44485/stable28 branch June 1, 2024 08:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AntreesyAntreesyAntreesy left review comments
@nfebenfebenfebe approved these changes
@artongeartongeAwaiting requested review from artonge
@provokateurinprovokateurinAwaiting requested review from provokateurinprovokateurin is a code owner

Assignees

@nfebenfebe

Labels

3. to reviewWaiting for reviews27-feedback28-feedbackfeature: sharingfeedback-requested

Projects

None yet

Milestone

Nextcloud 28.0.7

Development

Successfully merging this pull request may close these issues.

4 participants

@nfebe@github-advanced-security@Antreesy@AndyScherzinger