Skip to content
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

Throws exception for very large JSON numbers #57

Open
lydell opened this issue Nov 20, 2023 · 1 comment
Open

Throws exception for very large JSON numbers #57

lydell opened this issue Nov 20, 2023 · 1 comment

Comments

@lydell
Copy link

lydell commented Nov 20, 2023

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 [email protected](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 [email protected](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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants