FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
401 stars 117 forks source link

Prevent deserialization of "" as `null` for `LocalDate`, `LocalDateTime` in "strict" (non-lenient) mode #114

Closed beytun closed 5 years ago

beytun commented 5 years ago

For LocalDate and LocalDateTime deserialization, empty string is mapped to null by LocalDateDeserializer and LocalDateTimeDeserializer respectively. So something like {"date": null} can't be distinguished from {"date": ""}. From the view of a strict API, the latter is an error.

It would be good to add some feature to treat empty string as invalid format, that LocalDateDeserializer and LocalDateTimeDeserializer could report this as an error instead of mapping to null.

cowtowncoder commented 5 years ago

Perhaps use of DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT would work here, although my main concern is that since default value is false, adding a check would make formerly accepted values invalid with 2.10...

Alternative way would be to add a new option/feature in JavaTimeModule, where it could be locally enabled/disabled.

cowtowncoder commented 5 years ago

I think that implementing either mechanism would be relatively easy, but should be discussed on mailing list before making change. Marking as easy so new contributors can find it.

kupci commented 5 years ago

From the view of a strict API, the latter is an error.

So maybe there is one more way: use the lenient/strict setting, and throw an exception, instead of returning null in the case of an empty string? For example, that is already used in LocalDateDeserializer to throw an exception:

        if (parser.hasToken(JsonToken.VALUE_NUMBER_INT)) {
            if (!isLenient()) {
                return _failForNotLenient(parser, context, JsonToken.VALUE_STRING);
            }
            return LocalDate.ofEpochDay(parser.getLongValue());
        }

Btw, a test case would be useful here, as I am not able to reproduce the issue, it fails with different exceptions for null vs empty string before it even gets to the LocalDate / LocalDateTime deserialization. It may have been fixed, or my test case is not exposing the issue.

kupci commented 5 years ago

I am able to reproduce the issue, so I'll post a "Feedback wanted" on the mailing list to discuss possible solutions further.

cowtowncoder commented 5 years ago

After discussions, thinking it through, I think use of lenient (vs strict) makes sense here as the immediate step.