RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

Stephen Colebourne scolebourne at joda.org
Wed Sep 19 21:20:28 UTC 2018


Thanks for looking at this.

The proposed fix does not tackle the bug fully. The bug is that the spec says

"The format consists of: The ISO_OFFSET_DATE_TIME where ..."

As such, the format must parse *any* offset, not just "Z" / "+00:00" etc.

In addition, the fix doesn't work properly. Parsers work off a
CharSequence which may be much longer than the instant. For example,
the instant might be followed by a literal space and then a ZoneId.
Using (length - 3) is simply not a valid approach - the parsing code
cannot use the length like that.

Furthermore, although there are numerous valid ISO-8601 ways of saying
zero, this format uses dashes and colons in the date/time part, so
ISO-8601 restricts the offset to only those formats that include
colons.

I think it simply needs the appendLiteral(Z) changing to appendOffsetId()
And line 3495 changes to use the offset from the newContext.

thanks
Stephen


On Wed, 19 Sep 2018 at 18:16, Pallavi Sonal <pallavi.sonal at oracle.com> wrote:
>
> Hi,
>
> Please review the changes to the following issue:
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8166138
>
>
>
> The proposed fix is located at:
>
> Webrev : http://cr.openjdk.java.net/~rpatil/8166138/webrev.00/
>
>
>
> As per ISO 8601 standards, an offset of zero, in addition to having the special representation "Z", can also be stated numerically as "+00:00", "+0000", or "+00" [1].  With this fix, Instant.parse() can parse a String containing the zero offsets in any of these three forms. Any other offset apart from "Z", "+00:00", "+0000", or "+00" will not be accepted in the input string to be parsed.
>
>
>
> [1] https://en.wikipedia.org/wiki/ISO_8601
>
>
>
> Thanks,
>
> Pallavi Sonal
>
>
>
>


More information about the core-libs-dev mailing list