Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

offsetdatetime deserialization does not handle nano seconds correctly #32

Closed
xzer opened this issue Jun 13, 2017 · 1 comment
Closed

Comments

@xzer
Copy link

xzer commented Jun 13, 2017

I reported it at the old FasterXML/jackson-datatype-jsr310, and then I found this repo is the current in maintain workspace, then I also checked the corresponding source, which suggests that the reported issue still exists. It is possible that I missed something in this new repo, but just report it again to make sure it is OK.

the origin issue report:

https://github.com/FasterXML/jackson-datatype-jsr310/issues/96

when InstantDeserializer get a long value for OffsetDateTime, it will entering the following method:

    protected T _fromLong(DeserializationContext context, long timestamp)
    {
        if(context.isEnabled(DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS)){
            return fromNanoseconds.apply(new FromDecimalArguments(
                    timestamp, 0, this.getZone(context)
            ));
        }
        return fromMilliseconds.apply(new FromIntegerArguments(
                timestamp, this.getZone(context)));
    }

Notice that, with the default configuration, READ_DATE_TIMESTAMPS_AS_NANOSECONDS is true, it will create a FromDecimalArguments with the passed timestamp value, and which will be passed to the following source:

    public static final InstantDeserializer<OffsetDateTime> OFFSET_DATE_TIME = new InstantDeserializer<>(
            OffsetDateTime.class, DateTimeFormatter.ISO_OFFSET_DATE_TIME,
            OffsetDateTime::from,
            a -> OffsetDateTime.ofInstant(Instant.ofEpochMilli(a.value), a.zoneId),
            a -> OffsetDateTime.ofInstant(Instant.ofEpochSecond(a.integer, a.fraction), a.zoneId),
            (d, z) -> d.withOffsetSameInstant(z.getRules().getOffset(d.toLocalDateTime())),
            true // yes, replace +0000 with Z
    );

to be precision, the following row:

a -> OffsetDateTime.ofInstant(Instant.ofEpochSecond(a.integer, a.fraction), a.zoneId),

which will handle the value as epoch seconds, rather than nano second.

Is there anything that I misunderstand? Or it is actually a bug?

@cowtowncoder
Copy link
Member

I think recent changes may have fixed that; if not (fails against 2.10.0.pr1), may be reopened/refiled with test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants