for files with multiple tzids in the VTIMEZONE, choose the last

[sujal]

This is looking to resolve an issue with node-ical parsing https://www.washougal.k12.wa.us/hathaway/calendars/category/school-events/?post_type=tribe_events&ical=1&eventDisplay=list

Here are the key facts as I understand right now:

• The school calendar feed had multiple TZIDs defined in a single VTIMEZONE block.
  • This seems to be against the spec
  • The validator at icalendar.org considers this feed to be valid. ¯\_(ツ)_/¯
• The node-ical library doesn’t handle multiple TZIDs - it expects a string, gets an array and causes the error when trying to use moment-timezone with the array.
• The workaround detects the object instead of a string, then simply selects the last tzid in the block.
  • This maybe is too naive - might be better to see which TZID is defined with more subblocks or more precision - e.g. take one with STANDARD & DAYLIGHT blocks over one that has only a STANDARD block… not sure if that’s better, though. Thought that feels a little arbitrary, too.

Appreciate any feedback on how best to account for this. If you have any insight on why the icalendar validator thinks the feed is ok, even though it seems to violate my understanding of the VTIMEZONE requirements, that would also be interesting to understand.

[jens-maus]

Well, I am actually not sure if we should really integrate that PR. The definition of VTIMEZONE is quite clear here:

'tzid' is REQUIRED, but MUST NOT occur more than once.

Thus, if you have a ics/ical with a VTIMEZONE that contains more than one TZID it is - per definition - undefined or better said invalid. Thus, IMHO the creator of that ics/ical should be notified instead to correct that error instead of assuming a parsing library like node-ical should contain workarounds like that.

[sujal]

Well, I am actually not sure if we should really integrate that PR. The definition of VTIMEZONE is quite clear here:

'tzid' is REQUIRED, but MUST NOT occur more than once.

Thus, if you have a ics/ical with a VTIMEZONE that contains more than one TZID it is - per definition - undefined or better said invalid. Thus, IMHO the creator of that ics/ical should be notified instead to correct that error instead of assuming a parsing library like node-ical should contain workarounds like that.

Do you know of another validator that parses this correctly? Would help make it easier to show that it's out of compliance.

For this particular issue, what would be a better option. I believe the library shouldn't throw this particular exception for this scenario - there should at least be a better exception. I can at least change it to make that clearer to users of the library. Happy to help with that if you have a recommendation.

[jens-maus]

For this particular issue, what would be a better option. I believe the library shouldn't throw this particular exception for this scenario - there should at least be a better exception. I can at least change it to make that clearer to users of the library. Happy to help with that if you have a recommendation.

Well, I agree that we could probably do a bit better than just letting node-ical run into a nodejs error. Thus, to me the only question that remains is: Should we really take the last TZID in the row or should we simply take the first one and ignore any subsequent?!? And potentially, we could add some runtime warning to let users know that something is fishy with that particular ics file.