[jtx Board] Task sync causes NullPointerException

[rfc2822]

Discussed in #1264

^{Originally posted by CrystalMV13 January 25, 2025}
Hello, I have been using Davx5 for a bit, and it has been working great now, but I've identified an issue in syncing recurring tasks; syncing it throws a NullPointerException. I have attached the debug info and logcat (edited to remove my email address and calendar names). The important calendar is cal1. The bug begins only once the first task is marked as complete.

debug-info.txt
logcat.txt

Thank you in advance!

[rfc2822]

CC @patrickunterwegs

[rfc2822]

According to the exception, something in JtxSyncManager… maybe a small missing null check or something like that.

@ArnyminerZ Can you please have a look

[ArnyminerZ]

I've taken a look, and the error could be coming from ical4android, though I'm not sure. There this call:

https://github.com/bitfireAT/ical4android/blob/883954c7ca500a30537974f0885512b7b1ad7e2a/lib/src/main/kotlin/at/bitfire/ical4android/JtxICalObject.kt#L701

And getICalendarFormat() can return null, which is then passed to CalendarOutputter.output. Since it's a Java call, maybe that's why it's not caught.

I'd say that is the case because the stack trace shows a call to:

at.bitfire.davdroid.sync.JtxSyncManager.downloadRemote$lambda$7$lambda$6$lambda$5(JtxSyncManager.kt:96)

which tries to write on the resource

davx5-ose/app/src/main/kotlin/at/bitfire/davdroid/sync/JtxSyncManager.kt

Line 96 in 6be42d4

resource.write(os)

and that's what calls the line above.

It's not clear why the error is being thrown at at.bitfire.davdroid.sync.JtxSyncManager.processICalObject(JtxSyncManager.kt:100), but I don't see why that line should be throwing a NPE.

[ArnyminerZ]

It could be even related to recurring tasks because here:

https://github.com/bitfireAT/ical4android/blob/883954c7ca500a30537974f0885512b7b1ad7e2a/lib/src/main/kotlin/at/bitfire/ical4android/JtxICalObject.kt#L681-L689

there's a return null when the recurring instance's component is neither VTODO nor VJOURNAL.

Edit: according to the debug info, cal1 does support VTODO.

Edit 2: But that's for the calendar, the error may be caused by the recurrence instance which is not inheriting the component...

[rfc2822]

When using Android Studio's Code / Analyze stack trace with a ProGuard unscrambler from the market place and the mapping.txt of 4.4.5-standard, the stack trace becomes:

java.lang.NullPointerException
	at at.bitfire.davdroid.sync.JtxSyncManager.processICalObject(JtxSyncManager.java:172)
	at at.bitfire.davdroid.sync.JtxSyncManager.downloadRemote$lambda$7$lambda$6$lambda$5(JtxSyncManager.java:141)
	at at.bitfire.davdroid.sync.JtxSyncManager.$r8$lambda$Hx504ukj7F58Jikc9nqqvOxc5II(JtxSyncManager.java:0)
	at at.bitfire.davdroid.sync.JtxSyncManager$$ExternalSyntheticLambda8.invoke(JtxSyncManager.java:0)
	at at.bitfire.davdroid.sync.SyncException$Companion.wrapWithRemoteResource(SyncException.java:38)
	at at.bitfire.davdroid.sync.JtxSyncManager.downloadRemote$lambda$7$lambda$6(JtxSyncManager.java:126)
…

In the 4.4.5 source code, this line is:

davx5-ose/app/src/main/kotlin/at/bitfire/davdroid/sync/JtxSyncManager.kt

Line 172 in d10add8

val local = localCollection.findRecurring(jtxICalObject.uid, jtxICalObject.recurid!!, jtxICalObject.dtstart!!)

So I think the problem must be there, probably in one of the !!

What do you think @ArnyminerZ @sunkup ?

[ArnyminerZ]

Good to know, then we should add some kind of handler for these edge cases.

[ArnyminerZ]

Technically, recurid cannot be null because it's checked before, but I don't see why dtstart couldn't. In any case, since they are all vars, anything can happen. To be sure, I'd make all the objects in JtxICalObject vals instead of vars, and LocalJtxICalObject a data class, though I'm not sure if the variables would be inherited and therefore copiable by LocalJtxICalObject, we'd have to see.

As another solution, what if we just handle that if dtstart is null, local should be null there, and a new local would be created?

[rfc2822]

We have received the iCalendar:

BEGIN:VCALENDAR
PRODID:-Vivaldi Calendar V1.0//EN
VERSION:2.0
CALSCALE:GREGORIAN
DTSTAMP:20250228T032800Z
[… VTIMEZONE …]
BEGIN:VTODO
DTSTAMP;VALUE=DATE-TIME:20250228T032800Z
DESCRIPTION:test desc
DUE;TZID=America/New_York:20250228T130000
RECURRENCE-ID;TZID=America/New_York:20250228T130000
SEQUENCE:1
SUMMARY:Test Task
UID:47a23c66-8c1a-4b44-bbe8-ebf33f8cf80f
STATUS:COMPLETED
COMPLETED;TZID=America/New_York:20250227T222800
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:
TRIGGER:-PT1H
END:VALARM
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:Test Task
TRIGGER:-PT24H
END:VALARM
END:VTODO
BEGIN:VTODO
DTSTAMP;VALUE=DATE-TIME:20250228T032800Z
DESCRIPTION:test desc
DUE;TZID=America/New_York:20250228T130000
SEQUENCE:1
SUMMARY:Test Task
UID:47a23c66-8c1a-4b44-bbe8-ebf33f8cf80f
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=FR;UNTIL=20250505T235959Z
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:
TRIGGER:-PT1H
END:VALARM
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:Test Task
TRIGGER:-PT24H
END:VALARM
END:VTODO
END:VCALENDAR

There's really no DTSTART (which is fine according to RFC 5445 if I remember correctly).

[rfc2822]

As another solution, what if we just handle that if dtstart is null, local should be null there, and a new local would be created?

Sure, make it as you think it's best, and it can then be discussed in the PR.

[ArnyminerZ]

Well, according to RFC 5545:

The following is REQUIRED if the component appears in an iCalendar object that doesn't specify the "METHOD" property; otherwise, it is OPTIONAL; in any case, it MUST NOT occur more than once.

Reference

So as far as my understanding goes, DTSTART should ALWAYS be present in those cases, even though it can be inherited from the recurrence.

Section 3.8.4.4 doesn't specify anything about making DTSTART not mandatory as well.

[sunkup]

Since it's a vtodo property RFC-5545/3-6-2-to-do-component applies and by the logic given there:

Either 'due' or 'duration' MAY appear in
            ; a 'todoprop', but 'due' and 'duration'
            ; MUST NOT occur in the same 'todoprop'.
            ; If 'duration' appear in a 'todoprop',
            ; then 'dtstart' MUST also appear in
            ; the same 'todoprop'.

So since the todoprop has a DUE it MUST have a DTSTART also. It does not however, and so this todo component / todoprop is to be considered invalid and ignored. Hence we can not evaluate findRecurring(jtxICalObject.uid, jtxICalObject.recurid!!, jtxICalObject.dtstart!!) since we can not calculate the recurrence (should add a null check to this) and val local will be null.

davx5-ose/app/src/main/kotlin/at/bitfire/davdroid/sync/JtxSyncManager.kt

Line 172 in d10add8

val local = localCollection.findRecurring(jtxICalObject.uid, jtxICalObject.recurid!!, jtxICalObject.dtstart!!)