Should not parse LocalDates from number (timestamp) -- or at least should have an option preventing

[billoneil]

In Jackson 2.8.x I had a unit test that parsed the following and expected an exception.

{
  "date": 123,
  "message": "Happy New Year!"
}

As of 2.9 related to this commit https://github.com/FasterXML/jackson-modules-java8/pull/22/files#diff-a1552641fab3c9690f2c8b0f5ee2fe89R102 it now parses as 1970-05-04

Is there a way to turn this off if I expect my dates to come in as a different format? I don't want to parse random integers as dates when it should be invalid input.

[cowtowncoder]

No mechanism to prevent that currently; integers are accepted by default for all date/time types (so change for 2.9, if it didn't work on 2.8) is more due to unifying handling than otherwise intended change.

But come to think of it now, it is not actually clear that numbers should work for LocalDate, given that it is pure date value with no canonical numeric value without timezone.
So perhaps it is something that should be blocked in 3.0. Since 2.9.x are already out, behavior can not be changed in a patch however.

As to blocking value you will need to register a custom deserializer that does this.

[billoneil]

Just thought I would mention the change in behavior to see if it was intended or not. I can work around it on my end if needed. Feel free to close if you don't plan on changing the behavior.

[cowtowncoder]

@billoneil Looks like this was due to #20 actually, so it was intentional. Although perhaps bit misguided, considering the part.

However! I do have one possibly useful idea wrt 2.9.x, your specific use case: there is property lenient for @JsonFormat; and that defaults to true, but may be configured differently at 3 levels: global default, per-type (LocalDate) and per-property.
I don't think this is yet supported by this particular deserializer, but it could be; and I think it would make sense... especially if coupled with format Shape for deserialization (not just serialization).

It is some work but it seems to me like a good way to do this -- my intent going forward is to limit number of on/off feature flags that are type-specific, and @JsonFormat (along with Config Overrides that allow specifying overrides for types and global default(s)).

[cowtowncoder]

I think that there should be a way to prevent this, but I am not sure what the correct path would be.
A new module-configuration feature is a possibility, I suppose.

[kupci]

I think that there should be a way to prevent this, but I am not sure what the correct path would be.
A new module-configuration feature is a possibility, I suppose.

What about the format Shape idea mentioned previously?

there is property lenient for @jsonformat; [snip] _ and I think it would make sense... especially if coupled with format Shape for deserialization (not just serialization)._

[cowtowncoder]

@kupci I go back and forth here, since for many date/time values integer representations ("timestamps") seem like a legit use case.

But for specific case of LocalDate (not even LocalDateTime which... one could argue could be a timestamp), number of days is sort of esoteric.

So, yes, if you could do a patch that blocks this if strict (not lenient) that'd be good.
Actually, one twist would be that if Shape of NUMBER or NUMBER_INT is specified, it should of course be accepted (but, I suppose, STRING not?).

Now, this would be great to get in 2.10 -- time is getting bit tight, release by 25th. :)
I'll give this issue priority if need be.

[kupci]

It turns out this bug is already partially fixed. The leniency check is already in the LocalDateDeserializer:

        // 06-Jan-2018, tatu: Is this actually safe? Do users expect such coercion?
        if (parser.hasToken(JsonToken.VALUE_NUMBER_INT)) {
            if (!isLenient()) {
                return _failForNotLenient(parser, context, JsonToken.VALUE_STRING);
            }
            return LocalDate.ofEpochDay(parser.getLongValue());
        }

As for the Shape of NUMBER_INT requirement, that still needs to be implemented in the Deserializer. Based on the code review, it looks like that was the original intention but it somehow only got to the LocalDateSerializer. Oh, and some documentation and tests of course.

Now one question - for the Shape, I see the constructor signatures were changed to add that setting, which of course I can follow for the Deserializer, maybe just LocalDateDeserializer as it doesn't need to be any higher at this point. However, I also notice that in the Serializer hierarchy, it is accessible already, as you can see it retrieved in the JSR310FormattedSerialzerBase.createContextual method:

       JsonFormat.Value format = findFormatOverrides(prov, property, handledType());
[snip]
            JsonFormat.Shape shape = format.getShape();

So maybe that's a cleaner way to go, but I don't see that same capability in the Deserializer hierarchy.

[cowtowncoder]

I agree that it would probably make sense to add a shared helper method in base class for deserializers too. I do not remember the history here, but in the past serializers only were affected (to select one of many representations), whereas deserializers were always "lenient" thereby not caring too much about intended shape. At least for datatypes; there may have been some exceptions.

So probably it just wasn't felt it was needed. But now is.

[kupci]

It's there (the findFormatOverrides, in StdDeserializer), not sure what I was thinking. That makes things easier. But I think I'll finish up #138 and then come back to this one, since at least it does have the leniency check.

[cowtowncoder]

@kupci Sounds good. I'm happy with the progress made here -- thank you very much for your fixes.
I think I could give you commit access to this module soon: I'll send an email about that.

[cowtowncoder]

Quick note: due to 2.10.0 release, now needs to go to 2.11 branch for 2.11.0, as it is behavioral change and could break legit usage: something to avoid in a patch.