[15] RFR: 8239836: ZoneRules.of() doesn't check transitionList/standardOffsetTL arguments validity

naoto.sato at oracle.com naoto.sato at oracle.com
Mon Mar 2 18:35:22 UTC 2020


Hi Roger, thanks for the review.

On 3/2/20 8:44 AM, Roger Riggs wrote:
> Hi Naoto,
> 
> look ok.
> 
> ZoneRules.java: 488, 644, 761, 791
> I'd suggest calling isFixedOffset() instead of repeating the code:
> standardTransitions.length == 0 && savingsInstantTransitions.length == 0

Modified as suggested:

http://cr.openjdk.java.net/~naoto/8239836/webrev.01/

> 
> It should be inlined in cases where the performance matters.

None of those locations is invoked during ZoneRules object 
instantiation. So I believe it is OK to replace them with isFixedOffset().

Naoto

> 
> Thanks, Roger
> 
> 
> On 2/27/20 3:41 PM, naoto.sato at oracle.com wrote:
>> Hello,
>>
>> Please review the fix to the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8239836
>>
>> The proposed changeset is located at:
>>
>> https://cr.openjdk.java.net/~naoto/8239836/webrev.00/
>>
>> It turned out that 'transitionList' is not necessarily a superset of
>> 'standardOffsetTransitionList', as in some occasions where a standard 
>> offset change and a savings change happen at the same time (canceling 
>> each other), resulting in no wall time transition. This means that the 
>> "rules" in the sample code is a valid ZoneRules instance.
>>
>> However, there is an assumption in ZoneRules where no (wall time) 
>> transition means the fixed offset zone. With that assumption, the 
>> example rule is considered a fixed offset zone which is not correct. 
>> So the fix proposed here is not to throw an IAE but to recognize the 
>> rule as a valid, non-fixed offset ZoneRules instance.
>>
>> Naoto
>>
>>
> 


More information about the core-libs-dev mailing list