Non-finite values of IEEE 754 floats are not decodable

[ysangkok]

Ok, maybe you don't even wanna support this. But it would be nice to specify if this is on purpose.

Aeson 2 can roundtrip plus/minus infinity and NaN just fine:

repl> Data.Aeson.decode @Double $ encode @Double ((1)/0)
Just Infinity
repl> Data.Aeson.decode @Double $ encode @Double ((-1)/0)
Just (-Infinity)
repl> Data.Aeson.decode @Double $ encode @Double ((0)/0)
Just NaN

But Hermes can't decode these encodings:

repl> newtype Person = Person Double deriving (Eq, Show)
repl> personDecoder = Data.Hermes.withObject $ \obj -> Person <$> atKey "lol" double obj
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double ((1)/0)) <> "}"
Left (SIMDException (HError {path = "/lol", errorMsg = "Error while getting value of type double. The JSON element does not have the requested type.", docLocation = "\"+inf\"}", docDebug = "json_iterator [ depth : 2, structural : '\"', offset : 8', error : No error ]"}))
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double ((-1)/0)) <> "}"
Left (SIMDException (HError {path = "/lol", errorMsg = "Error while getting value of type double. The JSON element does not have the requested type.", docLocation = "\"-inf\"}", docDebug = "json_iterator [ depth : 2, structural : '\"', offset : 8', error : No error ]"}))
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double ((0)/0)) <> "}"
Left (SIMDException (HError {path = "/lol", errorMsg = "Error while getting value of type double. The JSON element does not have the requested type.", docLocation = "null}", docDebug = "json_iterator [ depth : 2, structural : 'n', offset : 8', error : No error ]"}))

Given that people are likely to use this library as a replacement for Aeson, I think it would be useful to point out differences like this.

[velveteer]

I suppose it's possible to write a decoder that would exhibit this behavior, but we would likely lose performance as it's not supported by simdjson: simdjson/simdjson#1540

[ysangkok]

It could be offered as an optional option for those that want to sacrifice performance for compatibility. Here is an implementation, what do you think?

diff --git a/src/Data/Hermes.hs b/src/Data/Hermes.hs
index ef8eff4..5d5e52c 100644
--- a/src/Data/Hermes.hs
+++ b/src/Data/Hermes.hs
@@ -29,6 +29,7 @@ module Data.Hermes
   , atKey
   , atOptionalKey
   , atOrderedKey
+  , atKeyNonFinite
   -- * JSON pointer decoder
   , Pointer
   , mkPointer
@@ -745,6 +746,15 @@ bool = getBool
 double :: Value -> Decoder Double
 double = getDouble
 
+atKeyNonFinite :: Key -> Object -> Decoder Double
+atKeyNonFinite key obj = do
+  mString <- atKey key (nullable string) obj
+  case mString of
+    Just "+inf" -> pure $ 1/0
+    Just "-inf" -> pure $ (-1)/0
+    Just _ -> fail "Expected either a null or the strings 'inf' or '-inf'"
+    Nothing -> pure $ 0/0
+
 -- | Parse a Scientific from a Value.
 scientific :: Value -> Decoder Sci.Scientific
 scientific = withRawByteString parseScientific

In testing:

repl> newtype Person = Person Double deriving (Eq, Show)
repl> personDecoder = Data.Hermes.withObject $ \obj -> Person <$> (atKey "lol" double obj <|> atKeyNonFinite "lol" obj)
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double ((1)/0)) <> "}"
Right (Person Infinity)
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double ((-1)/0)) <> "}"
Right (Person (-Infinity))
repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": " <> (toStrict $ encode @Double (0/0)) <> "}"
Right (Person NaN)

One problem is that with this error message, it could be confusing to the user because of how Alternative doesn't allow for composing error messages. It doesn't tell the user a JSON numeral is a valid value. One way to fix that would be to use https://hackage.haskell.org/package/validation .

repl> Data.Hermes.decodeEither personDecoder $ "{\"lol\": \"evil string\"}"
Left (InternalException (HError {path = "", errorMsg = "Expected either a null or the strings 'inf' or '-inf'", docLocation = "}", docDebug = "json_iterator [ depth : 1, structural : '}', offset : 21', error : No error ]"}))

[velveteer]

I implemented your approach and benchmarked it against doubleNonFinite from #5. Composing double with atKeyNonFinite is 3-4x slower than double, and doubleNonFinite is worst-case 2x slower than double, at least on my machine.

Let me know if that looks good to you, or if you think we can do better here.