Skip to content

Commit ad0c68d

Browse files
rfc2822rfc2822
authored
Handle invalid events with DTEND < DTSTART / negative DURATION (#148)
* Handle invalid events with DTEND before DTSTART - Add validation to ensure DTEND is after DTSTART - Update tests to cover cases where DTEND is before DTSTART * Add test to verify that calendar provider allows events with DTEND = DTSTART * Handle invalid events with negative duration - Add tests for events with DTEND before DTSTART and negative DURATION - Update DurationBuilder to ignore negative durations - Update EndTimeBuilder to ignore DTEND before DTSTART * Comments * Take absolute amount of negative durations * - Update TimeApiExtensions.kt to throw IllegalArgumentException for invalid TemporalAmount - Fix typo in EndTimeBuilder.kt comment * Don't build DURATION from DTEND that is before DTSTART * Pass TemporalAmount instead of ical4j.Duration when possible * More tests * More tests for DTEND=DTSTART * KDoc, imports
1 parent 6ff0b64 commit ad0c68d

File tree

7 files changed

+327
-48
lines changed

7 files changed

+327
-48
lines changed

‎lib/src/androidTest/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendarProviderBehaviorTest.kt‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,43 @@ class AndroidCalendarProviderBehaviorTest {
5757
}
5858

5959

60+
/**
61+
* To make sure that's not a problem to insert an event with DTEND = DTSTART.
62+
*/
63+
@Test
64+
fun testInsertEventWithDtEndEqualsDtStart() {
65+
val values = contentValuesOf(
66+
Events.CALENDAR_ID to calendar.id,
67+
Events.DTSTART to 1759403653000, // Thu Oct 02 2025 11:14:13 GMT+0000
68+
Events.DTEND to 1759403653000,
69+
Events.TITLE to "Event with DTSTART = DTEND"
70+
)
71+
val id = calendar.addEvent(Entity(values))
72+
73+
// Google Calendar 2025.44.1-827414499-release correctly shows this event [2025/11/29]
74+
75+
val event2 = calendar.getEventRow(id)
76+
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
77+
}
78+
79+
/**
80+
* To make sure that's not a problem to insert an (invalid/useless) RRULE with UNTIL before the event's DTSTART.
81+
*/
82+
@Test
83+
fun testInsertEventWithRRuleUntilBeforeDtStart() {
84+
val values = contentValuesOf(
85+
Events.CALENDAR_ID to calendar.id,
86+
Events.DTSTART to 1759403653000, // Thu Oct 02 2025 11:14:13 GMT+0000
87+
Events.DURATION to "PT1H",
88+
Events.TITLE to "Event with useless RRULE",
89+
Events.RRULE to "FREQ=DAILY;UNTIL=20251002T000000Z"
90+
)
91+
val id = calendar.addEvent(Entity(values))
92+
93+
val event2 = calendar.getEventRow(id)
94+
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
95+
}
96+
6097
/**
6198
* To verify that it's a problem to insert a recurring all-day event with a duration of zero seconds.
6299
* See:
@@ -114,24 +151,6 @@ class AndroidCalendarProviderBehaviorTest {
114151
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
115152
}
116153

117-
/**
118-
* To make sure that's not a problem to insert an (invalid/useless) RRULE with UNTIL before the event's DTSTART.
119-
*/
120-
@Test
121-
fun testInsertEventWithRRuleUntilBeforeDtStart() {
122-
val values = contentValuesOf(
123-
Events.CALENDAR_ID to calendar.id,
124-
Events.DTSTART to 1759403653000, // Thu Oct 02 2025 11:14:13 GMT+0000
125-
Events.DURATION to "PT1H",
126-
Events.TITLE to "Event with useless RRULE",
127-
Events.RRULE to "FREQ=DAILY;UNTIL=20251002T000000Z"
128-
)
129-
val id = calendar.addEvent(Entity(values))
130-
131-
val event2 = calendar.getEventRow(id)
132-
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
133-
}
134-
135154

136155
/**
137156
* Reported as https://issuetracker.google.com/issues/446730408.

‎lib/src/main/kotlin/at/bitfire/ical4android/util/TimeApiExtensions.kt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,21 @@ object TimeApiExtensions {
112112

113113
/***** Durations *****/
114114

115+
/**
116+
* Returns the absolute (positive) temporal amount.
117+
*/
118+
fun TemporalAmount.abs(): TemporalAmount =
119+
when (this) {
120+
is Duration ->
121+
this.abs()
122+
is Period ->
123+
if (this.isNegative)
124+
this.negated()
125+
else
126+
this
127+
else -> throw IllegalArgumentException("TemporalAmount must be Period or Duration")
128+
}
129+
115130
fun TemporalAmount.toDuration(position: Instant): Duration =
116131
when (this) {
117132
is Duration -> this

‎lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import android.content.Entity
1010
import android.provider.CalendarContract.Events
1111
import androidx.annotation.VisibleForTesting
1212
import at.bitfire.ical4android.util.DateUtils
13+
import at.bitfire.ical4android.util.TimeApiExtensions.abs
1314
import at.bitfire.ical4android.util.TimeApiExtensions.toDuration
1415
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
1516
import at.bitfire.ical4android.util.TimeApiExtensions.toRfc5545Duration
@@ -18,9 +19,9 @@ import net.fortuna.ical4j.model.Property
1819
import net.fortuna.ical4j.model.component.VEvent
1920
import net.fortuna.ical4j.model.property.DtEnd
2021
import net.fortuna.ical4j.model.property.DtStart
21-
import net.fortuna.ical4j.model.property.Duration
2222
import net.fortuna.ical4j.model.property.RDate
2323
import net.fortuna.ical4j.model.property.RRule
24+
import java.time.Duration
2425
import java.time.Period
2526
import java.time.temporal.TemporalAmount
2627

@@ -42,8 +43,13 @@ class DurationBuilder: AndroidEntityBuilder {
4243
}
4344

4445
val dtStart = from.requireDtStart()
45-
val duration = from.duration
46-
?: calculateFromDtEnd(dtStart, from.endDate)
46+
47+
// calculate DURATION from DTEND - DTSTART, if necessary
48+
val calculatedDuration = from.duration?.duration
49+
?: calculateFromDtEnd(dtStart, from.endDate) // ignores DTEND < DTSTART
50+
51+
// use default duration, if necessary
52+
val duration = calculatedDuration?.abs() // always use positive duration
4753
?: defaultDuration(DateUtils.isDate(dtStart))
4854

4955
/* [RFC 5545 3.8.2.5]
@@ -54,7 +60,7 @@ class DurationBuilder: AndroidEntityBuilder {
5460
so we wouldn't have to take care of that. However it expects seconds to be in "P<n>S" format,
5561
whereas we provide an RFC 5545-compliant "PT<n>S", which causes the provider to crash:
5662
https://github.com/bitfireAT/synctools/issues/144. So we must convert it ourselves to be on the safe side. */
57-
val alignedDuration = alignWithDtStart(duration.duration, dtStart)
63+
val alignedDuration = alignWithDtStart(duration, dtStart)
5864

5965
/* TemporalAmount can have months and years, but the RFC 5545 value must only contain weeks, days and time.
6066
So we have to recalculate the months/years to days according to their position in the calendar.
@@ -74,13 +80,13 @@ class DurationBuilder: AndroidEntityBuilder {
7480
* @return Temporal amount that is
7581
*
7682
* - a [Period] (days/months/years that can't be represented by an exact number of seconds) when [dtStart] is a DATE, and
77-
* - a [java.time.Duration] (exact time that can be represented by an exact number of seconds) when [dtStart] is a DATE-TIME.
83+
* - a [Duration] (exact time that can be represented by an exact number of seconds) when [dtStart] is a DATE-TIME.
7884
*/
7985
@VisibleForTesting
8086
internal fun alignWithDtStart(amount: TemporalAmount, dtStart: DtStart): TemporalAmount {
8187
if (DateUtils.isDate(dtStart)) {
8288
// DTSTART is DATE
83-
return if (amount is java.time.Duration) {
89+
return if (amount is Duration) {
8490
// amount is Duration, change to Period of days instead
8591
Period.ofDays(amount.toDays().toInt())
8692
} else {
@@ -90,7 +96,7 @@ class DurationBuilder: AndroidEntityBuilder {
9096

9197
} else {
9298
// DTSTART is DATE-TIME
93-
return if (amount is java.time.Period) {
99+
return if (amount is Period) {
94100
// amount is Period, change to Duration instead
95101
amount.toDuration(dtStart.date.toInstant())
96102
} else {
@@ -100,32 +106,38 @@ class DurationBuilder: AndroidEntityBuilder {
100106
}
101107
}
102108

109+
/**
110+
* Calculates the DURATION from DTEND - DTSTART, if possible.
111+
*
112+
* @param dtStart start date/date-time
113+
* @param dtEnd (optional) end date/date-time (ignored if not after [dtStart])
114+
*
115+
* @return temporal amount ([Period] or [Duration]) or `null` if no valid end time was available
116+
*/
103117
@VisibleForTesting
104-
internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): Duration? {
105-
if (dtEnd == null)
118+
internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): TemporalAmount? {
119+
if (dtEnd == null || dtEnd.date.toInstant() <= dtStart.date.toInstant())
106120
return null
107121

108122
return if (DateUtils.isDateTime(dtStart) && DateUtils.isDateTime(dtEnd)) {
109123
// DTSTART and DTEND are DATE-TIME → calculate difference between timestamps
110124
val seconds = (dtEnd.date.time - dtStart.date.time) / 1000
111-
Duration(java.time.Duration.ofSeconds(seconds))
125+
Duration.ofSeconds(seconds)
112126
} else {
113127
// Either DTSTART or DTEND or both are DATE:
114128
// - DTSTART and DTEND are DATE → DURATION is exact number of days (no time part)
115129
// - DTSTART is DATE, DTEND is DATE-TIME → only use date part of DTEND → DURATION is exact number of days (no time part)
116130
// - DTSTART is DATE-TIME, DTEND is DATE → amend DTEND with time of DTSTART → DURATION is exact number of days (no time part)
117131
val startDate = dtStart.date.toLocalDate()
118132
val endDate = dtEnd.date.toLocalDate()
119-
Duration(Period.between(startDate, endDate))
133+
Period.between(startDate, endDate)
120134
}
121135
}
122136

123-
private fun defaultDuration(allDay: Boolean): Duration =
124-
Duration(
125-
if (allDay)
126-
Period.ofDays(1)
127-
else
128-
java.time.Duration.ZERO
129-
)
137+
private fun defaultDuration(allDay: Boolean): TemporalAmount =
138+
if (allDay)
139+
Period.ofDays(1)
140+
else
141+
Duration.ZERO
130142

131143
}

‎lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/EndTimeBuilder.kt‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import android.content.Entity
1010
import android.provider.CalendarContract.Events
1111
import androidx.annotation.VisibleForTesting
1212
import at.bitfire.ical4android.util.DateUtils
13+
import at.bitfire.ical4android.util.TimeApiExtensions.abs
1314
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
1415
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDateTime
1516
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
@@ -28,6 +29,7 @@ import java.time.LocalDate
2829
import java.time.Period
2930
import java.time.ZoneId
3031
import java.time.ZonedDateTime
32+
import java.time.temporal.TemporalAmount
3133

3234
class EndTimeBuilder: AndroidEntityBuilder {
3335

@@ -47,10 +49,22 @@ class EndTimeBuilder: AndroidEntityBuilder {
4749
}
4850

4951
val dtStart = from.requireDtStart()
50-
val dtEnd = from.endDate?.let { alignWithDtStart(it, dtStart = dtStart) }
51-
?: calculateFromDuration(dtStart, from.duration)
52+
53+
// potentially calculate DTEND from DTSTART + DURATION, and always align with DTSTART value type
54+
val calculatedDtEnd = from.getEndDate(/* don't let ical4j calculate DTEND from DURATION */ false)
55+
?.let { alignWithDtStart(it, dtStart = dtStart) }
56+
?: calculateFromDuration(dtStart, from.duration?.duration)
57+
58+
// ignore DTEND when not after DTSTART and use default duration, if necessary
59+
val dtEnd = calculatedDtEnd
60+
?.takeIf { it.date.toInstant() > dtStart.date.toInstant() } // only use DTEND if it's after DTSTART [1]
5261
?: calculateFromDefault(dtStart)
5362

63+
/**
64+
* [1] RFC 5545 3.8.2.2 Date-Time End:
65+
* […] its value MUST be later in time than the value of the "DTSTART" property.
66+
*/
67+
5468
// end time: UNIX timestamp
5569
values.put(Events.DTEND, dtEnd.date.time)
5670

@@ -122,12 +136,21 @@ class EndTimeBuilder: AndroidEntityBuilder {
122136
}
123137
}
124138

139+
/**
140+
* Calculates the DTEND from DTSTART + DURATION, if possible.
141+
*
142+
* @param dtStart start date/date-time
143+
* @param duration (optional) duration
144+
*
145+
* @return end date/date-time (same value type as [dtStart]) or `null` if [duration] was not given
146+
*/
125147
@VisibleForTesting
126-
internal fun calculateFromDuration(dtStart: DtStart, duration: net.fortuna.ical4j.model.property.Duration?): DtEnd? {
148+
internal fun calculateFromDuration(dtStart: DtStart, duration: TemporalAmount?): DtEnd? {
127149
if (duration == null)
128150
return null
129151

130-
val dur = duration.duration
152+
val dur = duration.abs() // always take positive temporal amount
153+
131154
return if (DateUtils.isDate(dtStart)) {
132155
// DTSTART is DATE
133156
if (dur is Period) {

‎lib/src/test/kotlin/at/bitfire/ical4android/util/TimeApiExtensionsTest.kt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package at.bitfire.ical4android.util
88

9+
import at.bitfire.ical4android.util.TimeApiExtensions.abs
910
import at.bitfire.ical4android.util.TimeApiExtensions.requireTimeZone
1011
import at.bitfire.ical4android.util.TimeApiExtensions.toDuration
1112
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
@@ -172,6 +173,39 @@ class TimeApiExtensionsTest {
172173
}
173174

174175

176+
@Test
177+
fun testTemporalAmount_abs_Duration_negative() {
178+
assertEquals(
179+
Duration.ofMinutes(1),
180+
Duration.ofMinutes(-1).abs()
181+
)
182+
}
183+
184+
@Test
185+
fun testTemporalAmount_abs_Duration_positive() {
186+
assertEquals(
187+
Duration.ofDays(1),
188+
Duration.ofDays(1).abs()
189+
)
190+
}
191+
192+
@Test
193+
fun testTemporalAmount_abs_Period_negative() {
194+
assertEquals(
195+
Period.ofWeeks(1),
196+
Period.ofWeeks(-1).abs()
197+
)
198+
}
199+
200+
@Test
201+
fun testTemporalAmount_abs_Period_positive() {
202+
assertEquals(
203+
Period.ofDays(1),
204+
Period.ofDays(1).abs()
205+
)
206+
}
207+
208+
175209
@Test
176210
fun testTemporalAmount_toDuration() {
177211
assertEquals(Duration.ofHours(1), Duration.ofHours(1).toDuration(Instant.EPOCH))

0 commit comments

Comments
 (0)