Skip to content

Commit 123ac07

Browse files
naotojnaotoj
committedMar 9, 2020
8239836: ZoneRules.of() doesn't check transitionList/standardOffsetTL arguments validity
Reviewed-by: rriggs, joehw, scolebourne
·
jdk-25+17jdk-15-ga
1 parent 5c8f935 commit 123ac07

File tree

2 files changed

+103
-40
lines changed

2 files changed

+103
-40
lines changed
 

‎src/java.base/share/classes/java/time/zone/ZoneRules.java

+10-7
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,10 @@ static ZoneRules readExternal(DataInput in) throws IOException, ClassNotFoundExc
470470
* @return true if the time-zone is fixed and the offset never changes
471471
*/
472472
public boolean isFixedOffset() {
473-
return savingsInstantTransitions.length == 0;
473+
return standardOffsets[0].equals(wallOffsets[0]) &&
474+
standardTransitions.length == 0 &&
475+
savingsInstantTransitions.length == 0 &&
476+
lastRules.length == 0;
474477
}
475478

476479
/**
@@ -486,7 +489,7 @@ public boolean isFixedOffset() {
486489
*/
487490
public ZoneOffset getOffset(Instant instant) {
488491
if (savingsInstantTransitions.length == 0) {
489-
return standardOffsets[0];
492+
return wallOffsets[0];
490493
}
491494
long epochSec = instant.getEpochSecond();
492495
// check if using last rules
@@ -572,7 +575,7 @@ public ZoneOffset getOffset(LocalDateTime localDateTime) {
572575
* There are various ways to handle the conversion from a {@code LocalDateTime}.
573576
* One technique, using this method, would be:
574577
* <pre>
575-
* List&lt;ZoneOffset&gt; validOffsets = rules.getOffset(localDT);
578+
* List&lt;ZoneOffset&gt; validOffsets = rules.getValidOffsets(localDT);
576579
* if (validOffsets.size() == 1) {
577580
* // Normal case: only one valid offset
578581
* zoneOffset = validOffsets.get(0);
@@ -640,8 +643,8 @@ public ZoneOffsetTransition getTransition(LocalDateTime localDateTime) {
640643
}
641644

642645
private Object getOffsetInfo(LocalDateTime dt) {
643-
if (savingsInstantTransitions.length == 0) {
644-
return standardOffsets[0];
646+
if (savingsLocalTransitions.length == 0) {
647+
return wallOffsets[0];
645648
}
646649
// check if using last rules
647650
if (lastRules.length > 0 &&
@@ -756,7 +759,7 @@ private ZoneOffsetTransition[] findTransitionArray(int year) {
756759
* @return the standard offset, not null
757760
*/
758761
public ZoneOffset getStandardOffset(Instant instant) {
759-
if (savingsInstantTransitions.length == 0) {
762+
if (standardTransitions.length == 0) {
760763
return standardOffsets[0];
761764
}
762765
long epochSec = instant.getEpochSecond();
@@ -786,7 +789,7 @@ public ZoneOffset getStandardOffset(Instant instant) {
786789
* @return the difference between the standard and actual offset, not null
787790
*/
788791
public Duration getDaylightSavings(Instant instant) {
789-
if (savingsInstantTransitions.length == 0) {
792+
if (isFixedOffset()) {
790793
return Duration.ZERO;
791794
}
792795
ZoneOffset standardOffset = getStandardOffset(instant);

‎test/jdk/java/time/test/java/time/zone/TestZoneRules.java

+93-33
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
package test.java.time.zone;
2525

2626
import java.time.DayOfWeek;
27+
import java.time.Duration;
2728
import java.time.Instant;
2829
import java.time.LocalDate;
2930
import java.time.LocalDateTime;
@@ -37,16 +38,17 @@
3738
import java.time.zone.ZoneOffsetTransitionRule;
3839
import java.time.zone.ZoneRules;
3940
import java.util.Collections;
41+
import java.util.List;
4042

41-
import org.testng.annotations.Test;
4243
import org.testng.annotations.DataProvider;
44+
import org.testng.annotations.Test;
4345
import static org.testng.Assert.assertEquals;
4446
import static org.testng.Assert.assertTrue;
4547

4648
/**
4749
* @summary Tests for ZoneRules class.
4850
*
49-
* @bug 8212970 8236903
51+
* @bug 8212970 8236903 8239836
5052
*/
5153
@Test
5254
public class TestZoneRules {
@@ -59,37 +61,44 @@ public class TestZoneRules {
5961
private static final ZoneId TOKYO = ZoneId.of("Asia/Tokyo");
6062
private static final LocalTime ONE_AM = LocalTime.of(1, 0);
6163

64+
private static final ZoneOffset OFF_0 = ZoneOffset.ofHours(0);
65+
private static final ZoneOffset OFF_1 = ZoneOffset.ofHours(1);
66+
private static final ZoneOffset OFF_2 = ZoneOffset.ofHours(2);
67+
private static final List EL = Collections.emptyList();
68+
private static final ZoneOffsetTransition ZOT = ZoneId.of("America/Los_Angeles").getRules().getTransitions().get(0);
69+
private static final ZoneOffsetTransitionRule ZOTR = ZoneId.of("America/Los_Angeles").getRules().getTransitionRules().get(0);
70+
6271
@DataProvider
6372
private Object[][] negativeDST () {
6473
return new Object[][] {
6574
// ZoneId, localDate, offset, standard offset, isDaylightSavings
6675
// Europe/Dublin for the Rule "Eire"
67-
{DUBLIN, LocalDate.of(1970, 6, 23), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
68-
{DUBLIN, LocalDate.of(1971, 6, 23), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
69-
{DUBLIN, LocalDate.of(1971, 11, 1), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
70-
{DUBLIN, LocalDate.of(2019, 6, 23), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
71-
{DUBLIN, LocalDate.of(2019, 12, 23), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
76+
{DUBLIN, LocalDate.of(1970, 6, 23), OFF_1, OFF_0, true},
77+
{DUBLIN, LocalDate.of(1971, 6, 23), OFF_1, OFF_0, true},
78+
{DUBLIN, LocalDate.of(1971, 11, 1), OFF_0, OFF_0, false},
79+
{DUBLIN, LocalDate.of(2019, 6, 23), OFF_1, OFF_0, true},
80+
{DUBLIN, LocalDate.of(2019, 12, 23), OFF_0, OFF_0, false},
7281

7382
// Europe/Prague which contains fixed negative savings (not a named Rule)
74-
{PRAGUE, LocalDate.of(1946, 9, 30), ZoneOffset.ofHours(2), ZoneOffset.ofHours(1), true},
75-
{PRAGUE, LocalDate.of(1946, 10, 10), ZoneOffset.ofHours(1), ZoneOffset.ofHours(1), false},
76-
{PRAGUE, LocalDate.of(1946, 12, 3), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
77-
{PRAGUE, LocalDate.of(1947, 2, 25), ZoneOffset.ofHours(1), ZoneOffset.ofHours(1), false},
78-
{PRAGUE, LocalDate.of(1947, 4, 30), ZoneOffset.ofHours(2), ZoneOffset.ofHours(1), true},
83+
{PRAGUE, LocalDate.of(1946, 9, 30), OFF_2, OFF_1, true},
84+
{PRAGUE, LocalDate.of(1946, 10, 10), OFF_1, OFF_1, false},
85+
{PRAGUE, LocalDate.of(1946, 12, 3), OFF_0, OFF_0, false},
86+
{PRAGUE, LocalDate.of(1947, 2, 25), OFF_1, OFF_1, false},
87+
{PRAGUE, LocalDate.of(1947, 4, 30), OFF_2, OFF_1, true},
7988

8089
// Africa/Windhoek for the Rule "Namibia"
81-
{WINDHOEK, LocalDate.of(1994, 3, 23), ZoneOffset.ofHours(1), ZoneOffset.ofHours(1), false},
82-
{WINDHOEK, LocalDate.of(2016, 9, 23), ZoneOffset.ofHours(2), ZoneOffset.ofHours(1), true},
90+
{WINDHOEK, LocalDate.of(1994, 3, 23), OFF_1, OFF_1, false},
91+
{WINDHOEK, LocalDate.of(2016, 9, 23), OFF_2, OFF_1, true},
8392

8493
// Africa/Casablanca for the Rule "Morocco" Defines negative DST till 2037 as of 2019a.
85-
{CASABLANCA, LocalDate.of(1939, 9, 13), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
86-
{CASABLANCA, LocalDate.of(1939, 11, 20), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
87-
{CASABLANCA, LocalDate.of(2018, 6, 18), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
88-
{CASABLANCA, LocalDate.of(2019, 1, 1), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
89-
{CASABLANCA, LocalDate.of(2019, 5, 6), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
90-
{CASABLANCA, LocalDate.of(2037, 10, 5), ZoneOffset.ofHours(0), ZoneOffset.ofHours(0), false},
91-
{CASABLANCA, LocalDate.of(2037, 11, 16), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
92-
{CASABLANCA, LocalDate.of(2038, 11, 1), ZoneOffset.ofHours(1), ZoneOffset.ofHours(0), true},
94+
{CASABLANCA, LocalDate.of(1939, 9, 13), OFF_1, OFF_0, true},
95+
{CASABLANCA, LocalDate.of(1939, 11, 20), OFF_0, OFF_0, false},
96+
{CASABLANCA, LocalDate.of(2018, 6, 18), OFF_1, OFF_0, true},
97+
{CASABLANCA, LocalDate.of(2019, 1, 1), OFF_1, OFF_0, true},
98+
{CASABLANCA, LocalDate.of(2019, 5, 6), OFF_0, OFF_0, false},
99+
{CASABLANCA, LocalDate.of(2037, 10, 5), OFF_0, OFF_0, false},
100+
{CASABLANCA, LocalDate.of(2037, 11, 16), OFF_1, OFF_0, true},
101+
{CASABLANCA, LocalDate.of(2038, 11, 1), OFF_1, OFF_0, true},
93102
};
94103
}
95104

@@ -108,6 +117,28 @@ private Object[][] transitionBeyondDay() {
108117
};
109118
}
110119

120+
@DataProvider
121+
private Object[][] emptyTransitionList() {
122+
return new Object[][] {
123+
// days, offset, std offset, savings, isDST
124+
{7, 1, 2, -1, true},
125+
{-7, 1, 1, 0, false},
126+
};
127+
}
128+
129+
@DataProvider
130+
private Object[][] isFixedOffset() {
131+
return new Object[][] {
132+
// ZoneRules, expected
133+
{ZoneRules.of(OFF_0), true},
134+
{ZoneRules.of(OFF_0, OFF_0, EL, EL, EL), true},
135+
{ZoneRules.of(OFF_0, OFF_1, EL, EL, EL), false},
136+
{ZoneRules.of(OFF_0, OFF_0, Collections.singletonList(ZOT), EL, EL), false},
137+
{ZoneRules.of(OFF_0, OFF_0, EL, Collections.singletonList(ZOT), EL), false},
138+
{ZoneRules.of(OFF_0, OFF_0, EL, EL, Collections.singletonList(ZOTR)), false},
139+
};
140+
}
141+
111142
/**
112143
* Test ZoneRules whether the savings are positive in time zones that have
113144
* negative savings in the source TZ files.
@@ -147,28 +178,57 @@ public void test_TransitionLastRuleYear() {
147178
59,
148179
59,
149180
999999999).toInstant(ZoneOffset.UTC);
150-
ZoneOffset offsetZero = ZoneOffset.ofHours(0);
151-
ZoneOffset offsetPlusOneHour = ZoneOffset.ofHours(1);
152-
ZoneRules zoneRulesA = ZoneRules.of(offsetPlusOneHour);
153-
ZoneOffsetTransition transition = ZoneOffsetTransition.of(LocalDateTime.ofEpochSecond(0, 0, offsetZero),
154-
offsetZero,
155-
offsetPlusOneHour);
181+
ZoneRules zoneRulesA = ZoneRules.of(OFF_1);
182+
ZoneOffsetTransition transition = ZoneOffsetTransition.of(LocalDateTime.ofEpochSecond(0, 0, OFF_0),
183+
OFF_0,
184+
OFF_1);
156185
ZoneOffsetTransitionRule transitionRule = ZoneOffsetTransitionRule.of(Month.JANUARY,
157186
1,
158187
DayOfWeek.SUNDAY,
159188
LocalTime.MIDNIGHT,
160189
true,
161190
ZoneOffsetTransitionRule.TimeDefinition.STANDARD,
162-
offsetZero,
163-
offsetZero,
164-
offsetPlusOneHour);
165-
ZoneRules zoneRulesB = ZoneRules.of(offsetZero,
166-
offsetZero,
191+
OFF_0,
192+
OFF_0,
193+
OFF_1);
194+
ZoneRules zoneRulesB = ZoneRules.of(OFF_0,
195+
OFF_0,
167196
Collections.singletonList(transition),
168197
Collections.singletonList(transition),
169198
Collections.singletonList(transitionRule));
170199
ZoneOffset offsetA = zoneRulesA.getOffset(maxLocalDateTime);
171200
ZoneOffset offsetB = zoneRulesB.getOffset(maxLocalDateTime);
172201
assertEquals(offsetA, offsetB);
173202
}
203+
204+
/**
205+
* Tests whether empty "transitionList" is correctly interpreted.
206+
* @bug 8239836
207+
*/
208+
@Test(dataProvider="emptyTransitionList")
209+
public void test_EmptyTransitionList(int days, int offset, int stdOffset, int savings, boolean isDST) {
210+
LocalDateTime transitionDay = LocalDateTime.of(2020, 1, 1, 2, 0);
211+
Instant testDay = transitionDay.plusDays(days).toInstant(ZoneOffset.UTC);
212+
ZoneOffsetTransition trans = ZoneOffsetTransition.of(
213+
transitionDay,
214+
OFF_1,
215+
OFF_2);
216+
ZoneRules rules = ZoneRules.of(OFF_1, OFF_1,
217+
Collections.singletonList(trans),
218+
Collections.emptyList(), Collections.emptyList());
219+
220+
assertEquals(rules.getOffset(testDay), ZoneOffset.ofHours(offset));
221+
assertEquals(rules.getStandardOffset(testDay), ZoneOffset.ofHours(stdOffset));
222+
assertEquals(rules.getDaylightSavings(testDay), Duration.ofHours(savings));
223+
assertEquals(rules.isDaylightSavings(testDay), isDST);
224+
}
225+
226+
/**
227+
* Tests whether isFixedOffset() is working correctly
228+
* @bug 8239836
229+
*/
230+
@Test(dataProvider="isFixedOffset")
231+
public void test_IsFixedOffset(ZoneRules zr, boolean expected) {
232+
assertEquals(zr.isFixedOffset(), expected);
233+
}
174234
}

0 commit comments

Comments
 (0)
Please sign in to comment.