Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading in-file timezones #10

Open
WhyNotHugo opened this issue Oct 30, 2021 · 10 comments
Open

Reading in-file timezones #10

WhyNotHugo opened this issue Oct 30, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@WhyNotHugo
Copy link
Contributor

I've a lot of events that look like this:

BEGIN:VCALENDAR
PRODID:-//Radicale//NONSGML Radicale Server//EN
VERSION:2.0
BEGIN:VTIMEZONE
TZID:local
X-RADICALE-NAME:local
BEGIN:STANDARD
DTSTART:20090314T230000
TZNAME:ART
TZOFFSETFROM:-0200
TZOFFSETTO:-0300
X-RADICALE-NAME:local
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
SUMMARY:Redacted summary
DTSTART;TZID="local;VALUE=DATE-TIME":20150315T123000
DTEND;TZID="local;VALUE=DATE-TIME":20150315T193000
DTSTAMP:20150315T103349Z
UID:762CJK7VMGEBP416C5TE3V6KFHVJXY2Y6IPD
SEQUENCE:0
X-RADICALE-NAME:762CJK7VMGEBP416C5TE3V6KFHVJXY2Y6IPD.ics
END:VEVENT
END:VCALENDAR

This doesn't seem to be invalid; the timezone is specific in the file. However, parsing these returns error unknown time zone local.

@emersion
Copy link
Owner

It seems like it's incorrect to assume that TZID is a time zone name. It's just an identifier used to refer to the timezone in the rest of the file. Here "local" is used, but "foo" or "bar" could be used as well.

The TZOFFSETFROM and TZOFFSETTO need to be used to parse the timezone instead.

@emersion emersion added the enhancement New feature or request label Nov 3, 2021
@WhyNotHugo
Copy link
Contributor Author

I've been investigating how to address this a bit more. We need to parse the
VTIMEZONE and create a Location object.

There's no simple constructor for these, only LoadLocationFromTZData. It
takes binary tzdata, from what I understand in tzfile(5) format.

Honestly, I think converting VTIMEZONE into tzfile into Location is a bit
of too much dancing. Serialising to a binary format as an intermediate is a lot
of ridiculous complexity for no real value. But I'm not sure there's any other
way to create a Location instance if there's no generic constructor, and most
of the API around Location (and zone, etc) is private.

I considered creating a location using FixedZone. This would work great for
de-serialising, printing, etc, but sounds like it might result in more issues
further down the line:

  • When changing a time, we should re-create the Location instance, since the
    new time might have a different offset.
  • We should never serialise the Location itself, and actually always keep the
    VTIMEZONE around.
  • Recurrences are very likely to break. E.g.: Every day at 10:00 on a timezone
    with DST will not yield the expected result.

I can imagine the recurrence issues being the biggest dealbreaker.

Any suggestions?

@emersion
Copy link
Owner

I agree with all you've written, and I don't have good ideas to solve this.

@WhyNotHugo
Copy link
Contributor Author

I've proposed adding a new public constructor for Location upstream (since the same issue is blocking other related projects too): golang/go#49951

@dilyanpalauzov
Copy link

DTSTART;TZID="local;VALUE=DATE-TIME":20150315T123000 Aren’t the quotes wrong here? I mean, the only property parameter name is TZID and its value is local;VALUE=DATE-TIME. For this value there is no definition in the VTIMEZONE component.

@WhyNotHugo
Copy link
Contributor Author

Looks like https://github.com/martin-sucha/timezones/blob/main/timezones.go can be used as a reference to encode system TZDATA into VTIMEZONE.

Also, https://github.com/martin-sucha/vtimezone2tzif can be used to convert VTIMEZONE into TZDATA into time.Location.

The process isn't ideal; there's lots of pointless conversions to intermediate states, but it's the best that can be done with go's existing API, and it seems that changing isn't desirable right now.

@Luzifer
Copy link

Luzifer commented Feb 28, 2024

