Repository: amzn/ion-java
Pull request #162 (2018-10-10 22:03:54)

Implements workaround for java.util.Date bug described in issue 160
See https://github.com/amzn/ion-java/issues/160.  Also there are ample comments in the diff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/.travis.yml b/.travis.yml
index 9bcf9994..0f696859 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,3 +1,10 @@
 language: java
 jdk:
+  - openjdk8
+  - openjdk9
+  - openjdk10
+  - openjdk11
   - oraclejdk8
+  - oraclejdk9
+  - oraclejdk10
+  - oraclejdk11
diff --git a/src/software/amazon/ion/Timestamp.java b/src/software/amazon/ion/Timestamp.java
index bd036ae9..64581548 100644
--- a/src/software/amazon/ion/Timestamp.java
+++ b/src/software/amazon/ion/Timestamp.java
@@ -23,6 +23,7 @@ import java.math.RoundingMode;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.GregorianCalendar;
+import java.util.TimeZone;
 import software.amazon.ion.impl.PrivateUtils;
 import software.amazon.ion.util.IonTextUtils;
 
@@ -276,14 +277,27 @@ public final class Timestamp
     @SuppressWarnings("deprecation")
     private void set_fields_from_millis(long millis)
     {
-        if(millis < MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS){
+        if(millis < MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS) {
             throw new IllegalArgumentException("year is less than 1");
         }
 
         Date date = new Date(millis);
 
-        // These fields are in the system timezone!
-        this._year    = checkAndCastYear(date.getYear() + 1900);
+        // https://github.com/amzn/ion-java/issues/160
+        // The java.util.Date(long) constructor expects an epoch time in milliseconds, and getYear(), getMonth(),
+        // getHour() on the resulting Date are supposed to return values adjusted to the default timezone.
+        // In Pacific Standard Time (offset -08:00), for a Date constructed with an epoch time equivalent to
+        // 0001-01-01T00:00:00.000Z, the Date.get*() methods should return values for
+        // 0000-12-31T16:00:00.000Z;  however, Date.getYear() incorrectly returns a value for year 1 (-1899)
+        // in this scenario. The following if/else block compensates for this bug:
+        int currentRawOffset = TimeZone.getDefault().getRawOffset();
+        if(currentRawOffset < 0 && MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS - currentRawOffset > millis) {
+            this._year = 0;
+        } else {
+            this._year = checkAndCastYear(date.getYear() + 1900);
+        }
+
+        // Note: date.get*() return values are in the local timezone!
         this._month   = checkAndCastMonth(date.getMonth() + 1);  // calendar months are 0 based, timestamp months are 1 based
         this._day     = checkAndCastDay(date.getDate(), _year, _month);
         this._hour    = checkAndCastHour(date.getHours());


Test patch:
diff --git a/test/software/amazon/ion/TimestampTest.java b/test/software/amazon/ion/TimestampTest.java
index ec55c093..c0017c75 100644
--- a/test/software/amazon/ion/TimestampTest.java
+++ b/test/software/amazon/ion/TimestampTest.java
@@ -860,6 +860,58 @@ public class TimestampTest
         assertEquals("2010-02-01T10:11:12.340Z", ts.toZString());
     }
 
+    /**
+     * Regression test for https://github.com/amzn/ion-java/issues/160
+     */
+    @Test
+    public void testNewTimestampFromYearOneRegressionBug()
+    {
+        // This is the same as the private field Timestamp.MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS
+        final long MINIMUM_TIMESTAMP_MILLIS = -62135769600000L;
+        // This is the minimum timestamp instantiating by parsing a timestamp string,
+        // which is not effected by the `java.util.Date` bug.
+        final Timestamp MINIMUM_TIMESTAMP = Timestamp.valueOf("0001-01-01T00:00:00.000Z");
+        // Save the original timezone since we modify it below.
+        final TimeZone originalTimeZone = TimeZone.getDefault();
+
+        try {
+            // The `java.util.Date` bug has a different range of times depending on the local offset, so
+            // let's test against all time zones.
+            for (final String tzId : TimeZone.getAvailableIDs()) {
+                final TimeZone tz = TimeZone.getTimeZone(tzId);
+                TimeZone.setDefault(tz);
+
+                // The Timestamps under test must be instantiated *after* the the call to TimeZone.setDefault()
+                // since `java.util.Date` references the default TimeZone when calculating the values for its
+                // date field accessor methods (i.e. Date.get*()).
+                Timestamp minimumTimestampFromMillis = Timestamp.forMillis(MINIMUM_TIMESTAMP_MILLIS, 0);
+                assertEquals(MINIMUM_TIMESTAMP, minimumTimestampFromMillis);
+
+                // Only perform further assertions on timezones with negative offsets because positive
+                // offsets create millisecond values that are less than Timestamp.MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS
+                if (tz.getRawOffset() < 0) {
+                    // This will be the latest millisecond in which the bug with `java.util.Date` can happen
+                    Timestamp maximumTimestampWithBug =
+                        Timestamp.forMillis(MINIMUM_TIMESTAMP_MILLIS - tz.getRawOffset(), 0);
+                    assertEquals(1, maximumTimestampWithBug.getYear());
+
+                    // Test one millisecond after, just to be sure.
+                    Timestamp timestampPlusOne =
+                        Timestamp.forMillis(MINIMUM_TIMESTAMP_MILLIS - tz.getRawOffset() + 1, 0);
+                    assertEquals(1, timestampPlusOne.getYear());
+
+                    // Also test one milliscond before, just to be sure.
+                    Timestamp timestampMinusOne =
+                        Timestamp.forMillis(MINIMUM_TIMESTAMP_MILLIS - tz.getRawOffset() - 1L, 0);
+                    assertEquals(1, timestampMinusOne.getYear());
+                }
+            }
+        } finally {
+            // Have to set the TimeZone back to its original value so as not to effect other tests.
+            TimeZone.setDefault(originalTimeZone);
+        }
+    }
+
     /**
      * Test for {@link Timestamp#createFromUtcFields(Precision, int, int, int, int, int, int, BigDecimal, Integer)}
      * ensuring that varying precisions produce Timestamps as expected as per