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

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

Conversation

@ArnyminerZ

@ArnyminerZ ArnyminerZ commented May 25, 2026 •

Copy link
Copy Markdown
Member

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

Copilot AI 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.

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

Copilot AI 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.

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

rfc2822 commented May 25, 2026

Copy link
Copy Markdown
Member

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

devvv4ever commented May 25, 2026 •

Copy link
Copy Markdown
Member

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

rfc2822 commented May 25, 2026

Copy link
Copy Markdown
Member

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

This branch has not been deployed

No deployments
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