thoth-org / Thoth.Json.Net

https://thoth-org.github.io/Thoth.Json/#.Net-and-NetCore-support
MIT License
33 stars 16 forks source link

Throws exception for very large JSON numbers #57

Open lydell opened 8 months ago

lydell commented 8 months ago

Whenever I use Decode.fromString, I expect it to return either Ok or Error and never throw exceptions.

However, if the JSON contains a really large number, a System.InvalidCastException exception is thrown.

The various int decoders do have bounds checking, and that works to some extent. But if the number is too crazy large, even the bounds check don’t help.

If this is a problem with how Newtonsoft does things, maybe add a try-with in Thoth.Json.Net?

❯ dotnet fsi

Microsoft (R) F# Interactive version 12.7.0.0 for F# 7.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> #r "nuget: Thoth.Json.Net, 11.0.0";;
[Loading /Users/simon/.packagemanagement/nuget/Cache/2fd0b2ac9e4082b4ba06da12786fc9dc357957f471b12f92af911c26d54f0284.fsx]
module FSI_0002.
       2fd0b2ac9e4082b4ba06da12786fc9dc357957f471b12f92af911c26d54f0284

> open Thoth.Json.Net;;

-- int fails:
> Decode.fromString Decode.int "9999999999999999999";;
System.InvalidCastException: Object must implement IConvertible.
   at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
   at Newtonsoft.Json.Linq.Extensions.Convert[T,U](T token)
   at Newtonsoft.Json.Linq.Extensions.Value[T,U](IEnumerable`1 value)
   at Newtonsoft.Json.Linq.Extensions.Value[U](IEnumerable`1 value)
   at Thoth.Json.Net.Decode.int@261.Invoke(String path, JToken value)
   at Thoth.Json.Net.Decode.fromValue[T](String path, FSharpFunc`2 decoder, JToken value)
   at Thoth.Json.Net.Decode.fromString[T](FSharpFunc`2 decoder, String value)
   at <StartupCode$FSI_0004>.$FSI_0004.main@() in /Users/simon/tmp/stdin:line 4
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
Stopped due to error

-- Let’s try int64 instead. Also fails:
> Decode.fromString Decode.int64 "9999999999999999999";;
System.InvalidCastException: Object must implement IConvertible.
   at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
   at Newtonsoft.Json.Linq.Extensions.Convert[T,U](T token)
   at Newtonsoft.Json.Linq.Extensions.Value[T,U](IEnumerable`1 value)
   at Newtonsoft.Json.Linq.Extensions.Value[U](IEnumerable`1 value)
   at Thoth.Json.Net.Decode.int64@277.Invoke(String path, JToken value)
   at Thoth.Json.Net.Decode.fromValue[T](String path, FSharpFunc`2 decoder, JToken value)
   at Thoth.Json.Net.Decode.fromString[T](FSharpFunc`2 decoder, String value)
   at <StartupCode$FSI_0005>.$FSI_0005.main@() in /Users/simon/tmp/stdin:line 5
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
Stopped due to error

-- Removing one `9`, it’s a valid in64:
> Decode.fromString Decode.int64 "999999999999999999";;
val it: Result<int64,string> = Ok 1000000000000000000L

-- And then, when using just int the bounds check works:
> Decode.fromString Decode.int "999999999999999999";;
val it: Result<int,string> =
  Error
    "Error at: `$`
Expecting an int but instead got: 999999999999999999
Reason: Value was either too large or too small for an int"
MangelMaxime commented 8 months ago

I suspect the problem is because of this line:

https://github.com/thoth-org/Thoth.Json.Net/blob/dde35518f9eda3c5d35b39dd97945892e4435018/src/Decode.fs#L213C21-L213C21

We are accessing the value as a float because it allows for big numbers (I think) and do the check after if the number is inside of the correct range.

We could change that to use float32 but this would probably just delay the problem. We can try using a try ... with.

As a note for my self, in the Thoth.Json.Core version of the library the exception will be different depending on the language so I need to think of a way to unify them.