`InstantDeserializer` is not respecting leniency w.r.t. to numeric values

[wong-git]

We annotated some constructor properties with @jsonformat(lenient = OptBoolean.FALSE) but found that the InstantDeserializer is not applying it when deserializing numeric values.

Expected _failForNotLenient(parser, context, JsonToken.VALUE_STRING) to be invoked for the numeric cases in InstantDeserialize#deserialize when not lenient.

(It looks like LocalDateDeserializer implementation is applying leniency in the case of an integer value as expected; InstantDeserializer should implement similarly.)

[kupci]

Are any SerializationFeatures used, or just defaults? If you have a test case to show the scenario(s), that would be helpful.

[wong-git]

Here's a test. The expectation is that it should throw a mapping exception. If the deserialize method of InstantDeserializer checked leniency and if not lenient, invoked _failForNotLenient for the case JsonTokenId.ID_NUMBER_INT, then this would behave as expected. (This applies to the ID_NUMBER_FLOAT case as well.)

POJO.java.txt
LeniencyTest.java.txt

[cowtowncoder]

One note: while there has been some confusion, leniency/strictness is meant (as of Jackson 2.12) to control specific values for legal types -- like "February 30, 2021" (accepted if lenient; not if strict) -- but it is not meant to be used to control coercions going forward.
Instead, coercion configuration is to be used to control allowed input shapes.

It is possible that InstantDeserializer does not use this mechanism correctly however so there may be a problem. But lenience is not the mechanism to check regarding whether numeric values are to be accepted.

[kupci]

@wong-git referenced the LocalDateDeserializer and, if I'm showing the right logic in that class, it checks for leniency or JsonFormat.Shape.NUMBER_INT so maybe something similar could be added to the InstantDeserializer for consistency, and also adding more robustness/input checking to the parsing logic.

        // 06-Jan-2018, tatu: Is this actually safe? Do users expect such coercion?
        if (parser.hasToken(JsonToken.VALUE_NUMBER_INT)) {
            // issue 58 - also check for NUMBER_INT, which needs to be specified when serializing.
            if (_shape == JsonFormat.Shape.NUMBER_INT || isLenient()) {
                return LocalDate.ofEpochDay(parser.getLongValue());
            }
            return _failForNotLenient(parser, context, JsonToken.VALUE_STRING);
        }

Expanding on @wong-git 's rule, what about this?
"Expected _failForNotLenient(parser, context, JsonToken.VALUE_STRING) to be invoked for the numeric cases in InstantDeserialize#deserialize when JsonFormat.Shape.NUMBER_INT or JsonFormat.Shape.NUMBER_FLOAT unless lenient"

[cowtowncoder]

Given that the default handling with 2.x is lenient, we could consider strict mode to prevent use of numbers.

At the same time we should probably also then check for coercion config.

In fact, I think the way forward would be to check, in case of number formats:

1. Applicable coercion config setting: fail if indicated that should be done (I can help finding the logic)
2. If no coercion config setting, consider lenience (for 2.x -- remove check from 3.0); fail in strict mode

Change in logic to be only done for 2.13, not 2.12.x, given that this is behavioral change.

[kupci]

Applicable coercion config setting: fail if indicated that should be done (I can help finding the logic)

Found some good documentation on coercion config, and from there I see the associated commits, but if you can recommend the logic that would be fine of course.

I think this use case (for Instants) is close to your rule below, except replace Java booleans with a Java date/time types:

_

This feature allows defining coercion rules like:

Let empty String value become POJO similar to being deserialized from { } JSON Input (especially useful for XML)
Let empty String value become null for specified type(s)
**Prevent coercion from JSON Numbers into Java booleans (by default non-zero JSON Integers map to Boolean values as true)**

_

FasterXML/jackson-databind#2113 (comment)

[cowtowncoder]

Yes, coercion block should either be added to

• LogicalType.DateTime (default for Date/Time types) OR
• EXACT Class that is declared for handling; this has precedence over LogicalType (or general default) configuration.

Either way, allowed coercion settings must be verified by deserializer (implemented by them); and we have to figure out expected rules considering backwards compatibility. Gets bit complicated quite quickly.