[External] : Re: Wrong behavior of LocalDateTimeStringConverter

[Kevin Rushforth]

Yes, that also seems like a bug.

-- Kevin


On 1/26/2021 5:18 PM, Nir Lisker wrote:
> I filed JDK-8260468[1].
>
> Looking at it more closely, it seems that the implementations of 
> toString and fromString are incorrect in another place. If the 
> converter is created with a given formatter and/or parser, then they 
> can have their own chronologies, but these methods always use the 
> default one in this case (since a chronology wasn't given in 
> construction time).
> I think that the default chronology should only be used if one was not 
> set for the parser or formatter.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8260468 
> <https://bugs.openjdk.java.net/browse/JDK-8260468>
>
> On Tue, Jan 26, 2021 at 11:57 PM Kevin Rushforth 
> <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>> wrote:
>
>     Yes, this does seem like a bug. It should be fixed in a separate bug
>     (prior to whatever refactoring you might propose) with an
>     associated CSR.
>
>     -- Kevin
>
>
>     On 1/26/2021 10:56 AM, Nir Lisker wrote:
>     > Hi,
>     >
>     > I was looking at LocalDateTimeStringConverter and I think that
>     there is a
>     > possibility for wrong behavior.
>     > The class uses an internal LdtConverter helper class that
>     instantiates all
>     > its fields, but they are not final. In its toString method, the
>     chronology
>     > field is reassigned if the formatting fails on that specific
>     value. Then,
>     > if another value is passed in, the new chronology field value is
>     used,
>     > which could give different results from having it been passed in
>     first. In
>     > addition, the default formatter and parser are built using the
>     chronology
>     > which was used at the time of their creation, and can become
>     > inconsistent if the chronology changes later.
>     >
>     > I think that this class should be immutable, with all null
>     checks being
>     > done on instantiation and assigning defaults based on that. I
>     propose to
>     > change this behavior as part of a refactoring effort I'm making
>     in the
>     > converters package.
>     >
>     > Note that this internal class is used by LocalDate and LocalTime
>     converters
>     > as well.
>     >
>     > - Nir
>