#53 Iso8601.toString patch

open
nobody
None
5
2009-07-06
2009-07-06
Antoni Mylka
No

I was investigating the issue number 2476115 from Aperture:

https://sourceforge.net/tracker/?func=detail&aid=2476115&group_id=150969&atid=779500

It boiled down to the Iso8601.toString method. It yields wrong results for floating times before 1985 when the current timezone is set to Asia/Kathmandu. on 01.01.1986 Kathmandu nudged the timezone offset from +0530 to +0545. And for some bizarre reason all floating dates before that time when converted from a string to DateTime and back to string return moved forward by 15 minutes.

To reproduce: set the timezone on your machine to Asia/Kathmandu and try to build the 1.0.rc2 release.

I tried to tweak the toString method, attached in the patch. I must admit that I don't fully comprehend why all those ifs are as they are, but my modification doesn't break any tests, makes the ical4j release build correctly in Kathmandu and fixes my problem 2476115.

Please examine it and apply that patch if it doesn't break anything else. If you'd like to do something different, please make sure that the build works correctly in Kathmandu.

(The patch also comments out a line in TimeZoneTest that failed outside australia, see my bug report for details).

Discussion

  • Antoni Mylka
    Antoni Mylka
    2009-07-06

    the patch, generated with Eclipse, in the format for use with the "Apply Patch" wizard

     
    Attachments
  • Ben Fortuna
    Ben Fortuna
    2009-07-07

    Hi Antoni,

    Thanks again for providing a very thorough description of the problem. I admit the code in Iso8601.toString() is quite obscure (and very ugly), but there is a reason behind it..

    I tried applying your patch and I get one test failure in DateTimeTest.testToString():

    junit.framework.ComparisonFailure: Incorrect string representation expected:<20000827T0[2]0000> but was:<20000827T0[3]0000>

    The test creates a new DateTime from a string and compares the result of toString() with the original:

    suite.addTest(new DateTimeTest(new DateTime("20000827T020000"), "20000827T020000"));

    The date specified in this test is the exact time when daylight savings switched to standard in Australia for that year, so effectively this date doesn't exist (i.e. at 2am it became 3am). Nonetheless I expect the date to be unchanged from the input for iCal4j, which is why I had to put in that ugly workaround.

    I think the reason that Kathmandu is giving incorrect results is probably due to a bug in the JVM's TimeZone.getRawOffset() / getDSTSavings(), or the JVM's timezone definition not being correct (*).

    Your solution is fine if we can find a way to get around the problem mentioned earlier.. I will try some experimenting to see if there's a solution that satisfies all the issues.

    regards,
    ben

    * This is actually a bit of an inconsistency with iCal4j, in that it relies on the system timezone for floating time, but uses internal timezone definitions for date-times with an associated timezone.

     
  • Ben Fortuna
    Ben Fortuna
    2009-07-07

    Oops correction: 6th paragrah I meant switch from standard to daylight, but u get the picture.