[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: Wrong behavior of LocalDateTimeStringConverter
From:       Nir Lisker <nlisker () gmail ! com>
Date:       2021-01-27 1:18:43
Message-ID: CA+0ynh_PP9bkfdmTrxQsmkwYLK_ATcdCuvj+xZqvS7gr2i-pmw () mail ! gmail ! com
[Download RAW message or body]

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

On Tue, Jan 26, 2021 at 11:57 PM Kevin Rushforth <kevin.rushforth@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
>
>
[prev in list] [next in list] [prev in thread] [next in thread]