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

Default to not over-escaping in SystemTextJson re #44 #45

Merged
merged 3 commits into from
Apr 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions src/FsCodec.SystemTextJson/Options.fs
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,27 @@ type Options private () =
[<Optional; DefaultParameterValue(null)>] ?indent : bool,
/// Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`. Defaults to false.
[<Optional; DefaultParameterValue(null)>] ?camelCase : bool,
/// Ignore null values in input data; defaults to false.
[<Optional; DefaultParameterValue(null)>] ?ignoreNulls : bool) =
/// Ignore null values in input data, don't render fields with null values; defaults to `false`.
[<Optional; DefaultParameterValue(null)>] ?ignoreNulls : bool,
/// Drop escaping of HTML-sensitive characters. defaults to `false`.
[<Optional; DefaultParameterValue(null)>] ?unsafeRelaxedJsonEscaping : bool) =

let indent = defaultArg indent false
let camelCase = defaultArg camelCase false
let ignoreNulls = defaultArg ignoreNulls false
let unsafeRelaxedJsonEscaping = defaultArg unsafeRelaxedJsonEscaping false
let options = JsonSerializerOptions()
if converters <> null then converters |> Array.iter options.Converters.Add
if indent then options.WriteIndented <- true
if camelCase then options.PropertyNamingPolicy <- JsonNamingPolicy.CamelCase; options.DictionaryKeyPolicy <- JsonNamingPolicy.CamelCase
if ignoreNulls then options.IgnoreNullValues <- true
if unsafeRelaxedJsonEscaping then options.Encoder <- System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping
options

/// Opinionated helper that creates serializer settings that provide good defaults for F#
/// - Always prepends `[JsonOptionConverter(); JsonRecordConverter()]` to any converters supplied
/// - no camel case conversion - assumption is you'll use records with camelCased names
/// Opinionated helper that creates serializer settings that represent good defaults for F# <br/>
/// - Always prepends `[JsonOptionConverter(); JsonRecordConverter()]` to any converters supplied <br/>
/// - no camel case conversion - assumption is you'll use records with camelCased names <br/>
/// - renders values with `UnsafeRelaxedJsonEscaping` - i.e. minimal escaping as per `NewtonsoftJson`<br/>
/// Everything else is as per CreateDefault:- i.e. emit nulls instead of omitting fields, no indenting, no camelCase conversion
static member Create
( /// List of converters to apply. Implicit [JsonOptionConverter(); JsonRecordConverter()] will be prepended and/or be used as a default
Expand All @@ -43,11 +48,14 @@ type Options private () =
/// Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`.
/// Defaults to false on basis that you'll use record and tuple field names that are camelCase (but thus not `CLSCompliant`).
[<Optional; DefaultParameterValue(null)>] ?camelCase : bool,
/// Ignore null values in input data; defaults to `false`.
[<Optional; DefaultParameterValue(null)>] ?ignoreNulls : bool) =
/// Ignore null values in input data, don't render fields with null values; defaults to `false`.
[<Optional; DefaultParameterValue(null)>] ?ignoreNulls : bool,
/// Drop escaping of HTML-sensitive characters. defaults to `true`.
[<Optional; DefaultParameterValue(null)>] ?unsafeRelaxedJsonEscaping : bool) =

Options.CreateDefault(
converters = (match converters with null | [||] -> defaultConverters | xs -> Array.append defaultConverters xs),
?ignoreNulls = ignoreNulls,
?indent = indent,
?camelCase = camelCase)
?camelCase = camelCase,
unsafeRelaxedJsonEscaping = defaultArg unsafeRelaxedJsonEscaping true)
29 changes: 29 additions & 0 deletions tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,28 @@ module StjCharacterization =
| Choice1Of2 v -> v = value
| Choice2Of2 m -> m.Contains "The JSON value could not be converted to Microsoft.FSharp.Core.FSharpOption`1[System.String]" @>

// System.Text.Json's JsonSerializerOptions by default escapes HTML-sensitive characters when generating JSON strings
// while this arguably makes sense as a default
// - it's not particularly relevant for event encodings
// - and is not in alignment with the FsCodec.NewtonsoftJson default options
// see https://github.com/dotnet/runtime/issues/28567#issuecomment-53581752 for lowdown
let asRequiredForExamples : System.Text.Json.Serialization.JsonConverter [] =
[| Converters.JsonOptionConverter()
Converters.JsonRecordConverter() |]
type OverescapedOptions() as this =
inherit TheoryData<System.Text.Json.JsonSerializerOptions>()

do // OOTB System.Text.Json over-escapes HTML-sensitive characters - `CreateDefault` honors this
this.Add(Options.CreateDefault(converters = asRequiredForExamples)) // the value we use here requires two custom Converters
// Options.Create provides a simple way to override it
this.Add(Options.Create(unsafeRelaxedJsonEscaping = false))
let [<Theory; ClassData(typedefof<OverescapedOptions>)>] ``provides various ways to use HTML-escaped encoding``(opts : System.Text.Json.JsonSerializerOptions) =
let value = { a = 1; b = Some "\"" }
let ser = Serdes.Serialize(value, opts)
test <@ ser = """{"a":1,"b":"\u0022"}""" @>
let des = Serdes.Deserialize(ser, opts)
test <@ value = des @>

(* Serdes + default Options behavior, i.e. the stuff we do *)

let [<Fact>] records () =
Expand All @@ -49,3 +71,10 @@ let [<Fact>] options () =
test <@ ser = """{"a":1,"b":"str"}""" @>
let des = Serdes.Deserialize ser
test <@ value = des @>

let [<Fact>] ``no over-escaping`` () =
let value = { a = 1; b = Some "\"+" }
let ser = Serdes.Serialize value
test <@ ser = """{"a":1,"b":"\"+"}""" @>
let des = Serdes.Deserialize ser
test <@ value = des @>