ICU-22616 fix value returned by Calendar::getTimeInMillis() after cal…

…l to Calendar::set() during ambiguous time
unicode-org · Jan 3, 2024 · e3bfca8 · e3bfca8
1 parent 1a60a03
commit e3bfca8
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 0 deletions.
diff --git a/icu4c/source/i18n/calendar.cpp b/icu4c/source/i18n/calendar.cpp
@@ -1216,6 +1216,15 @@ Calendar::set(UCalendarDateFields field, int32_t value)
     if (fAreFieldsVirtuallySet) {
         UErrorCode ec = U_ZERO_ERROR;
         computeFields(ec);
+
+        // handle the ambiguous time that occurs twice at the end of DST:
+        // if we're in DST, then we want the first occurrence of this time,
+        // otherwise we want the second occurrence of this time
+        if (fFields[UCAL_DST_OFFSET] != 0) {
+            setRepeatedWallTimeOption(UCAL_WALLTIME_FIRST);
+        } else {
+            setRepeatedWallTimeOption(UCAL_WALLTIME_LAST);
+        }
     }
     fFields[field]     = value;
     /* Ensure that the fNextStamp value doesn't go pass max value for int32_t */

diff --git a/icu4c/source/test/cintltst/cdattst.c b/icu4c/source/test/cintltst/cdattst.c
@@ -49,6 +49,7 @@ static void TestForceGannenNumbering(void);
 static void TestMapDateToCalFields(void);
 static void TestNarrowQuarters(void);
 static void TestExtraneousCharacters(void);
+static void TestTimeAtEndOfDST(void);
 static void TestParseTooStrict(void);
 static void TestHourCycle(void);
 static void TestLocaleNameCrash(void);
@@ -75,6 +76,7 @@ void addDateForTest(TestNode** root)
     TESTCASE(TestMapDateToCalFields);
     TESTCASE(TestNarrowQuarters);
     TESTCASE(TestExtraneousCharacters);
+    TESTCASE(TestTimeAtEndOfDST);
     TESTCASE(TestParseTooStrict);
     TESTCASE(TestHourCycle);
     TESTCASE(TestLocaleNameCrash);
@@ -2145,3 +2147,35 @@ static void TestLocaleNameCrash(void) {
 }
 
 #endif /* #if !UCONFIG_NO_FORMATTING */
+
+
+static void TestTimeAtEndOfDST(void) {
+    // regression test for ICU-22616
+    UErrorCode status;
+    UCalendar *cal;
+    double set_millis, get_millis;
+
+    status = U_ZERO_ERROR;
+
+    // America/Los_Angeles -- end of DST
+    // start time:
+    //    UTC:   Sun Nov 05 2023 07:00:00
+    //    local: Sun Nov 05 2023 00:00:00
+    // step through three hours, 15 minutes at a time
+    // checking for mismatches between set millis and get millis
+    // during the hour that happens twice at the end of DST
+    cal = ucal_open(u"America/Los_Angeles", -1, "en_US", UCAL_DEFAULT, &status);
+    for (set_millis = 1699167600000; set_millis < 1699167600000 + (1000*60*60*3); set_millis += (1000*60*15)) {
+        ucal_clear(cal);
+        ucal_setMillis(cal, set_millis, &status);
+
+        // Note that the call to ucal_set() sets fIsTimeSet to false so then
+        // Calendar::getTimeInMillis() calls Calendar::updateTime()
+        // instead of just returning fTime.
+        ucal_set(cal, UCAL_ERA, 1);
+
+        get_millis = ucal_getMillis(cal, &status);
+        assertTrue("ucal_getMillis() returns value set by ucal_setMillis()", get_millis == set_millis);
+    }
+    ucal_close(cal);
+}
diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/util/Calendar.java b/icu4j/main/core/src/main/java/com/ibm/icu/util/Calendar.java
@@ -2228,6 +2228,15 @@ public final void set(int field, int value)
     {
         if (areFieldsVirtuallySet) {
             computeFields();
+
+            // handle the ambiguous time that occurs twice at the end of DST:
+            // if we're in DST, then we want the first occurrence of this time,
+            // otherwise we want the second occurrence of this time
+            if (fields[DST_OFFSET] != 0) {
+                setRepeatedWallTimeOption(WALLTIME_FIRST);
+            } else {
+                setRepeatedWallTimeOption(WALLTIME_LAST);
+            }
         }
         fields[field] = value;
         /* Ensure that the fNextStamp value doesn't go pass max value for 32 bit integer */

diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/timezone/TimeZoneRuleTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/timezone/TimeZoneRuleTest.java
@@ -1849,4 +1849,35 @@ public void TestT8943() {
             errln("Fail: Exception thrown - " + e.getMessage());
         }
     }
+
+    @Test
+    public void TestTimeAtEndOfDST() {
+        // regression test for ICU-22616
+
+        // America/Los_Angeles -- end of DST
+        // start time:
+        //    UTC:   Sun Nov 05 2023 07:00:00
+        //    local: Sun Nov 05 2023 00:00:00
+        // step through three hours, 15 minutes at a time
+        // checking for mismatches between set millis and get millis
+        // during the hour that happens twice at the end of DST
+        final long start_millis = 1699167600000L;
+        long get_millis;
+        TimeZone losangeles = TimeZone.getTimeZone("America/Los_Angeles", TimeZone.TIMEZONE_ICU);
+        Calendar cal = new GregorianCalendar(losangeles);
+        for (long set_millis = start_millis; set_millis < start_millis + (1000*60*60*3); set_millis += (1000*60*15)) {
+            cal.clear();
+            cal.setTimeInMillis(set_millis);
+
+            // Note that the call to cal.set() sets isTimeSet to false so then
+            // Calendar::getTimeInMillis() calls Calendar::updateTime()
+            // instead of just returning time.
+            cal.set(Calendar.ERA, GregorianCalendar.AD);
+
+            get_millis = cal.getTimeInMillis();
+            if ( get_millis != set_millis) {
+                errln("FAIL: value returned by getTimeInMillis() does not match value set by setTimeInMillis()");
+            }
+        }
+    }
 }