-
Notifications
You must be signed in to change notification settings - Fork 122
Deserializing ZonedDateTime with basic TZ offset notation (0000) #131
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
Comments
Note to check to see what Spring is doing here, i.e. is this a Spring bug, or a different format, but still valid, because as described in the issue 1624, the colon is correct for ISO 8601, extended format:
|
This is an older post but any recommendations on best practices here? I found one of my unit tests failing after refactoring from using Joda datetime to the Java 8 libraries, it's honestly quite annoying how they only support the extended 8601 format as mentioned above. All the purposed workarounds i've managed to google myself forward to have been quite honestly nasty. |
I don't think we have best practice for this yet (and there are a few other cases like this, with Spring), though how about the following workaround?
Also would be curious what other workarounds you found, just to get some ideas on what people are doing. |
Unfortunately that's only a fix that works with UTC time strings. From what i can see on the jsr310 InstantDeserializer.java class, its hardcoded to check for "+0000" specifically, any timezone other then that it will ignore and thus fail. It can technically updated to be a bit more hardened towards other timezones, i guess that could be a fair fix? Other fixes i have looked at usually includes Using DateTimeFormater to restructure the date string, but as far as i can tell, there is not a generic solution as most of them base themselves on parsing one specific ISO 8601 format. Since i unfortunately have an "open" API with many users i'd rather not have to specify which iso 8601 format they should use. |
@oeystein I hope I am not omitting some important piece of context, but to me a work-around to allow supporting "colon-less" offsets for things other than "+0000" does sound like a good addition, and PR would be welcome. If you (or anyone else) would want to try it, branch to base would be |
I have some time to create a PR, given we have @oeystein to hopefully help make sure this works from the Spring side of things. A few questions on implementation Thinking "b" to be more resilient to future changes to the |
I feel that for default format, and for 2.13(.0), it'd be fine to just accept both. Users who want stricter handling / more control could specify I do not have strong preference wrt 2a/2b, going with 2b works for me. |
Apologizes for the late reply from my part, i had to use the hours i had in the weekend to get an overview of the library before proposing a solution. I think it would be smart to make it a default setting, I've seen both versions used depending on the source and it took me some reading to figure out why one was accepted and the other wasn't, especially being used to the joda module from before. But this obviously depends on the policies you guys have for the library and how it should behave. As for question number two, the example of the DateTimeFormatter unfortunately breaks one format which includes the physical timezone, for example "2016-10-27T16:36:08.993+02:00:00[Europe/Paris]". The DateTimeFormatter.ISO_ZONED_DATE_TIME as you brought up currently includes this. We could obviously could just alter the proposed DateTimeFormatter format to also include the Timezone location, but as you mentioned previously resiliency against changes is a huge plus in the book. This also brings up another point. Any changes should probably both be done ZonedDateTime and OffsetDateTime. The format without a physical timezone is more accurate to be used with OffsetDateTime, however ZonedDateTime accepts it as well so i see no point of limiting this in any way. |
That is a good point, in fact there's a bug recorded for this, #38. And I see though there's an interesting solution for that bug, it also has the UTC limitation you mentioned above. Since you have some good ideas on this, would you like to work on the PR? We can assist with code reviews of course. (I think I should've waited for your reply above.) |
Sure, i can have a look on it. That thread adds a lot of additional interesting information so i'll see if i can manage to pull something from it and come up with a solution that seems solid. |
Finally found time for one last check & merged. Thank you for contributing this @oeystein! One last question here: do we want to allow skipping the check if "strict" mode is enabled? That's just one line guard statement, like:
in new But I am not sure if it makes sense to add or not: default is "lenient", for what that is worth; and this check is only used if no custom |
Thanks for the merge @cowtowncoder! This was an issue that really started to bother me, so glad to have it fixed. And obviously always happy to contribute to a project that i have used for 5+ years. And re: lenient vs. strict - I personally have no strong feeling either way. Meanwhile having it be bypassed on a strict setting, i feel personally that as long as a pattern can be specified manually, and someone only wants to use a truly strict pattern they can already do so in a proper way rather then taking a guesswork on what the library will do. I also took a brief look on the code base to see where it was used before, and found this code comment from you last year, so if it's a legacy setting it really should not be used further regardless?
|
@oeystein That comment is bit incomplete, and should perhaps be clarified. The scope of strict/lenient setting has been limited to concern value ranges -- like, say, February 30 (lenient? Ok - strict? fail) -- but not covering type/shape coercions, since for those there is new On using leniency here (vs not): yes, we can leave it out. If there is desire to use it, it can be easily added. And there is still time to consider what to do with all of these for Jackson 3.0... |
Pardon my commenting on a previously closed issue. I'm having this problem myself and noticed something off in the discussion: Isn't the problematic character (idx. 23) the '+', and not the missing colon?
Please correct me if I'm mistaken, I could very easily be misunderstanding something. |
I'm not a 100% sure on why the Java error has "could not be parsed at index 23", because you are correct, that is the '+' symbol, however I think it is that it is indicating it has trouble with that entire block, i.e. the time scale components. What version of Jackson are you using? I see this is for [update] Ah, just checking and I don't think this fix has been included yet in the latest available releases which is 2.12, so likely the issue. You can subscribe to the mailing lists, which have the notifications about the new releases/release candidates, and I know it is always helpful to have a few folks testing the newer features out. |
Yes, @kupci is correct. I added label and and milestone to indicate that this will be addressed in upcoming 2.13 (see https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.13). |
This is an error which is passed on from the ZonedDateTime.parse() method; which Jackson utilizes for the actual parsing. I'm not sure in what scenario that it would report the error at the incorrect place but my guess it could have something to do with java version and/or implementation. I tested it briefly on my setup with openjdk-11, and for me it reports the error correctly at index 26. |
Unfortunately I'm having trouble reproducing the exact bug listed in this post, and that I was (supposedly) experiencing earlier. If I run the code in the OP's initial post (changing only the log / printout and the assertion for notNull), I get the following output: com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java.time.ZonedDateTime` from String "2019-08-22T12:36:46.361+0000": Failed to deserialize java.time.ZonedDateTime: (java.time.format.DateTimeParseException) Text '2019-08-22T12:36:46.361+0000' could not be parsed, unparsed text found at index 26
Label: 0 1 2
0123456789012345678901234567
String: 2019-08-22T12:36:46.361+0000
^
| which seems to be the "correct" location reported for this error. I can confirm that I'm using Jackson version Sidenote: I was also able to work around the problem by deserializing the entire model object containing the |
I stumbled upon deserialization issues when the given string equals to ISO8601 "basic" format.
2019-08-22T12:36:46.361+0000
Exception message:
This format misses out a colon in offset in comparison to extended format (as described at: https://bugs.openjdk.java.net/browse/JDK-8176547).
2019-08-22T12:36:46.361+00:00
The issue states that java.time.format.DateTimeFormatter.ISO_ZONED_DATE_TIME does not support this basic format.
However, as this format is provided by Spring exception handler I was wondering if there exists any workaround to successfully parse this format. Maybe I am simply missing the obvious here?
As current workaround I had to register a custom InstantDeserializer and set com.fasterxml.jackson.datatype.jsr310.deser.InstantDeserializer.replaceZeroOffsetAsZ to true.
Any hint is very much appreciated.
Reproduce
The text was updated successfully, but these errors were encountered: