Bug #62169 for iCal-Parser: events timezone is ignored

Fri Oct 15 10:56:10 2010 Vladimir.Marek [...] oracle.com - Ticket created

Subject:	events timezone is ignored
Date:	Fri, 15 Oct 2010 16:55:21 +0200
To:	bug-iCal-Parser [...] rt.cpan.org
From:	Vladimir Marek <Vladimir.Marek [...] oracle.com>

Hi, Thanks for iCal::Parser! After some investigation I have found out that timezone of an event is being ignored while passing the date to DateTime::Format::ICal in the convert_value function. The event looks like this: TYPE: DTEND $VAR1 = { 'value' => '20100903T093000', 'param' => { 'TZID' => 'America/Los_Angeles' } }; DateTime::Format::ICal can parse strings containing timezone in the format "TZID=America/Los_Angeles:20100903T093000". I fixed the issue locally via this patch: --- /tmp/vm156888/x/perl-5.12.2/lib/site_perl/5.12.2/iCal/Parser.pm pá říj 15 16:45:29 2010 +++ /tmp/vm156888/x/perl-5.12.2/lib/site_perl/5.12.2/iCal/Parser.pm.good2 pá říj 15 16:42:12 2010 @@ -229,10 +229,13 @@ my($self,$type,$hash)=@_; my $value=$hash->{value}; + my $this_tz=""; + $this_tz="TZID=$hash->{param}{TZID}:" if defined $hash->{param}{TZID}; return $value unless $value; #should protect from invalid datetimes if ($type eq 'TRIGGER') { #can be date or duration! + $value="$this_tz$value"; return $dfmt->parse_duration($value) if $value =~/^[-+]?P/; return $dfmt->parse_datetime($value)->set_time_zone($self->{tz}); } @@ -241,7 +244,9 @@ map { $h{$_}=$hash->{param}{$_} } keys %{ $hash->{param} }; return \%h; } - return $dfmt->parse_duration($value) if $TYPES{durations}{$type}; + if ($TYPES{durations}{$type}) { + return $dfmt->parse_duration($this_tz.$value); + } return $value unless $TYPES{dates}{$type}; #mozilla calendar bug: negative dates on todos! @@ -255,10 +260,11 @@ # so, handle the exception my $date; eval { - $date=$dfmt->parse_datetime($s)->set_time_zone($self->{tz}); + $date=$dfmt->parse_datetime($this_tz.$s)->set_time_zone($self->{tz}); }; push @dates, $date and next unless $@; die $@ if $@ && $type ne 'DTEND'; + $value="$this_tz$value"; push @dates, $dfmt->parse_datetime(--$value)->set_time_zone($self->{tz}); } But I am not convinced that it's completely correct fix. a) ... if $value =~/^[-+]?P/; will be broken if timezone is prepended to $value b) I do not uderstand what $dfmt->parse_datetime(--$value) does, resp. the --$value ... So with the hope that you'll be able to fix it properly I'm sending what I currently have :) Thank you -- Vlad

Fri May 31 13:11:09 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

From:

bruce [...] momjian.us

I have successfully used this patch against version 1.16 with a Google calendar file. Events with specified non-local TZID times were not being properly processed before this patch. I suggest this patch be cleaned up and merged into an updated version.

Fri May 31 13:11:09 2013 The RT System itself - Status changed from 'new' to 'open'

Fri May 31 14:29:57 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

From:

bruce [...] momjian.us

Just to clarify, the TZID timezone value on this iCal line in version 1.16 is currently ignored: DTSTART;TZID=Asia/Karachi:20130520T180000 The patch fixes this. While Google calendar does not use this format for single events (those are mapped to local or UTC), it does output these TZID entries for recurring events.

Thu Jun 06 19:00:49 2013 Vladimir.Marek [...] oracle.com - Correspondence added

