[Kde-pim] Review Request: iCal format fixes for CREATED and DTSTAMP

Christian Mollekopf chrigi_1 at fastmail.fm
Wed Nov 28 19:33:58 GMT 2012


> On Nov. 28, 2012, 12:21 p.m., David Jarvie wrote:
> > This looks good - things seem to have been a bit screwed up before. However, this will cause problems for KAlarm, which would purge alarms unintentionally when it reads a calendar created with the old code, with DTSTAMP and CREATED reversed. There needs to be backwards compatibility for calendars created using the old code, so that for them DTSTAMP is read into the Incidence's 'created' property, and CREATED is read into the 'dtstamp' property.
> 
> Christian Mollekopf wrote:
>     I don't quite follow. DTSTAMP is now the last-modification-date (except for iTip where we have a METHOD property), and CREATED is just not update on every write anymore. If you require the time the object has last been written you should probably use Akonadi::Item::modificationTime().
>     
>     If you can give me a pointer to your code I can take a look, compatibility code shouldn't be required I think.
> 
> David Jarvie wrote:
>     KAlarm uses Incidence::setCreated() and Incidence::created() to set and retrieve the date an archived event was created. It uses this date to determine whether the event should be purged from the calendar (the user can configure that events should be purged, i.e. deleted, a set number of days after they were archived). The old icalformat_p.cpp stored the date set by these methods in the DTSTAMP attribute, not the CREATED attribute. Your patch will change this so that the created value will be stored in the CREATED attribute. So if KAlarm is using your patched code to access a calendar which was written by the old software, when it uses Incidence::created() to fetch a value, the value of DTSTAMP will be returned instead, which will be set to some unrelated value. So the decision on whether to purge archived events will be based on a wrong date.
>     
>     Any other software which relies on either CREATED or DTSTAMP values will also have problems when reading calendars written with the old library, since their values are now swapped.

If you accessed the creation date before, you got back the timestamp of serialization. If you access the same object now, you still get back the very same date (on the created property only what is written out changed). What has changed is that if you write a new object out, it will not write a current timestamp, but instead use the created-date of the incidence. 

So:

Before:
* CREATED: write: current timestamp read: CREATED property
* DTSTAMP: write: created-date of incidence read: never read
Now:
* CREATED: write: created-date of incidence read: CREATED property
* DTSTAMP: write: last-modified-date (without METHOD) read: DTSTAMP property

So I don't think it will affect your code in that respect. The only code which could potentially be affected if one relies on created containing the time of serialization, and I can't think of any situation where that would make any sense. So IMO this patch should be safe.


- Christian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107480/#review22692
-----------------------------------------------------------


On Nov. 26, 2012, 7:21 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107480/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2012, 7:21 p.m.)
> 
> 
> Review request for KDEPIM, KDEPIM-Libraries and David Jarvie.
> 
> 
> Description
> -------
> 
> This patch fixes the uses of CREATED and DTSTAMP in respect to RFC5545 (iCal) and RFC5546 (iTip).
> 
> CREATED was used as serialization timestamp, which is IMO wrong as it should preserve the creation date of the conceptual ical object. The dtstamp was written as creation-date but never read back, but it should be the last-modification-date without METHOD (iTip), and the time of serialization with a METHOD.
> 
> This patch fixes the following problems:
> * dtstamp and created exchanged when writing any ical object
> * created date is not preserved
> * last-modification-date is not preserved
> * vFreebusy iTip messages contains a created date (but must not)
> 
> As a sideeffect this patch also fixed the RecurNext* and RecurPrev* unittests.
> 
> This patch should be safe to apply to existing systems.
> 
> 
> This addresses bugs 310448 and 310469.
>     http://bugs.kde.org/show_bug.cgi?id=310448
>     http://bugs.kde.org/show_bug.cgi?id=310469
> 
> 
> Diffs
> -----
> 
>   kcalcore/icalformat_p.cpp 418a92a6e1f033d1110add754c4966436585dea2 
> 
> Diff: http://git.reviewboard.kde.org/r/107480/diff/
> 
> 
> Testing
> -------
> 
> checked existing unit tests (all pass). Running the patch on my productive system now.
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/



More information about the kde-pim mailing list