Skip to content

fix(imip): don't report success when a calendar cannot process iMip - #62722

Open
kesselb wants to merge 1 commit into
masterfrom
fix/imip-calendar-capability-check
Open

fix(imip): don't report success when a calendar cannot process iMip#62722
kesselb wants to merge 1 commit into
masterfrom
fix/imip-calendar-capability-check

Conversation

@kesselb

@kesselb kesselb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

handleIMip() checked IHandleImipMessage only inside the match, so a writable calendar unable to process iMip returned true without doing anything.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@kesselb kesselb added this to the Nextcloud 35 milestone Jul 31, 2026
@kesselb kesselb self-assigned this Jul 31, 2026
@kesselb kesselb added bug 3. to review Waiting for reviews labels Jul 31, 2026
@kesselb
kesselb requested review from Altahrim, come-nc, icewind1991 and provokateurin and removed request for a team July 31, 2026 10:39
@kesselb
kesselb force-pushed the fix/imip-calendar-capability-check branch from 9311118 to d723df5 Compare July 31, 2026 10:43
@kesselb
kesselb force-pushed the fix/imip-calendar-capability-check branch from d723df5 to a57902c Compare July 31, 2026 10:49

@SebastianKrupinski SebastianKrupinski left a comment

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.

The logic here is completely wrong.

// use the primary calendar of the user, otherwise the first one that can process iMip messages
			$primaryCalendar = $this->getPrimaryCalendar($userId);
			$calendar = $primaryCalendar !== null && $this->canHandleImip($primaryCalendar)
				? $primaryCalendar
				: $userCalendars[0];

This assumes that the first user calendar is writable and can handle imip messages. This assumption is completely wrong, as the first calendar can be the "birthday calender" which is read only, this can event be a subscription, also getPrimaryCalendar() already returns the first user calendar when there is no default configured

@tcitworld

Copy link
Copy Markdown
Member

This assumes that the first user calendar is writable and can handle imip messages. This assumption is completely wrong, as the first calendar can be the "birthday calender" which is read only, this can event be a subscription, also getPrimaryCalendar() already returns the first user calendar when there is no default configured

$userCalendars are already filtered above through canHandleImip.

@kesselb

kesselb commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

also getPrimaryCalendar() already returns the first user calendar when there is no default configured

It does not?

handleIMip() checked IHandleImipMessage only inside the match, so a
writable calendar unable to process iMip returned true without doing
anything.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb
kesselb force-pushed the fix/imip-calendar-capability-check branch from a57902c to cf01026 Compare July 31, 2026 15:50
@kesselb kesselb changed the title fix(calendar): don't report success when a calendar cannot process iMip fix(imip): don't report success when a calendar cannot process iMip Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants