Menu

#81 TimeZone.getOffset(y,m,d,...) returns wrong results

Release (1.0)
open
nobody
None
7
2014-04-12
2014-03-08
RH
No

TimeZone.getOffset(final int era, final int year, final int month, final int day, final int dayOfWeek, final int milliseconds) returns wrong values because it mistakenly sets DAY_OF_YEAR instead of DAY_OF_MONTH to day and doesn't take into account that MONTH values begin with 0.

This leads to obscure bugs like https://github.com/rfc2822/davdroid/issues/186 – the DateParser of my OpenJDK 1.7.0 seems to call getOffset(long) when a timezone is passed, while Android's Java seems to call getOffset(era,y,m,d,dow,ms). This is why only Android is effected in the referenced issue.

The other thing is that this bug is dependent on the local clock time of the device running ical4j because DAY_OF_YEAR is set instead of DAY_OF_MONTH, so DAY_OF_MONTH is set to the current time, leading to test results depending on the local date.

1 Attachments

Discussion

  • RH

    RH - 2014-03-08

    Sample testing code:

        TimeZone tz = DefaultTimeZoneRegistryFactory.getInstance().createRegistry().getTimeZone("Europe/Vienna");
        final DateFormat format = new SimpleDateFormat("yyyyMMdd'T'HHmmss");
        format.setTimeZone(tz);
    
        Date date = format.parse("20140302T080000");
    
        java.util.Calendar c = java.util.Calendar.getInstance(TimeZones.getUtcTimeZone());
        c.setTime(date);
    
        Log.i(TAG, "Zone offset = " + c.get(java.util.Calendar.ZONE_OFFSET));
        Log.i(TAG, "DST offset = " + c.get(java.util.Calendar.DST_OFFSET));
        Log.i(TAG, "Hour = " + c.get(java.util.Calendar.HOUR_OF_DAY));
        Log.i(TAG, "Time = " + c.getTimeInMillis());
    

    You may also modify format.setTimeZone(tz); to pass the java.util.TimeZone (Europe/Vienna) instead of net.fortuna.ical4j.model.TimeZone(Europe/Vienna), and then it works because java.util.TimeZone doesn't have this issue.

     
    • Ben Fortuna

      Ben Fortuna - 2014-03-15

      Hi,

      I am just trying to create a test to demonstrate the problem. So far I have this:

      ~~~~~~~~
      setup: 'create date with ical4j timezone'
      final DateFormat format1 = new SimpleDateFormat("yyyyMMdd'T'HHmmss");
      TimeZone tz1 = tzRegistry.getTimeZone("Europe/Vienna");
      format1.setTimeZone(tz1);
      java.util.Date date1 = format1.parse("20140302T080000");
      java.util.Calendar c1 = java.util.Calendar.getInstance(TimeZones.getUtcTimeZone());
      c1.setTime(date1);

      and: 'create date with java timezeone'
      final DateFormat format2 = new SimpleDateFormat("yyyyMMdd'T'HHmmss");
      java.util.TimeZone tz2 = java.util.TimeZone.getTimeZone("Europe/Vienna");
      format2.setTimeZone(tz2);
      java.util.Date date2 = format2.parse("20140302T080000");
      java.util.Calendar c2 = java.util.Calendar.getInstance(TimeZones.getUtcTimeZone());
      c2.setTime(date2);

      expect:
      c1.get(java.util.Calendar.ZONE_OFFSET) == c2.get(java.util.Calendar.ZONE_OFFSET)
      c1.get(java.util.Calendar.DST_OFFSET) == c2.get(java.util.Calendar.DST_OFFSET)
      c1.get(java.util.Calendar.HOUR_OF_DAY) == c2.get(java.util.Calendar.HOUR_OF_DAY)
      c1.getTimeInMillis() == c2.getTimeInMillis() ~~~~~~~~~~

      However this test is passing ok. Shouldn't I be seeing different values from the two different calendar instances?

       
      • RH

        RH - 2014-03-15

        I don't know. Wouldn't it be better to test the getOffset(y,m,d,...) method explicitly? Another problem with testing is that the result depends on the local system date, i.e. the test will succeed on one day and fail on another (because DAY_OF_MONTH isn't overwritten, hence set to the current day).

        You would have to call getOffset() for 20140302T080000 (Europe/Vienna) with a local device date of 2014-02-28 to see the error – it should return 1 hour offset, but returns 2.

         

        Last edit: RH 2014-03-15
      • Ben Fortuna

        Ben Fortuna - 2014-03-17

        Yes I think it is better to test getOffset directly. The difficulty is in setting the local system time for testing.. This may be a good reason to investigate using JodaTime in ical4j.

         
  • Chandra

    Chandra - 2014-03-08

    Our team in the company i work also ran into issue in caldav syncing. We traced through the code and found the same root cause for the issue. But we find that the month-1 change is not necessary. the right month value is being sent to this method.

    format.setTimeZone(tz); (with tz that is net.fortuna.ical4j.model.TimeZone pointing to 'America/Los_Angeles')
    Date date = format.parse("20140313T100000");

    calls the set time zone with the following parameters:

    era = 1 (AD)
    year = 2014
    month = 2
    day = 13
    dayOfWeek = 5
    milliseconds= 36000000

    all these parameters are correct. The only issue was set the day to dayOfTheYear (since in the above case 13th day of the year is invalid for month 2 it is ignored).

    Since the Day of the month is still the present day of the month, it will pick the mentioned of the day week in the present week.

    i.e
    if the code
    final Calendar cal = Calendar.getInstance();
    cal.set(Calendar.ERA, 1);
    cal.set(Calendar.YEAR, 2014);
    cal.set(Calendar.MONTH, 2);
    cal.set(Calendar.DAY_OF_MONTH, 13);
    cal.set(Calendar.DAY_OF_WEEK, 5);
    cal.set(Calendar.MILLISECOND, 36000000);
    Which TimeZone.getOffset runs,

    is run on march 8th. it will give a Mar 6th
    is run on march 9th. it will give a Mar 13th
    is run on march 16th. it will give a Mar 20th

     
  • RH

    RH - 2014-04-12

    Any news on this? Can I provide further information to help fixing this? As soon as it is fixed, I'd include the master version in DAvdroid, because in the meanwhile this bug has occured for a bunch of people.

     

Log in to post a comment.