Unexpected UTCTime [de]serialization behaviour

[defanor]

Here is an example:

λ> let t = UTCTime (fromGregorian 2016 12 23) (secondsToDiffTime $ 25 * 3600)
λ> t
2016-12-23 23:59:3660 UTC
λ> encode t
"\"2016-12-23T25:00:00Z\""
λ> eitherDecode (encode t) :: Either String UTCTime
Left "Error in $: could not parse date: Failed reading: invalid time"

Though the time is indeed invalid in this case, I'd expect decode (encode a) == a to hold. It's rather unfortunate when trying to deserialize aeson-generated JSON with aeson, but even if it is for compatibility with other libraries/languages, perhaps serialization shouldn't produce the values that are considered invalid (even though that may lead to information loss). In any case, it's not quite an expected behaviour: input requirements are more strict than output ones.

[phadej]

but you construct invalid UTCTime,
utctDayTime :: DiffTime -- ^ the time from midnight, 0 <= t < 86401s (because of leap-seconds)

Valid options:

• validate hard UTCTime invariant, i.e. error in this case
• clamp seconds
• don't do anything, use newtype for UTCTime with broken invariants

I'm not sure which variant is the best one

[bos]

I think the current behaviour is quite reasonable. It doesn't prevent someone from deliberately constructing and serializing a bogus UTCTime, but that's not the package's job. And it rejects bogus ones. Job done.

[defanor]

An invalid UTCTime could be constructed indeliberately (by mistake) as well, in fact that's how I've found it: by parsing it from a format, composition of which turned out to be buggy (while I've assumed that it's not). That was a bug, and now I'm doing basic validation (akin to that in aeson), but was surprising to find it as a deserialization error. Not a big deal though, just writing to explain it better.

[phadej]

I guess the reasoning is that in Haskell we don't program defensively, as type-safety should guarantee most things. If some other library violated invariant, it's that library problem. it's not aeson job to verify data getting in is valid, it's job of other library to validate that time going out is not invalid.

That's said, there is fromGregorianValid to construct valid Day, but no such analogue for UTCTime, which would be mkUTCTimeValid :: Day -> DiffTime -> Maybe UTCTime (Nothing) or mkUTCTime (clip) or mkUTCTimeRollOver (rollerover to prev / next days).

@defanor do you want to open issue / make PR to time?

[defanor]

fromGregorianValid checks for leap years, while I doubt that it's viable to check for leap seconds in mkUTCTimeValid -- so it still would not be quite consistent (and the time would only be valid just as in "satisfying the given rough invariant"), but it shouldn't harm to open an issue and discuss there as well; I'll do it now.

OOC, why doesn't aeson reuse time's own functions for formatting and parsing? Though neither does it hold in time that parse (print x) == x, and it manages to violate the invariant anyway (with e.g. 99 seconds), but seems like it should also be fixed there.

[phadej]

because we parse Text directly, and format/parse different format, we don't need parseTimeM/formatTime flexiblity (we need different one, actually).