Skip to content
This repository was archived by the owner on May 26, 2026. It is now read-only.

Fix mishandling of exceptions in UTC#442

Open
ArnyminerZ wants to merge 3 commits into
mainfrom
fix-davx5-898
Open

Fix mishandling of exceptions in UTC#442
ArnyminerZ wants to merge 3 commits into
mainfrom
fix-davx5-898

Conversation

@ArnyminerZ
Copy link
Copy Markdown
Member

@ArnyminerZ ArnyminerZ commented May 25, 2026

Closes https://github.com/bitfireAT/davx5/issues/898

The TemporalAdapter.isBefore call in ICalendarGenerator.write crashed when comparing a UTC Instant (for example 2025-08-22T03:30:00Z from an exception's DTSTART) against a ZonedDateTime (from the main event's DTSTART), because ical4j internally tries ZonedDateTime.from(Instant) which always throws DateTimeException.

Added a failing test that showcases the issue, and a fix for it.

@ArnyminerZ ArnyminerZ requested a review from Copilot May 25, 2026 14:59
@ArnyminerZ ArnyminerZ self-assigned this May 25, 2026
@ArnyminerZ ArnyminerZ added the pr-bugfix Fixes something that isn't working (only used for PRs) label May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a crash in ICalendarGenerator.write when determining the earliest DTSTART across a main event and its recurrence exceptions, specifically when exceptions use a UTC Instant while the main event uses a ZonedDateTime (DAVx5 #898 scenario).

Changes:

  • Add a regression test covering a recurring event whose exception DTSTART is a UTC Instant.
  • Normalize temporals before calling TemporalAdapter.isBefore to avoid DateTimeException when comparing mixed temporal types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/src/main/kotlin/at/bitfire/synctools/icalendar/ICalendarGenerator.kt Normalizes exception/main DTSTART values prior to comparison to prevent crashes on mixed temporal types.
lib/src/test/kotlin/at/bitfire/synctools/icalendar/ICalendarGeneratorTest.kt Adds a test reproducing the UTC Instant exception DTSTART crash and asserting calendar output is produced.

Comment thread lib/src/main/kotlin/at/bitfire/synctools/icalendar/ICalendarGenerator.kt Outdated
@ArnyminerZ ArnyminerZ changed the title Fix davx5 898 Fix mishandling of exceptions in UTC May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ArnyminerZ ArnyminerZ requested a review from rfc2822 May 25, 2026 15:12
@rfc2822
Copy link
Copy Markdown
Member

rfc2822 commented May 25, 2026

Is there an issue or did you discover that by chance?

Is it related to #424 somehow (that is also about crash with Instant)?

@devvv4ever
Copy link
Copy Markdown
Member

devvv4ever commented May 25, 2026

Is there an issue or did you discover that by chance?

Is it related to #424 somehow (that is also about crash with Instant)?

Issue is https://github.com/bitfireAT/davx5/issues/898 (which I forgot to add to Development Plan ... sorry - did so now)

@ArnyminerZ
Copy link
Copy Markdown
Member Author

Is there an issue or did you discover that by chance?

No, this is a fix for https://github.com/bitfireAT/davx5/issues/898

Is it related to #424 somehow (that is also about crash with Instant)?

I don't think so, they come from different places

@rfc2822
Copy link
Copy Markdown
Member

rfc2822 commented May 25, 2026

No, this is a fix for bitfireAT/davx5#898

Ah, nice, wasn't aware of that one! I linked it to the PR.

@ArnyminerZ
Copy link
Copy Markdown
Member Author

I linked it to the PR.

I didn't link it originally so that we remember to update the synctools version in DAVx⁵ as well, but I guess it will eventually get updated anyway so

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

Labels

pr-bugfix Fixes something that isn't working (only used for PRs)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants