Skip to content

Conversation

@SebastianKrupinski
Copy link
Contributor

Summary

Do not load IMipPlugin before user is authenticated and user session is initilized


// calendar plugins
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Server::getRequest has been marked as deprecated

// calendar plugins
if ($this->requestIsForSubtree(['calendars', 'public-calendars', 'system-calendars', 'principals'])) {
$this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest(), \OC::$server->getConfig()));

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Server::getConfig has been marked as deprecated
\OC::$server->getCommentsManager(),
$userSession
));
if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OC\Server::getConfig has been marked as deprecated
\OC::$server->getCommentsManager(),
$userSession
));
if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') {

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\IConfig::getAppValue has been marked as deprecated
$senderName = $senderName->getValue() ?? null;
// Due to a bug in sabre, the senderName property for an iTIP message can actually also be a VObject Property
// If the iTIP message senderName is null or empty use the user session name as the senderName
if (($iTipMessage->senderName instanceof Parameter) && !empty(trim($iTipMessage->senderName->getValue()))) {

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of trim cannot be null, possibly null value provided
$senderName = $iTipMessage->senderName;
}
else {
$senderName = $this->userSession->getUser()->getDisplayName();
Copy link
Member

Choose a reason for hiding this comment

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

Might be an opportunity to trim $senderName afterwards for all cases.

$senderName = $iTipMessage->senderName;
}
else {
$senderName = $this->userSession->getUser()->getDisplayName();
Copy link
Member

Choose a reason for hiding this comment

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

Even though this should be safe when registered through dav server, it might not be when used through apps/dav/appinfo/v1/caldav.php (very legacy stuff, but still). I'd add a check to see if getUser() isn't null (and psalm would be happy).

@SebastianKrupinski
Copy link
Contributor Author

@tcitworld thank you for the input. I will apply your recommendations.

@tcitworld
Copy link
Member

OCA\DAV\Tests\unit\CalDAV\Schedule\IMipPluginTest needs to be changed with the new constructor.

Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Lint fix, otherwise ✅

// Due to a bug in sabre, the senderName property for an iTIP message can actually also be a VObject Property
// If the iTIP message senderName is null or empty use the user session name as the senderName
if (($iTipMessage->senderName instanceof Parameter) && !empty(trim($iTipMessage->senderName->getValue()))) {
$senderName = trim($iTipMessage->senderName->getValue());

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of trim cannot be null, possibly null value provided
} elseif (is_string($iTipMessage->senderName) && !empty(trim($iTipMessage->senderName))) {
$senderName = trim($iTipMessage->senderName);
} elseif ($this->userSession->getUser() !== null) {
$senderName = trim($this->userSession->getUser()->getDisplayName());

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getDisplayName on possibly null value
@SebastianKrupinski SebastianKrupinski merged commit c17fcc4 into master May 4, 2024
@SebastianKrupinski SebastianKrupinski deleted the fix/issue-45081 branch May 4, 2024 13:36
@welcome
Copy link

welcome bot commented May 4, 2024

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@SebastianKrupinski SebastianKrupinski added this to the Nextcloud 30 milestone Jul 23, 2024
@blizzz blizzz mentioned this pull request Jul 24, 2024
@susnux
Copy link
Contributor

susnux commented Sep 2, 2024

@SebastianKrupinski could we backport this to stable29 ?

@SebastianKrupinski
Copy link
Contributor Author

@SebastianKrupinski could we backport this to stable29 ?

I have no issue back porting it... Let me just ask if there is any reason we shoudn't

@ChristophWurst
Copy link
Member

Go :shipit:

@SebastianKrupinski
Copy link
Contributor Author

/backport to stable29

@SebastianKrupinski
Copy link
Contributor Author

/backport to stable28

@nextcloud nextcloud deleted a comment from backportbot bot Sep 2, 2024
@nextcloud nextcloud deleted a comment from backportbot bot Sep 2, 2024
@SebastianKrupinski
Copy link
Contributor Author

Backport for 29 #47790

Backport for 28 #47791

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: From Name is not setting properly in Calendar invitations created from Thunderbird

5 participants