|Summary:||ActiveSync does not handle Outlook all day events correctly|
|Product:||SyncEvolution||Reporter:||Graham Cobb <g+syncevolution>|
|Component:||ActiveSync||Assignee:||SyncEvolution Community <syncevolution-issues>|
|Status:||RESOLVED FIXED||QA Contact:|
|i915 platform:||i915 features:|
Patch to fix London timezone handling bug
Patch to improve logging problems with all day event times
Patch to fix theoretical problem with timezone calculation
Patch to turn all day event into not all day event if Exchange supplies wrong timezone info
Correct logging to stop memory leak
Description Graham Cobb 2014-10-20 12:40:35 UTC
Discussion from mailing list... On Mon, 2014-10-20 at 10:17 +0100, Graham Cobb wrote: > I know I have had trouble with all day events before, but it might have > been in my OpenSync days (or even my own OWASync!). > > I have just noticed, in my testing of 188.8.131.52, that all day events are > not working properly. I don't suppose this is new in this version -- I > probably didn't notice it before. Right. That code hasn't changed in quite a while. The relevant code to look at are the _eas2ical_convert_datetime_property() and _eas2ical_convert_component() methods activesyncd/eas-daemon/libeas/eas-cal-info-translator.c. > I have an all day event in Outlook, scheduled for this Thursday > (20141023). I did a one-way sync from Exchange (activesync) to files, > and the file ended up with the following: > > DTSTART;VALUE=DATE:20141022 > DTEND;VALUE=DATE:20141023 > > To make matters worse, I then synced those files with Owncloud and > Owncloud now believes this is an all day event on the 22nd! (And so does > Thunderbird when I synced that against Owncloud). > > I have not yet attempted to debug this (looking at what activesync > sends, etc) but wondered if anyone either has seen this before, or has a > view on what the correct VCALENDAR file should look like for this event. Correct for an all-day event on (and just on) the 23rd would be: DTSTART;VALUE=DATE:20141023 DTEND;VALUE=DATE:20141024 The end date(-time) is exclusive. The key problem with the conversion is that Microsoft requires putting a time into the start and end fields. But which time does an all-day event start and end at? Local time? UTC? Does it depend on where someone is? This is all not defined in the Microsoft documentation. It only documents a flag that marks an event as all-day. If I remember correctly, in practice it had to be the local time of the user. So if I am in GMT+2, the 00:00 time as to be shifted by two hours, thus causing the date to also change. Obviously this shift goes wrong when the time zones are not set correctly (or at least consistently). I have no idea how that is supposed to be handled correctly in all cases.
Comment 1 Graham Cobb 2014-10-28 23:03:22 UTC
Created attachment 108597 [details] [review] Patch to fix London timezone handling bug Patch to fix the actual problem reported in this bug. The problem was because the timezone handling code incorrectly optimized away the timezone for the UK daylight savings time, which meant the all day event start and end times were not being correctly converted to local time.
Comment 2 Graham Cobb 2014-10-28 23:06:42 UTC
Created attachment 108598 [details] [review] Patch to improve logging problems with all day event times This second patch just improves the logging when all day event times fail the "sanity test" (ie. are not at midnight).
Comment 3 Graham Cobb 2014-10-28 23:08:59 UTC
Created attachment 108599 [details] [review] Patch to fix theoretical problem with timezone calculation This third patch fixes a bug unlikely to be seen in real life. It would only occur if the "StandardBias" feild of a timezone was set to a non-zero value. This is not the normal way timezones are used but could happen in theory.
Comment 4 Graham Cobb 2014-10-29 18:43:29 UTC
Created attachment 108643 [details] [review] Patch to turn all day event into not all day event if Exchange supplies wrong timezone info This patch adds code to effectively turn an all day event into a non-all-day event if the time is not midnight. It actually just changes the format to be a full time, instead of just a day, but that has the effect of it not being an all day event in most applications.
Comment 5 Patrick Ohly 2014-10-30 20:21:29 UTC
Comment on attachment 108598 [details] [review] Patch to improve logging problems with all day event times Review of attachment 108598 [details] [review]: ----------------------------------------------------------------- A better formatting of the commit message would be a short summary line followed by an empty line, then full description. g_warning("All day event with no timezone does not start at UTC midnight: %s", icaltime_as_ical_string_r(tt)); This leaks the string, because the _r variant returns the newly allocated string and expects the caller to free it. The other locations with the normal version are okay because libical then retains ownership in a ring buffer. Can you fix that in a future patch? For now I'll take the patch as-is because I cannot test it myself and don't want to break something that you already tested successfully.
Comment 6 Graham Cobb 2014-11-11 20:54:13 UTC
Created attachment 109306 [details] [review] Correct logging to stop memory leak Sorry about the commit message style -- I am used to a different style. Hope this one is better. This fixes the logging memory leak. I originally thought the _r variants used the ring buffers but after reading the code I realised that, in fact, the variants without the _r used the ring. But I missed one of the calls to correct.
Comment 7 Patrick Ohly 2015-02-09 13:52:06 UTC
Memory leak patch merged, thanks.