Skip to content

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

Merged
kesselb merged 1 commit into
masterfrom
fix/imip-calendar-capability-check
Aug 3, 2026
Merged

fix(imip): don't report success when a calendar cannot process iMip#62722
kesselb merged 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
@SebastianKrupinski

Copy link
Copy Markdown
Contributor

$userCalendars are already filtered above through canHandleImip.

Ah. you're right I missed the change on the $usersCalendars at the beginning.

@kesselb
kesselb merged commit a5361d4 into master Aug 3, 2026
199 checks passed
@kesselb
kesselb deleted the fix/imip-calendar-capability-check branch August 3, 2026 15:18
@kesselb

kesselb commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

/backport to stable34

@kesselb

kesselb commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

/backport to stable33

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