Better preserve timezones in ical

lib/ice_cube/builders/ical_builder.rb

@@ -35,13 +35,46 @@ def self.ical_utc_format(time)
 
     def self.ical_format(time, force_utc)
       time = time.dup.utc if force_utc
+
+      # Keep the time zone. strftime would serialize the abbreviated zone name
+      # (eg. EEST), which is not reversible, as the same abbreviation is shared
+      # by several zones. This would result in issues in parsing.
+      if time.respond_to?(:time_zone)
+        return ";TZID=#{tzid_for(time.time_zone)}:#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%S")}" # local time specified
+      end
+
       if time.utc?
         ":#{IceCube::I18n.l(time, format: "%Y%m%dT%H%M%SZ")}" # utc time
       else
-        ";TZID=#{IceCube::I18n.l(time, format: "%Z:%Y%m%dT%H%M%S")}" # local time specified
+        # Convert to UTC as TZID=+xxxx format is not recognized by JS libraries
+        warn_missing_time_zone
+        utc_time = time.dup.utc
+        ":#{IceCube::I18n.l(utc_time, format: "%Y%m%dT%H%M%SZ")}" # converted to utc time
+      end
+    end
+
+    # RFC 5545 leaves the TZID registry unspecified but points implementers at
+    # the IANA (Olson) database, which is what other iCalendar implementations
+    # expect. ActiveSupport zones may carry a Rails-specific label instead (eg.
+    # "Eastern Time (US & Canada)"), so prefer the underlying IANA identifier;
+    # ActiveSupport still resolves those, and everyone else can too.
+    def self.tzid_for(zone)
+      if zone.respond_to?(:tzinfo) && zone.tzinfo.respond_to?(:name)
+        zone.tzinfo.name
+      else
+        zone.name
       end
     end
 
+    # A schedule built from plain Time objects hits this on every serialization,
+    # so warn once per process rather than once per occurrence.
+    def self.warn_missing_time_zone
+      return if @missing_time_zone_warned
+
+      @missing_time_zone_warned = true
+      warn "IceCube: Time object does not have timezone info. Coercing into UTC: #{caller(2..2).first}"
+    end
+
     def self.ical_duration(duration)
       hours = duration / 3600
       duration %= 3600

lib/ice_cube/parsers/ical_parser.rb

@@ -14,19 +14,23 @@ def self.schedule_from_ical(ical_string, options = {})
       end
 
       lines.each do |line|
-        (property, value) = line.split(":")
-        (property, _tzid) = property.split(";")
+        (name_and_params, value) = split_content_line(line)
+        next if value.nil?
+
+        (property, *params) = split_unquoted(name_and_params, ";")
+        tzid = tzid_from_params(params)
+
         case property
         when "DTSTART"
-          data[:start_time] = TimeUtil.deserialize_time(value)
+          data[:start_time] = TimeUtil.deserialize_time_with_zone(value, tzid)
         when "DTEND"
-          data[:end_time] = TimeUtil.deserialize_time(value)
+          data[:end_time] = TimeUtil.deserialize_time_with_zone(value, tzid)
         when "RDATE"
           data[:rtimes] ||= []
-          data[:rtimes] += value.split(",").map { |v| TimeUtil.deserialize_time(v) }
+          data[:rtimes] += value.split(",").map { |v| TimeUtil.deserialize_time_with_zone(v, tzid) }
         when "EXDATE"
           data[:extimes] ||= []
-          data[:extimes] += value.split(",").map { |v| TimeUtil.deserialize_time(v) }
+          data[:extimes] += value.split(",").map { |v| TimeUtil.deserialize_time_with_zone(v, tzid) }
         when "DURATION"
           data[:duration] # FIXME
         when "RRULE"
@@ -37,6 +41,50 @@ def self.schedule_from_ical(ical_string, options = {})
       Schedule.from_hash data
     end
 
+    # Split a content line into its property part (name and parameters) and its
+    # value, at the first colon that is not inside a quoted parameter value.
+    # Parameter values are quoted precisely so they may contain a colon
+    # (RFC 5545 section 3.1), as in DTSTART;TZID="GMT+05:00":20130101T090000.
+    # Returns a nil value for a line with no colon at all.
+    def self.split_content_line(line)
+      in_quotes = false
+      line.each_char.with_index do |char, index|
+        case char
+        when '"' then in_quotes = !in_quotes
+        when ":" then return [line[0, index], line[(index + 1)..]] unless in_quotes
+        end
+      end
+      [line, nil]
+    end
+
+    # Split on +delimiter+, ignoring delimiters inside a quoted value.
+    def self.split_unquoted(string, delimiter)
+      parts = [+""]
+      in_quotes = false
+      string.each_char do |char|
+        in_quotes = !in_quotes if char == '"'
+        if char == delimiter && !in_quotes
+          parts << +""
+        else
+          parts.last << char
+        end
+      end
+      parts
+    end
+
+    # Find the TZID parameter among a property's parameters. TZID is not
+    # necessarily the first parameter (DTSTART;VALUE=DATE-TIME;TZID=... is
+    # equally valid), parameter names are case-insensitive, and the value may be
+    # double-quoted (RFC 5545 sections 3.1 and 3.2.19).
+    def self.tzid_from_params(params)
+      param = params.find { |p| p =~ /\ATZID=/i }
+      return nil unless param
+
+      tzid = param.split("=", 2).last.to_s.strip
+      tzid = tzid[1..-2] if tzid.length >= 2 && tzid.start_with?('"') && tzid.end_with?('"')
+      tzid.empty? ? nil : tzid
+    end
+
     def self.rule_from_ical(ical)
       raise ArgumentError, "empty ical rule" if ical.nil?
 

lib/ice_cube/time_util.rb

@@ -87,7 +87,8 @@ def self.serialize_time(time)
       case time
       when Time, Date
         if time.respond_to?(:time_zone)
-          {time: time.utc, zone: time.time_zone.name}
+          # avoid .utc as it changes the object timezone
+          {time: time.getutc, zone: time.time_zone.name}
         else
           time
         end
@@ -113,6 +114,96 @@ def self.deserialize_time(time_or_hash)
       end
     end
 
+    # Deserialize a time in the named time zone (an iCalendar TZID).
+    #
+    # TZID values are not registered by RFC 5545, which points implementers at
+    # the IANA (Olson) database instead, so +tzid+ is resolved against whichever
+    # zone database is available: ActiveSupport first, then TZInfo. ice_cube has
+    # no runtime dependency on either, and a TZID may also simply be unknown
+    # (Outlook, for example, emits Windows zone names such as "Eastern Standard
+    # Time"). When the zone cannot be resolved this warns and falls back to
+    # zone-less parsing rather than raising, so an unrecognised TZID degrades to
+    # the pre-TZID behaviour instead of breaking the parse.
+    def self.deserialize_time_with_zone(time_value, tzid)
+      return deserialize_time(time_value) if tzid.nil? || tzid.empty?
+
+      unless (zone = find_zone(tzid))
+        reason = if zone_database_available?
+          "unknown TZID #{tzid.inspect} (an IANA identifier such as \"America/New_York\" is expected)"
+        else
+          "cannot resolve TZID #{tzid.inspect} without a time zone database (require \"active_support/time\" or \"tzinfo\")"
+        end
+        warn "IceCube: #{reason}; parsing #{time_value.to_s.strip.inspect} " \
+             "without a time zone at: #{caller(1..1).first}"
+        return deserialize_time(time_value)
+      end
+
+      time_in_zone(time_value, zone) || deserialize_time(time_value)
+    end
+
+    # Whether any time zone database ice_cube knows how to use is loaded.
+    def self.zone_database_available?
+      (Time.respond_to?(:find_zone) || defined?(TZInfo::Timezone)) ? true : false
+    end
+
+    # Look up an IANA time zone identifier, preferring ActiveSupport (which also
+    # accepts Rails' own zone labels) and falling back to TZInfo. Returns nil
+    # when neither is loaded or the identifier is not recognised.
+    def self.find_zone(tzid)
+      if Time.respond_to?(:find_zone)
+        Time.find_zone(tzid)
+      elsif defined?(TZInfo::Timezone)
+        begin
+          TZInfo::Timezone.get(tzid)
+        rescue
+          nil
+        end
+      end
+    end
+
+    # Interpret an iCalendar date-time string as a wall clock reading in +zone+.
+    # A trailing "Z" marks the value as UTC, in which case it is converted into
+    # the zone rather than read as local time. Returns nil if the value cannot
+    # be interpreted, leaving the caller to fall back.
+    def self.time_in_zone(time_value, zone)
+      time_value = time_value.to_s.strip
+      if zone.respond_to?(:parse) # ActiveSupport::TimeZone
+        zone.parse(time_value)
+      else # TZInfo::Timezone
+        time_in_tzinfo_zone(time_value, zone)
+      end
+    rescue
+      nil
+    end
+
+    # TZInfo gives the correct offset for the instant, but only ActiveSupport
+    # zones survive as a zone on the Time object, so occurrences after a DST
+    # transition keep the start time's offset rather than following the zone.
+    # Note that TZInfo returns a TimeWithOffset whose #zone is an abbreviation
+    # (eg. "EST"); match_zone treats that as a system-local time and would
+    # relocate every occurrence into the system zone, so flatten it to a plain
+    # Time carrying just the offset.
+    def self.time_in_tzinfo_zone(time_value, zone)
+      time = Time.parse(time_value)
+      local = if time_value.end_with?("Z")
+        zone.utc_to_local(time)
+      else
+        zone.local_time(*CLOCK_VALUES.map { |unit| time.public_send(unit) })
+      end
+      warn_tzinfo_dst_limitation
+      Time.new(local.year, local.month, local.day, local.hour, local.min, local.sec, local.utc_offset)
+    end
+
+    # Once per process: this caveat applies to every TZID parsed this way.
+    def self.warn_tzinfo_dst_limitation
+      return if @tzinfo_dst_warned
+
+      @tzinfo_dst_warned = true
+      warn "IceCube: resolving TZID with TZInfo. Occurrences will keep the start " \
+           "time's UTC offset across DST transitions; require \"active_support/time\" " \
+           "for full DST support."
+    end
+
     # Get a more precise equality for time objects
     # Ruby provides a Time#hash method, but it fails to account for UTC
     # offset (so the current date may be different) or DST rules (so the

spec/examples/from_ical_spec.rb

@@ -134,7 +134,7 @@ module IceCube
     ICAL
 
     ical_string_with_multiple_rules = <<-ICAL.gsub(/^\s*/, "")
-  DTSTART;TZID=CDT:20151005T195541
+  DTSTART;TZID=America/Chicago:20151005T195541
   RRULE:FREQ=WEEKLY;BYDAY=MO,TU
   RRULE:FREQ=WEEKLY;INTERVAL=2;WKST=SU;BYDAY=FR
     ICAL
@@ -461,5 +461,93 @@ def sorted_ical(ical)
         expect(schedule.to_ical.split("\n").find { |x| x =~ /RRULE/ }).to eq("RRULE:FREQ=WEEKLY;UNTIL=20130531T100000Z;BYDAY=TH")
       end
     end
+
+    describe "TZID parameter handling" do
+      it "should read TZID when it is not the first parameter" do
+        schedule = IceCube::Schedule.from_ical "DTSTART;VALUE=DATE-TIME;TZID=America/New_York:20130101T090000"
+        expect(schedule.start_time.utc_offset).to eq(-5 * 3600)
+        expect(schedule.start_time).to eq(Time.utc(2013, 1, 1, 14, 0, 0))
+      end
+
+      it "should read a TZID whose parameter name is lower case" do
+        schedule = IceCube::Schedule.from_ical "DTSTART;tzid=America/New_York:20130101T090000"
+        expect(schedule.start_time).to eq(Time.utc(2013, 1, 1, 14, 0, 0))
+      end
+
+      it "should read a double-quoted TZID" do
+        schedule = IceCube::Schedule.from_ical %(DTSTART;TZID="America/New_York":20130101T090000)
+        expect(schedule.start_time).to eq(Time.utc(2013, 1, 1, 14, 0, 0))
+      end
+
+      it "should not split the value on a colon inside a quoted parameter" do
+        schedule = IceCube::Schedule.from_ical %(DTSTART;TZID="Etc/GMT+5":20130101T090000)
+        expect(schedule.start_time).to eq(Time.utc(2013, 1, 1, 14, 0, 0))
+      end
+
+      it "should ignore a line with no value at all" do
+        expect {
+          IceCube::Schedule.from_ical "BEGIN:VEVENT\nDTSTART;TZID=America/New_York:20130101T090000\nEND"
+        }.not_to raise_error
+      end
+
+      it "should fall back to zone-less parsing and warn for an unknown TZID", expect_warnings: true do
+        schedule = nil
+        warnings = capture_warnings do
+          schedule = IceCube::Schedule.from_ical "DTSTART;TZID=Not/AZone:20130101T090000"
+        end
+        expect(warnings).to match(/unknown TZID "Not\/AZone"/)
+        expect(schedule.start_time).to eq(Time.parse("20130101T090000"))
+      end
+
+      it "should not raise when a time zone database is unavailable" do
+        # ice_cube has no runtime dependency on ActiveSupport or TZInfo, so a
+        # TZID must degrade rather than blow up when neither is loaded.
+        allow(TimeUtil).to receive(:find_zone).and_return(nil)
+        allow(TimeUtil).to receive(:zone_database_available?).and_return(false)
+
+        schedule = nil
+        warnings = capture_warnings do
+          schedule = IceCube::Schedule.from_ical "DTSTART;TZID=America/New_York:20130101T090000"
+        end
+        expect(warnings).to match(/without a time zone database/)
+        expect(schedule.start_time).to eq(Time.parse("20130101T090000"))
+      end
+    end
+
+    describe "ActiveSupport time zone labels" do
+      # Serialization emits IANA identifiers, but ActiveSupport names its zones
+      # with its own labels, and earlier versions wrote those into the TZID.
+      # Those strings are still out there, so they have to keep parsing.
+      it "should parse a TZID given as an ActiveSupport zone label" do
+        schedule = IceCube::Schedule.from_ical "DTSTART;TZID=Eastern Time (US & Canada):20100510T090000"
+        expect(schedule.start_time).to eq(Time.utc(2010, 5, 10, 13, 0, 0))
+        expect(schedule.start_time.utc_offset).to eq(-4 * 3600)
+      end
+
+      it "should follow DST for a TZID given as an ActiveSupport zone label" do
+        schedule = IceCube::Schedule.from_ical <<~ICAL
+          DTSTART;TZID=Eastern Time (US & Canada):20130308T090000
+          RRULE:FREQ=DAILY
+        ICAL
+        expect(schedule.first(4).map(&:utc_offset)).to eq([-5, -5, -4, -4].map { |h| h * 3600 })
+      end
+
+      it "should read a zone label and an IANA identifier as the same zone" do
+        labelled = IceCube::Schedule.from_ical <<~ICAL
+          DTSTART;TZID=Eastern Time (US & Canada):20130308T090000
+          RRULE:FREQ=DAILY
+        ICAL
+        iana = IceCube::Schedule.from_ical <<~ICAL
+          DTSTART;TZID=America/New_York:20130308T090000
+          RRULE:FREQ=DAILY
+        ICAL
+        expect(labelled.first(4)).to eq(iana.first(4))
+      end
+
+      it "should re-serialize a zone label as its IANA identifier" do
+        schedule = IceCube::Schedule.from_ical "DTSTART;TZID=Eastern Time (US & Canada):20100510T090000"
+        expect(schedule.to_ical).to eq("DTSTART;TZID=America/New_York:20100510T090000")
+      end
+    end
   end
 end