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

Prevent deserialization of "" as null for Duration, Instant, LocalTime, OffsetTime, and YearMonth in "strict" (non-lenient) mode #138

Closed
kupci opened this issue Sep 16, 2019 · 3 comments
Labels
active Issue being actively investigated and/or worked on

Comments

@kupci
Copy link
Member

kupci commented Sep 16, 2019

For the java.time classes Duration, Instant, LocalTime, OffsetTime and YearMonth, an empty string is mapped to null by the associated deserializer, for example DurationDeserializer, InstantDeserializer, etc. 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, so that the associated deserializer (DurationDeserializer, etc) could report this as an error instead of mapping to null. This capability should be noted in the documentation.

This issue is related to issue #114 which was fixed for LocalDate and LocalDateTime. However, the fix is still required for the java.time types listed here.

@cowtowncoder
Copy link
Member

Ok, merging to master was some work.. .but is done.

I did however uncover one concern: #68 fix was undone by this change (2 tests broke). I think I will just comment out those tests on short term, but need to figure how these settings should work together.

@kupci
Copy link
Member Author

kupci commented Oct 18, 2019

Cool, and I will take a look at #68. Lenient is set by default so empty string should pass through, so probably something else..

@cowtowncoder
Copy link
Member

I think what happened was more along the lines that I had added new handling in master, but due to changes from 2.11, merging, those are overridden. It's fine as nothing is released, and more a question of what should be the overall algorithm for determining acceptance. And not so much a bug introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
active Issue being actively investigated and/or worked on
Projects
None yet
Development

No branches or pull requests

2 participants