fix: prevent double timezone conversion for CalDAV events from Zimbra (#27877)#28026
Open
kiro-dev28 wants to merge 1 commit intocalcom:mainfrom
Open
fix: prevent double timezone conversion for CalDAV events from Zimbra (#27877)#28026kiro-dev28 wants to merge 1 commit intocalcom:mainfrom
kiro-dev28 wants to merge 1 commit intocalcom:mainfrom
Conversation
…calcom#27877) Root cause: ical.js resolves DTSTART;TZID=... into a zone-aware ICAL.Time. Calling convertToZone() on an already-zoned time re-interprets the local value as UTC first, then applies the offset a second time. For a 09:00 Europe/Berlin event (UTC+1) this produces: convertToZone step 1: treat 09:00 as UTC → subtract 1h offset → 08:00 convertToZone step 2: apply +1h offset again → 10:00 ← wrong The 15-min slot then shows start=10:00, end=09:00 (unchanged) = -1h duration. Fix: 1. Guard convertToZone with isFloating() — only call it when ical.js could not resolve a timezone (floating time), not when it already produced a zone-aware time. This preserves the existing behaviour for iCal feeds without VTIMEZONE (where the time IS floating and conversion is needed). 2. Improve VTIMEZONE lookup for vendor-prefixed TZIDs (Zimbra uses /zimbra.com/standard/Europe/Berlin; Mozilla uses /mozilla.org/20070129_1/Europe/Berlin). If the exact-match lookup fails, extract the last path segment and retry — this matches the corresponding VTIMEZONE whose TZID is the plain IANA name. Tests added in CalendarService.test.ts: - Non-recurring event with standard VTIMEZONE: verifies start=08:00 UTC (09:00 Berlin), end > start (positive duration). - Zimbra vendor-prefixed TZID: verifies the fallback lookup resolves the VTIMEZONE correctly and end > start. Fixes calcom#27877
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/lib/CalendarService.ts">
<violation number="1" location="packages/lib/CalendarService.ts:515">
P2: TZID fallback only takes the last segment (e.g., “Berlin”), so it won’t match standard IANA TZIDs like “Europe/Berlin”. This makes the vendor-prefixed lookup fail and can still select the wrong VTIMEZONE when multiple exist.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // "/zimbra.com/standard/Europe/Berlin" or "/mozilla.org/20070129_1/Europe/Berlin". | ||
| // Extract the IANA part after the last "/" and retry the lookup. | ||
| if (!vtimezone && tzid.includes("/")) { | ||
| const ianaCandidate = tzid.split("/").pop(); |
Contributor
There was a problem hiding this comment.
P2: TZID fallback only takes the last segment (e.g., “Berlin”), so it won’t match standard IANA TZIDs like “Europe/Berlin”. This makes the vendor-prefixed lookup fail and can still select the wrong VTIMEZONE when multiple exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/lib/CalendarService.ts, line 515:
<comment>TZID fallback only takes the last segment (e.g., “Berlin”), so it won’t match standard IANA TZIDs like “Europe/Berlin”. This makes the vendor-prefixed lookup fail and can still select the wrong VTIMEZONE when multiple exist.</comment>
<file context>
@@ -505,7 +505,18 @@ export default abstract class BaseCalendarService implements Calendar {
+ // "/zimbra.com/standard/Europe/Berlin" or "/mozilla.org/20070129_1/Europe/Berlin".
+ // Extract the IANA part after the last "/" and retry the lookup.
+ if (!vtimezone && tzid.includes("/")) {
+ const ianaCandidate = tzid.split("/").pop();
+ if (ianaCandidate) {
+ vtimezone = allVtimezones.find((vtz) => vtz.getFirstPropertyValue("tzid") === ianaCandidate);
</file context>
|
@zomars fixes double timezone conversion for Zimbra CalDAV events — was causing events to show at the wrong time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes CalDAV events synced from Zimbra showing a negative duration (end time before start time, e.g. start 10:00 AM, end 9:00 AM for a 15-minute slot).
Root Cause
When a CalDAV server sends an event with
DTSTART;TZID=Europe/Berlin:20260105T090000, ical.js parses this into a zone-awareICAL.Timethat already correctly represents 09:00 local → 08:00 UTC.The subsequent
event.startDate.convertToZone(zone)call was being made unconditionally.convertToZoneon a non-floating time re-interprets the local wall-clock value as UTC first, then applies the zone offset a second time:For a 15-minute event this produces start=10:00, end=09:00 — a -1h duration.
Fix
1. Guard
convertToZonewithisFloating()— only convert times that ical.js left in floating (zone-less) form. Already-zoned times (isFloating() === false) are correctly resolved by ical.js and must not be converted again.2. Improve VTIMEZONE lookup for vendor-prefixed TZIDs — Zimbra uses
DTSTART;TZID=/zimbra.com/standard/Europe/Berlinbut the VTIMEZONE component hasTZID:Europe/Berlin. The exact-match lookup failed silently, causing a mismatched VTIMEZONE to be selected. Now falls back to extracting the IANA suffix after the last/.Tests
Two new test cases added to
CalendarService.test.ts:Related
Fixes #27877