Subject:	Re: [rt.cpan.org #62169] events timezone is ignored
Date:	Fri, 7 Jun 2013 01:00:32 +0200
To:	"https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg via RT" <bug-iCal-Parser [...] rt.cpan.org>
From:	Vladimir Marek <Vladimir.Marek [...] Oracle.COM>

Hi, Show quoted text

> I have successfully used this patch against version 1.16 with a Google > calendar file. Events with specified non-local TZID times were not > being properly processed before this patch. I suggest this patch be > cleaned up and merged into an updated version.

When I filed the bug, I was not sure whether the patch I have is correct. At this time I am sure it is not correct. I don't remember any details today, but it seems that the recurring evens are wrong around daylight savings time change. I think the problem is that DST does change in a different moment in each country. My patch translates the date into local time zone and then again computes the recurrence of the event in local timezone. But it has been some time and I might be all wrong ... Still this is the best thing I have. Cheers -- Vlad

Sat Jun 15 13:26:57 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

I have given the idea more thought as well. Your code might work because if the timezone changes in the origin country, like Pakistan, we are going to map the time entry to Pakistan time, then to local time. I thought that might handle the DST issue. One larger issue is that your patch only works because the TZID happens to be something Perl understands, e.g. America/New_York. (Google generates that format.) If it is one of those odd TZIDs, like created on Windows, I am afraid your patch is not going to work. That is what those GMT+0500 stuff is that you had comments about in your patch. I am wondering if your patch should only try its magic if it thinks the TZID is something Perl can understand, and fall back to trying to find a GMT offset number in the TZID. Frankly, I am surprised at the poor iCal parsing support available in any language, and that Perl's isn't fully functional, e.g. doesn't understand VTIMEZONE.

Sat Jun 15 13:54:10 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

Just to summarize, the TZID is supposed to map to the VTIMEZONE section at the top of the iCal file, which contains GMT offsets and DST information. Because we don't have VTIMEZONE support, your patch punts the TZID for Perl to process. That works fine, but only if the TZID is something Perl can understand.

Sun Jun 16 11:41:36 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

Here is an example of a TZID that contains a GMT offset: DTSTART;TZID="(GMT-05.00) Eastern Time (US & Canada)":20090205T100000 I don't know how many TZIDs have either a designation that Perl understands or a GMT offset, but we should support both if we can.

Sun Jun 16 12:08:00 2013 https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg - Correspondence added

From:

bruce [...] momjian.us

Here is an example of an entry that isn't understood by Perl, and doesn't have a GMT offset: DTSTART;TZID="Chennai, Kolkata, Mumbai, New Delhi":20090220T200000 I think we have to give up on those until we support VTIMEZONE.

Sun Mar 05 21:28:03 2017 https://me.yahoo.com/lkjasdifjoijwe2323#348d4 - Correspondence added

From:

bruce [...] momjian.us

On Sun Jun 16 12:08:00 2013, https://www.google.com/accounts/o8/id?id=AItOawlsFr716ITHX5fFqEoy1RDF6vBODDmICWg wrote: Show quoted text

> Here is an example of an entry that isn't understood by Perl, and > doesn't have a GMT offset: > > DTSTART;TZID="Chennai, Kolkata, Mumbai, New Delhi":20090220T200000 > > I think we have to give up on those until we support VTIMEZONE.

I can confirm I still need this patch on version 1.21. I can provide a reproduceable test case if desired.

Sun Mar 05 21:34:47 2017 https://me.yahoo.com/lkjasdifjoijwe2323#348d4 - Correspondence added

From:

bruce [...] momjian.us

On Sun Mar 05 21:28:03 2017, https://me.yahoo.com/lkjasdifjoijwe2323#348d4 wrote: Show quoted text

> > I think we have to give up on those until we support VTIMEZONE.

> > I can confirm I still need this patch on version 1.21. I can provide > a reproduceable test case if desired.

Attached is an updated patch for version 1.21.

Subject:

TZID.1.21.diff

62169 events timezone is ignored Hi, Thanks for iCal::Parser! After some investigation I have found out that timezone of an event is being ignored while passing the date to DateTime::Format::ICal in the convert_value function. The event looks like this: TYPE: DTEND $VAR1 = { 'value' => '20100903T093000', 'param' => { 'TZID' => 'America/Los_Angeles' } }; DateTime::Format::ICal can parse strings containing timezone in the format "TZID=America/Los_Angeles:20100903T093000". I fixed the issue locally via this patch: *** ./Parser.pm.orig 2016-12-08 05:53:40.000000000 -0500 --- ./Parser.pm 2017-03-05 18:35:18.818392189 -0500 *************** *** 232,237 **** --- 232,239 ---- my($self,$type,$hash)=@_; my $value=$hash->{value}; + my $this_tz=""; + $this_tz="TZID=$hash->{param}{TZID}:" if defined $hash->{param}{TZID}; return $value unless $value; #should protect from invalid datetimes # Trim common types that do not allow whitespaces *************** *** 240,245 **** --- 242,248 ---- if ($type eq 'TRIGGER') { #can be date or duration! + $value="$this_tz$value"; return $dfmt->parse_duration($value) if $value =~/^[-+]?P/; return $dfmt->parse_datetime($value)->set_time_zone($self->{tz}); } *************** *** 248,254 **** map { $h{$_}=$hash->{param}{$_} } keys %{ $hash->{param} }; return \%h; } ! return $dfmt->parse_duration($value) if $TYPES{durations}{$type}; return $value unless $TYPES{dates}{$type}; #mozilla calendar bug: negative dates on todos! --- 251,259 ---- map { $h{$_}=$hash->{param}{$_} } keys %{ $hash->{param} }; return \%h; } ! if ($TYPES{durations}{$type}) { ! return $dfmt->parse_duration($this_tz.$value); ! } return $value unless $TYPES{dates}{$type}; #mozilla calendar bug: negative dates on todos! *************** *** 262,271 **** # so, handle the exception my $date; eval { ! $date=$dfmt->parse_datetime($s)->set_time_zone($self->{tz}); }; push @dates, $date and next unless $@; die $@ if $@ && $type ne 'DTEND'; push @dates, $dfmt->parse_datetime(--$value)->set_time_zone($self->{tz}); } --- 267,277 ---- # so, handle the exception my $date; eval { ! $date=$dfmt->parse_datetime($this_tz.$s)->set_time_zone($self->{tz}); }; push @dates, $date and next unless $@; die $@ if $@ && $type ne 'DTEND'; + $value="$this_tz$value"; push @dates, $dfmt->parse_datetime(--$value)->set_time_zone($self->{tz}); }

Wed Nov 27 13:46:15 2019 bruce [...] momjian.us - Correspondence added

I have developed a further refined patch against 1.21 that fixes another time zone bug. If you have a recurring events, the events must be at the same time in the origin time zone, even if daylight saving time changes happen in the origin or local time zone. This cumulative patch includes the above fixes, and a fix for this by moving the datetime back to the origin time zone before generating recurring entries. It also improves the above patches by using a cleaner fix. This patch still requires that ics file time zone specifications be recognized to the operating system. I could use eval{} to avoid errors, but I would rather know when an entry is not properly time-zone adjusted. It would be nice to have this in a released version of the module.

Subject:

TZID.1.21-current.diff

--- lib/iCal/Parser.pm.orig 2019-11-12 11:40:17.083546619 -0500 +++ lib/iCal/Parser.pm 2019-11-27 13:31:42.131158257 -0500 @@ -102,6 +102,9 @@ my %e=(idref=>$self->_cur_calid); + my $orig_tzid = $event->{properties}->{DTSTART}[0]->{param}{TZID}; + my $shift_to_local_tz = 0; + $self->map_properties(\%e,$event); $self->add_objects($event,\%e); @@ -130,6 +133,15 @@ return; } if (my $recur=delete $e{RRULE}) { + if (defined($orig_tzid) && $orig_tzid ne $self->{tz}->name) { + # Use the original time zone so future events are the same + # time of day in that time zone, independent of daylight + # saving time changes in that time zone. This might change + # the time of day in the local time zone. + $start->set_time_zone($orig_tzid); + # shift to local time zone later + $shift_to_local_tz = 1; + } $set=$dfmt->parse_recurrence(recurrence=>$recur, dtstart=>$start, #cap infinite repeats until =>$self->{span}->end); @@ -161,20 +173,21 @@ return if defined $set->count && $set->count==0; if (my $dates=delete $e{'EXDATE'}) { - #mozilla/sunbird set exdate to T00..., so, get first start date - #and set times on exdates - my $d=$set->min; - my $exset=DateTime::Set->from_datetimes - (dates=>[ - map {$_->set(hour=>$d->hour,minute=>$d->minute, - second=>$d->second) - } @$dates]); + if ($shift_to_local_tz) { + for my $dt (@$dates) { + $dt->set_time_zone($orig_tzid); + } + } $set=$set ->complement(DateTime::Set->from_datetimes(dates=>$dates)); } $set=$set->intersection($self->{span}) if $self->{span}; my $iter=$set->iterator; while (my $dt=$iter->next) { + # convert "floating" time zone to original time zone, then local time zone + $dt->set_time_zone($orig_tzid)->set_time_zone($self->{tz}) + if ($shift_to_local_tz); + #bug found by D. Sweet. Fix alarms on entries #other than first my $new_event={%e,DTSTART=>$dt,DTEND=>$dt+$duration}; @@ -232,6 +245,7 @@ my($self,$type,$hash)=@_; my $value=$hash->{value}; + my $entry_tz = defined($hash->{param}{TZID}) ? $hash->{param}{TZID} : $self->{tz}; return $value unless $value; #should protect from invalid datetimes # Trim common types that do not allow whitespaces @@ -241,7 +255,7 @@ if ($type eq 'TRIGGER') { #can be date or duration! return $dfmt->parse_duration($value) if $value =~/^[-+]?P/; - return $dfmt->parse_datetime($value)->set_time_zone($self->{tz}); + return $dfmt->parse_datetime($value)->set_time_zone($entry_tz)->set_time_zone($self->{tz}); } if ($TYPES{hash}{$type}) { my %h=(value=>$value); @@ -260,14 +274,11 @@ # I have a sample calendar "Employer Tax calendar" # which has an allday event ending on 20040332! # so, handle the exception - my $date; - eval { - $date=$dfmt->parse_datetime($s)->set_time_zone($self->{tz}); - }; + my $date=$dfmt->parse_datetime($s)->set_time_zone($entry_tz)->set_time_zone($self->{tz}); push @dates, $date and next unless $@; die $@ if $@ && $type ne 'DTEND'; push @dates, - $dfmt->parse_datetime(--$value)->set_time_zone($self->{tz}); + $dfmt->parse_datetime(--$value)->set_time_zone($entry_tz)->set_time_zone($self->{tz}); } return @dates; }

Wed Nov 27 13:46:16 2019 bruce [...] momjian.us - Fixed in 1.21 added