golang-ical icon indicating copy to clipboard operation
golang-ical copied to clipboard

[BUG] AddTimezone results into an invalid .ics

Open GustavoOS opened this issue 11 months ago • 6 comments

Current Behavior:

        calendar := ics.NewCalendar()
	calendar.SetMethod(ics.MethodPublish)
	calendar.SetProductId(CALENDAR_PRODID)
	calendar.AddTimezone("America/Sao_Paulo")

results into the following .ics file

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//my-site.com//Agenda//PT
METHOD:PUBLISH
BEGIN:VTIMEZONE
TZID:America/Sao_Paulo
END:VTIMEZONE
END:VCALENDAR

This is an invalid .ics because it fails to follow RFC 5545 3.6.5, which states that At least one STANDARD or DAYLIGHT property must be defined.

Expected Behavior:

BEGIN:VCALENDAR
VERSION:2.0
METHOD:PUBLISH
PRODID:-//my-site.com//Agenda//PT
BEGIN:VTIMEZONE
TZID:America/Sao_Paulo
LAST-MODIFIED:20240422T053451Z
TZURL:https://www.tzurl.org/zoneinfo-outlook/America/Sao_Paulo
X-LIC-LOCATION:America/Sao_Paulo
BEGIN:STANDARD
TZNAME:-03
TZOFFSETFROM:-0300
TZOFFSETTO:-0300
DTSTART:19700101T000000
END:STANDARD
END:VTIMEZONE
END:VCALENDAR

Links

-.ics Validator

GustavoOS avatar Jan 10 '25 15:01 GustavoOS

I don't do any sanity checking, those fields would have to be added by the using application. What do you suggest happens here?

arran4 avatar Jan 11 '25 08:01 arran4

My suggestion is that AddTimezone would generate the following code

BEGIN:VTIMEZONE
TZID:America/Sao_Paulo
LAST-MODIFIED:20240422T053451Z
TZURL:https://www.tzurl.org/zoneinfo-outlook/America/Sao_Paulo
X-LIC-LOCATION:America/Sao_Paulo
BEGIN:STANDARD
TZNAME:-03
TZOFFSETFROM:-0300
TZOFFSETTO:-0300
DTSTART:19700101T000000
END:STANDARD
END:VTIMEZONE

Ive set this function as a workaround

import (
	"fmt"
	ics "github.com/arran4/golang-ical"
	"log/slog"
	"net/http"
)
var IcalTimestampFormatUtc = "20060102T150405Z"

func SetTimeZone(calendar *ics.Calendar, location string) {
	url := fmt.Sprintf("https://www.tzurl.org/zoneinfo-outlook/%s", location)
	res, err := http.Get(url)
	if err != nil {
		slog.Warn("Timezone not found")
		return
	}
	tzCal, err := ics.ParseCalendar(res.Body)
	if err != nil {
		slog.Warn("Error parsing timezone", err)
		return
	}
	timeZones := tzCal.Timezones()
	for _, tz := range timeZones {
		calendar.AddVTimezone(tz)
	}
}

GustavoOS avatar Jan 11 '25 13:01 GustavoOS

It's been a while since I checked out this project, and this kind of relates to my issue #74, and @GustavoOS is right, the TZID field shouldn't exist on the top level calendar object in general, only a VTIMEZONE. This is what the spec says on the topic:

Property Name

TZID

Purpose

This property specifies the text value that uniquely identifies the "VTIMEZONE" calendar component in the scope of an iCalendar object.

Property Parameters

IANA and non-standard property parameters can be specified on this property.

Conformance

This property MUST be specified in a "VTIMEZONE" calendar component.

— https://icalendar.org/iCalendar-RFC-5545/3-8-3-1-time-zone-identifier.html

And then the only fields that actually use that TZID property should be Date-Time fields.

@GustavoOS I like your workaround, it gets the job done with a minimal change! @arran4 This is basically also what I was looking for in #74 as well.


On a related note, after trying this workaround, I ended up running into another issues using the WithTZID attribute for formatting dates. According to the iCal spec for Date-Time fields, when using the TZID: prefix you shouldn't be used if the UTC time specifier is include in the time string:

The "TZID" property parameter MUST NOT be applied to DATE-TIME properties whose time values are specified in UTC.

FORM #3

DATE WITH LOCAL TIME AND TIME ZONE REFERENCE

The date and local time with reference to time zone information is identified by the use the "TZID" property parameter to reference the appropriate time zone definition. "TZID" is discussed in detail in Section 3.2.19. For example, the following represents 2:00 A.M. in New York on January 19, 1998:

TZID=America/New_York:19980119T020000

— https://icalendar.org/iCalendar-RFC-5545/3-3-5-date-time.html

And after testing it personally, if you include both, the date will end up being treated like it's formatted relative timezone.

I was trying to figure out the best way to handle this, and it seems like the ideal would be for this package to format locally when using the WithTZID attribute. Some potential approaches I thought of are:

  • Assume the SetStartAt, SetEndAt, etc function are accepting a date already in the expected timezone when using the option and just format using icalTimestampFormatLocal.
  • Use time.LoadLocation on the WithTZID passed ID, and use that for the time.In argument.
  • Add new utilities that mirror the existing date setters like this:
func (cb *ComponentBase) SetStartAtInTimezone(t time.Time, tz *time.Location, params ...PropertyParameter) {
	params = append(params, WithTZID(tz.String()))
	cb.SetProperty(ComponentPropertyDtStart, t.In(tz).Format(icalTimestampFormatLocal), params...)
}

Or there could be some combination of these, I haven't fully thought through it.

My local workaround currently is just to manually call event.SetProperty after formatting the timestamp manually:

const (
    // Local copy of this
	icalTimestampFormatLocal = "20060102T150405"
)

showtimeTimezone, err := common.LoadLocation(ctx, showtimeTimezoneID)
if err != nil {
	return nil, errors.Wrap(err, "loading showtime timezone")
}

startsAtInTimezone := startsAt.In(showtimeTimezone)
// event.SetStartAt(startsAtInTimezone, ics.WithTZID(showtimeTimezoneID))
event.SetProperty(ics.ComponentPropertyDtStart, startsAtInTimezone.Format(icalTimestampFormatLocal), ics.WithTZID(showtimeTimezoneID))

Do you have any thoughts @arran4? I'd be happy to put up a PR to handle this alternative formatting if you're interested in a specific one of these approaches. Because as it stands, the WithTZID attribute is kind of useless if there's no built-in way to format the timestamps properly.

Also let me know if this is too off topic and I can make a separate issue.

csandman avatar May 21 '25 04:05 csandman

Actually, scratch the first part of my last comment. I'm now realizing it actually is adding the TZID field inside a VTIMEZONE element. However, I do agree with @GustavoOS that it's pretty useless to have a VTIMEZONE object with only an ID, as it can't really be used for anything. Also it would be invalid.

The second part of my comment still stands though, it would be great if there was a better way to actually use the timezones included in the calendar by setting date-time values with it. Honestly, at least just the DTSTART and DTEND attributes, because those are the ones that can actually affect how a calendar event is displayed.

csandman avatar May 22 '25 16:05 csandman

I'm a bit snowed under right now I will have to look at this in detail between contracts. - If urgent we can work something out.

arran4 avatar May 22 '25 23:05 arran4

I'm a bit snowed under right now I will have to look at this in detail between contracts. - If urgent we can work something out.

It's not super urgent because I did get that workaround working! It just seems like something that would be better if supported internally, instead of having to manually call event.SetProperty, and copying icalTimestampFormatLocal to my own project.

csandman avatar May 28 '25 00:05 csandman