This sadly is a feature required to parse Office365 calendars too. The Exchange Server always embeds a timezone into the calendar with kinda random names (Romance Standard Time in the test calendar below, W. Europe Standard Time in my personal one even though both of them are just Europe/Berlin time calendars) causing a parse using this library to fail as of every event having a TZID reference to the embedded (IMHO useless and kinda broken) timezone.

Test-Calendar with one event in Office365
BEGIN:VCALENDAR
METHOD:PUBLISH
PRODID:Microsoft Exchange Server 2010
VERSION:2.0
X-WR-CALNAME:test
BEGIN:VTIMEZONE
TZID:Romance Standard Time
BEGIN:STANDARD
DTSTART:16010101T030000
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-1SU;BYMONTH=10
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:16010101T020000
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-1SU;BYMONTH=3
END:DAYLIGHT
END:VTIMEZONE
BEGIN:VEVENT
DESCRIPTION:\n
UID:040000008200E00074C5B7101A82E00800000000CF1B0975336ADA01000000000000000
 010000000C0B11B2D87B2B24C9725C1A358375FE2
SUMMARY:test
DTSTART;TZID=Romance Standard Time:20240228T130000
DTEND;TZID=Romance Standard Time:20240228T132500
CLASS:PUBLIC
PRIORITY:5
DTSTAMP:20240228T104759Z
TRANSP:OPAQUE
STATUS:CONFIRMED
SEQUENCE:0
LOCATION:
X-MICROSOFT-CDO-APPT-SEQUENCE:0
X-MICROSOFT-CDO-BUSYSTATUS:BUSY
X-MICROSOFT-CDO-INTENDEDSTATUS:BUSY
X-MICROSOFT-CDO-ALLDAYEVENT:FALSE
X-MICROSOFT-CDO-IMPORTANCE:1
X-MICROSOFT-CDO-INSTTYPE:0
X-MICROSOFT-DONOTFORWARDMEETING:FALSE
X-MICROSOFT-DISALLOW-COUNTER:FALSE
X-MICROSOFT-REQUESTEDATTENDANCEMODE:DEFAULT
X-MICROSOFT-ISRESPONSEREQUESTED:FALSE
END:VEVENT
END:VCALENDAR

@wansing
Copy link

wansing commented Jul 8, 2024

I had the same issue. I ended up pre-processing the calendar events, deleting TZID parameters which can't be loaded with LoadLocation. This is really hacky, it will produce wrong results, and I should probably cache LoadLocation results, but might still be helpful for someone:

for _, propid := range []string{ical.PropDateTimeStart, ical.PropDateTimeEnd} {
	prop := event.Props.Get(propid)
	if prop != nil {
		tzid := prop.Params.Get(ical.PropTimezoneID)
		if tzid != "" {
			_, err := time.LoadLocation(tzid)
			if err != nil {
				prop.Params.Del(ical.PropTimezoneID)
			}
		}
	}
}

start, err := event.DateTimeStart(location) // works now
// ...

@truecrunchyfrog
Copy link

truecrunchyfrog commented Jul 21, 2024

Looks like https://github.com/martin-sucha/timezones/blob/main/timezones.go can be used as a reference to encode system TZDATA into VTIMEZONE.

Also, https://github.com/martin-sucha/vtimezone2tzif can be used to convert VTIMEZONE into TZDATA into time.Location.

The process isn't ideal; there's lots of pointless conversions to intermediate states, but it's the best that can be done with go's existing API, and it seems that changing isn't desirable right now.

Did you find how to generate the VTIMEZONE from a time.Location, if you happened to look deeper into it?
I'm stuck on how to split the timezones.Zone.Offset into TZOFFSETFROM and TZOFFSETTO.

And also how to get a timezones.Template from a timezone name (e.g. America/New_York). What i'm thinking is looking into a tzif binary and finding the timezone's data that way, and then passing it to timezones.LoadTZData().

@WhyNotHugo
Copy link
Contributor Author

Sorry, I didn't look into this any further and don't have anything new to add.

Looking at the timezones package that you linked, it sounds like you're going to have to parse the VTIMEZONE yourself and create the new instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants