Time decoders should not ignore superfluous characters

[ysangkok]

Given that the OpenAPI date-time decoding codegen defaults to UTCTime, it's way too easy to throw away the offset by accident. You end up parsing a local time as a UTC time, which gives the wrong result. Users of the library might be parsing timestamps from a source that initially doesn't send the offset, then adds it and specifies a regex. The codegen is run again, but the regex is ignored and the utcTime decoder will happily ignore the offset.

Proposed solutions:

• Don't ignore superfluous characters
• Require people to configure the appropriate datetime type instead of defaulting to UTCTime
• Have the default date-time type be one that isn't a prefix of the others. For example ZonedTime instead of UTCTime

  ✗ prop_decodeZonedTimeAsUTCTime failed 
    after 1 test and 3 shrinks.
    shrink path: 1:a3

        ┏━━ test/Spec.hs ━━━
    745 ┃ prop_decodeZonedTimeAsUTCTime :: HH.Property
    746 ┃ prop_decodeZonedTimeAsUTCTime =
    747 ┃   HH.property $ do
    748 ┃     zonedTime <- HH.forAll genZonedTime
        ┃     │ 2000-01-01 00:00:00 + 0000
    749 ┃     case FA.decode FC.utcTime $ FA.encode FC.zonedTime zonedTime of
    750 ┃       Right _ -> fail "Decoding should have failed"
    751 ┃       Left _ -> pure ()

    Decoding should have failed

[ysangkok]

Test patch

diff --git a/json-fleece-aeson/test/Spec.hs b/json-fleece-aeson/test/Spec.hs
index 412ae5e..6b2ae03 100644
--- a/json-fleece-aeson/test/Spec.hs
+++ b/json-fleece-aeson/test/Spec.hs
@@ -71,6 +71,7 @@ tests =
   , ("prop_encode_union", prop_encode_union)
   , ("prop_decode_taggedUnion", prop_decode_taggedUnion)
   , ("prop_encode_taggedUnion", prop_encode_taggedUnion)
+  , ("prop_decodeZonedTimeAsUTCTime", prop_decodeZonedTimeAsUTCTime)
   , ("prop_utcTimeAndZonedTime", prop_utcTimeAndZonedTime)
   ]

@@ -741,6 +742,14 @@ taggedUnionGen =
           <*> Gen.bool
     ]

+prop_decodeZonedTimeAsUTCTime :: HH.Property
+prop_decodeZonedTimeAsUTCTime =
+  HH.property $ do
+    zonedTime <- HH.forAll genZonedTime
+    case FA.decode FC.utcTime $ FA.encode FC.zonedTime zonedTime of
+      Right _ -> fail "Decoding should have failed"
+      Left _ -> pure ()
+
 prop_utcTimeAndZonedTime :: HH.Property
 prop_utcTimeAndZonedTime =
   HH.property $ do

[jlavelle]

We use attoparsec-iso8601 to parse the times and the UTCTime parser used by FC.utcTime does not throw away the offset. It works by parsing a local time and timezone first and then converting it to a UTCTime. Am I missing something?

Users of the library might be parsing timestamps from a source that initially doesn't send the offset, then adds it and specifies a regex.

What do you mean about the regex?

[jlavelle]

Also I don't understand why you'd expect decoding in that test to fail

[ysangkok]

Oh, I see, the offset is not being thrown away, it is getting integrated into the UTCTime. I was assuming that if the encoded data included an offset, and the decoded data didn't, then the data must have been lost. But that's not the case.

regex

I was referring to the pattern property.