release 2.3.34 breaking changes against RFC5545 for TZID with UTC

[zijianhuang]

in 2.3.3, the following ical text could be correctly deserialized and then serialized :

BEGIN:VCALENDAR
PRODID:-//APS//Tasks Manager//EN
VERSION:2.0
BEGIN:VTODO
DTSTAMP:20170406T044849Z
SEQUENCE:0
UID:8e8bb9b76d70465fbfd5ec538bf2c877
END:VTODO
END:VCALENDAR

However, in 2.3.4, the serialized text becomes:

BEGIN:VCALENDAR
PRODID:-//APS//Tasks Manager//EN
VERSION:2.0
BEGIN:VTODO
DTSTAMP;TZID=UTC:20170406T044849
SEQUENCE:0
UID:8e8bb9b76d70465fbfd5ec538bf2c877
END:VTODO
END:VCALENDAR

Technically this is correct, however, just more verbose. And according to page 28 of RFC5545:
The "TZID" property parameter MUST NOT be applied to DATE properties and DATE-TIME or TIME properties whose time values are specified in UTC.

It will be nice that TZID is not specified if the time is in UTC, just as 2.3.33 supports.

[rianjs]

You consider that a breaking change?

[zijianhuang]

This could be a gray area of breaking change. As I said, the new serialized result is correct technically, but just breaking my existing test cases assuming that the round trip of deserialization and serialization get the same iCal text. The change in 2.3.4 is not really breaking the standard, but it will be nice to respect that The "TZID" property parameter MUST NOT be applied to DATE properties and DATE-TIME or TIME properties whose time values are specified in UTC. So that the serialized time property text could be shorter:
DTSTAMP:20170406T044849Z
is shorter than
DTSTAMP;TZID=UTC:20170406T044849

As you can see most examples in rfc5545 use the shorter form.

I know that logically the standard does not mandate that the round trip of deserialization and serialization produce the identical texts. However, in the past when doing serialization for other types of projects, I had found that if the round trip produce the same texts, it could make the unit testing and integration testing for the lib and the applications become easier.

[rianjs]

I know that logically the standard does not mandate that the round trip of deserialization and serialization produce the identical texts.

Right. If you don't specify a DTSTAMP, for example, the text will never be identical, as an example. I agree that it seems like it should be identical in most cases, but it often won't be. I think checking for semantic equality is always the best approach. Checking text is brittle. Some things are modeled as HashSet behind the scenes, which have no ordering guarantees.

Textual equality is fraught with peril.

So that the serialized time property text could be shorter:

Having the Z suffix is actually longer in some cases. Imagine you have 25 EXDATEs, each with a Z suffix. :) (Ask me how I know.) TZID=UTC should compress pretty well, going over the wire if you have some form of gzip or deflate compression turned on.

---

I'm sympathetic to your request. Maybe there should be a switch to use Z suffixes over explicit TZID=UTC when it's possible. That could get complicated; there's already a bug if you are using (or will make use of) EXDATE and RDATE recurrences. Those are allowed to have time zones. Behind the scenes, those are a list of CalDateTimes. In that case, what is the source of truth about the underlying time zones? Is it the list that owns them, or is it the CalDateTimes themselves? What if the time zones are different on each CalDateTime? ical.net doesn't sort the datetimes by time zone before creating the lists; that's left up to the application developer.

These are the weaknesses of the current API. With this change, I have made the ownership of the time zone a property of the parent list. Maybe that's not the best choice, but it can evolve with time.

Another reason to prefer explicit TZID: Telerik's libraries don't understand the Z suffix when deserializing to DateTime. It assumes DateTimeKind.Unspecified which is treated the same as DateTimeKind.Local, which causes events to drift by the magnitude and direction of the UTC offset on each serialization round-trip. I think Telerik is pretty popular in the .NET world. I actually think this might be a weakness of .NET's DateTime.Parse methods, but I haven't looked too closely.

[zijianhuang]

And I have just located the change you made a few days ago where you had made such in-source comment:

//Historically, dday.ical substituted TZID=UTC with Z suffixes on DateTimes. However this behavior isn't part of the spec. Some popular libraries
//like Telerik's RadSchedule components will only understand the DateTimeKind.Utc if the ical text says TZID=UTC. Anything but that is treated as
//DateTimeKind.Unspecified, which is problematic.

I would say that the dday.ical way is part of the rfc5545 as indicated in comments above. However Telerik's RadSchedule is problematic not respecting the spec well, so it will be better to raise a ticket asking the telerik team to fix it.

Meanwhile, before the telerik's team could fix it, it may be good to have a switch in ical.net to support both the telerik way and the dday.ical way, since not every ical.net users are using telerik components.

[posics]

Actually, if:

page 28 of RFC5545: The "TZID" property parameter MUST NOT be applied to DATE properties and DATE-TIME or TIME properties whose time values are specified in UTC

And the old version adhered to this whereas the new one doesn't then one could (and probably should) consider it a breaking change, not a gray area. If RFC says no "TZID=UTC" if there is already a 'Z' at the end of it then it's obvious RFC has been broken. No offence meant, not being an ass, just pointing out a "technicality". So even if the example date stamp is valid in a broader sense it's invalid per RFC. And from my ublnderstanding of github rules, "fonlow" just volunteered to fix it.

[zijianhuang]

@rianjs,

thanks for your detailed explanations quite inspiring.

@"checking for semantic equality is always the best approach. Checking text is brittle. Some things are modeled as HashSet behind the scenes, which have no ordering guarantees."

I very much agree on that, as I have been doing with XML serialization. I had actually forked this project however, would like to listen to more contexts before committing altering the codebase.

[rianjs]

And the old version adhered to this whereas the new one doesn't then one could (and probably should) consider it a breaking change, not a gray area. If RFC says no "TZID=UTC" if there is already a 'Z' at the end of it then it's obvious RFC has been broken.

Yeah, you're right. That being said, ical4j doesn't consider it an error, either. I'm not sure why that restriction is in the spec... there doesn't seem to be a good reason for it?

I don't view slavish adherence to the ical spec to be a main goal of ical.net, though I do try to let the spec guide the development where and when possible. There will always be tension between the real world and spec-based pedantry. As in this case, particularly where other libraries are concerned. :)

[zijianhuang]

I think the spec had provided some flexibility that both formats are correct, while a technical implementation may pick either or both. It is a bit ashamed that telerik supports only the one with TZID.

@rjanjs, I agree with you on your way of making balance and trade-off. Just for your reference, I had just checked Google Calendar, and found the Google Calendar export to ics always using yyyyMMddTHHmmssZ UTC format without using TZID which GC could import both formats correctly. This means Telerik Scheduler could not handle properly ics exported from Google Calendar.

Obviously Telerik Scheduler has a bug, while the change in ical.net 2.3.34 is a workaround to take care of telerick scheduler users. As far as I could see now, ical.net 2.3.33 support both formats in serialization and deserialzation, while in 2.3.34, support both formats in deslerialization and only the TZID format for serialization. Still within the spec. Not a bad trade-off.

[atajadod]

I haven't read the RFC, but the new version does not seem to work with outlook!

[zijianhuang]

@atajadod , can you be more specific about what not working?

[rianjs]

I haven't read the RFC, but the new version does not seem to work with outlook!

Can you be more specific?

Also, what happens if you open your calendar file, and stick Zs on the end of times? Does it work then?

I'm not sure how well a bool includeTzIdIWhenUtc would work with the current API. Very inelegant. I wonder if I can just do both, so the serialized form would look like this:

EXDATE;TZID=UTC:20170404T123000Z,20170403T123000Z

Would that make Telerik, Outlook, Google, and ical.net happy?

Edit: ical4j flags it as an error.

[rianjs]

Notes for myself: break was from PR #261

[atajadod]

@zijianhuang, if everything's fine, outlook shows me a calendar but in this case it shows me a .ics attachment with a name like "not supported format" of so.
@rianjs great. thanks.

[rianjs]

@atajadod - Can you open the broken .ics file, and pull out a VEVENT or two, and share it here?

[atajadod]

@rianjs here you go

not supported calendar message.zip

[atajadod]

@rianjs if you prefer text:

BEGIN:VCALENDAR
METHOD:REQUEST
PRODID:-//github.com/rianjs/ical.net//NONSGML ical.net 2.2//EN
VERSION:2.0
BEGIN:VEVENT
CLASS:PUBLIC
CREATED;TZID=UTC:20170407T212300
DESCRIPTION:
DTEND;TZID=UTC:20170601T220000
DTSTAMP;TZID=UTC:20170407T212300
DTSTART;TZID=UTC:20170601T210000
LAST-MODIFIED;TZID=UTC:20170407T212300
LOCATION:TED
RRULE:FREQ=DAILY;COUNT=1;BYDAY=TH
SEQUENCE:0
SUMMARY:Travel Request training
TRANSP:Transparent
UID:ca3b59d3-3c28-4840-96ef-e698f14e47bc
X-ALT-DESC:

You have been enrolled in Travel Request training.

\n<
h4>CourseName:\n

Travel Request

END:VEVENT
END:VCALENDAR

[rianjs]

I'm going to put the Z back. It's way more disruptive than I would have imagined. I guess that's why we have standards. :)

[TwistyPop]

Do you have a rough ETA on when it will be reverted to zulu suffix?

[rianjs]

I'd like to say "soon", but I have a lot on my plate right now. The pull request that broke the behavior is linked above, and I'm happy to review, merge, and publish a pull request that unbreaks the Z.

[TwistyPop]

I looked at the PR #261. I have reverted the code and updated the test cases. see PR #296

[rianjs]

Available in:

• 3.0.10-net-core-beta
• 2.3.2

https://www.nuget.org/packages/Ical.Net/