-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Offers APIs allowing to customise the underlying JSON library (with the risk of breaking something by the user) #194
Comments
Hello, This is caused by NewtonsSoft which introduce the max depth 64 for working around a security vulnerability. I see 2 solutions, either I set the Or, the user shadow the Note This is a first time that I see such feature as TBH I am not sure what is the best decision to take here. If people with more experience in security domain etc. can voice in I am interested in reading what you think? |
With the upcoming changes to Each JSON backend can provide additional functions, where options specific to that backend are supplied. For example, let fromString (decoder: Decoder<'T>) : Result<'T, string> =
// ...
let unsafeFromString (decoder: Decoder<'T>) : 'T =
// ...
let fromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : Result<'T, string> =
// ...
let unsafeFromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : 'T =
// ... This allows each backend to be configured in arbitrary ways, whilst also keeping a simple default. |
that sounds good. As I understand its not yet in the package? |
Indeed, this is an opportunity. We could decide on a set of common public API they all should have like: Decode.fromString
Decode.unsafeFromString
// etc. and also expose more specialised handlers to expose customisation for people that need it. These handlers can be dangerous because if not setup correctly then user could generate a JSON which is not supported by other backend. For exemple, in the case of NewtonSoft the following settings are mandartory: let serializationSettings =
JsonSerializerSettings(
DateParseHandling = DateParseHandling.None,
CheckAdditionalContent = true
)
// Pseudo code, I don't know if this is allowed by NewtonSoft
let fromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : Result<'T, string> =
options.DateParseHandling <- DateParseHandling.None
options.CheckAdditionalContent <- true
The new packages are already published in beta:
But they are not yet officially announced. I am finishing the documentation so people know how to consume then, even if there is not much changes in reality. The main changes is that to share APIs you don't need to use compiler directives anymore. Also these packages for now only support the manual API, auto API will be supported soon too thanks to @njlr work. |
// Pseudo code, I don't know if this is allowed by NewtonSoft
let fromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : Result<'T, string> =
options.DateParseHandling <- DateParseHandling.None
options.CheckAdditionalContent <- true I don't think this is appropriate since it prevents users from having full control - maybe they have a good reason for One approach in Elm is to have an So perhaps this? module Decode =
let fromString (decoder: Decoder<'T>) : Result<'T, string> =
// ...
let unsafeFromString (decoder: Decoder<'T>) : 'T =
// ...
module Advanced =
module Decode =
let fromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : Result<'T, string> =
// ...
let unsafeFromStringWithOptions (options : JsonSerializerSettings) (decoder: Decoder<'T>) : 'T =
// ... |
That's what I tough too.
This is indeed something we are already using in Thoth.Json.Net |
I dont know about enforcing some values, maybe we need to be flexible here. I implemented it similar in my solution but this function was in a shared project used by fable and .net. And JsonSerializerSettings cannot be used with fable. So it only works for .net so far. i had to put the directives around that. #if !FABLE_COMPILER
let fromStringWithOptions (options: JsonSerializerSettings) (decoder: Decoder<'T>) =
fun value ->
try
let serializer = JsonSerializer.Create(options)
...
#endif |
This situation should not happen anymore with the new API and packages. The decoder/encoder API is runtime agnostics meaning that you can share them across packages without the need for compiler directives.
module Shared.Types
open Thoh.Json.Core
type User =
{
Name : string
}
module User =
let decoder : Decoder<User> =
Decode.object (fun get ->
{
Name = get.Required.Field "name" Decode.string
}
)
open Shared.Typed
open Thoth.Json.JavaScript
Decode.fromString "json" User.decoder
open Shared.Typed
open Thoth.Json.Newtonsoft
Decode.fromString "json" User.decoder |
Thanks for all the answers. I got that working now. Still have to make the move to the new core packages |
I am re-opening because we want to provide ability to the user to customise the options |
Hi. I am using a json with more than 64 level of hierarchy. How can I set the maxdepth in an easy way without overriding the fromstring function.
error i am getting:
Given an invalid JSON: The reader's MaxDepth of 64 has been exceeded. Path 'documentLibs[0].folders[0].folders[0].folders[1].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0].folders[0]', line 1418, position 129.
currently i am overwriting the function but this doesnt struck me as a good solution.
The text was updated successfully, but these errors were encountered: