Fix #87: check null fields before Deserialize so value types don't fail

Author: Tarmil

src/FSharp.SystemTextJson/Record.fs

@@ -124,11 +124,13 @@ type JsonRecordConverter<'T>(options: JsonSerializerOptions, fsOptions: JsonFSha
                 | ValueSome (i, p) when not p.Ignore ->
                     if p.MustBePresent then
                         requiredFieldCount <- requiredFieldCount + 1
-                    fields.[i] <- JsonSerializer.Deserialize(&reader, p.Type, options)
 
-                    if p.MustBeNonNull && isNull fields.[i] then
+                    reader.Read() |> ignore
+                    if p.MustBeNonNull && reader.TokenType = JsonTokenType.Null then
                         let msg = sprintf "%s.%s was expected to be of type %s, but was null." recordType.Name p.Name p.Type.Name
                         raise (JsonException msg)
+                    else
+                        fields.[i] <- JsonSerializer.Deserialize(&reader, p.Type, options)
                 | _ ->
                     reader.Skip()
             | _ -> ()

src/FSharp.SystemTextJson/Union.fs

@@ -241,17 +241,17 @@ type JsonUnionConverter<'T>
             | false, _ -> ValueNone
 
     let readField (reader: byref<Utf8JsonReader>) (case: Case) (f: Field) options =
-        let v = JsonSerializer.Deserialize(&reader, f.Type, options)
-        if isNull v && f.MustBeNonNull then
+        reader.Read() |> ignore
+        if f.MustBeNonNull && reader.TokenType = JsonTokenType.Null then
             let msg = sprintf "%s.%s(%s) was expected to be of type %s, but was null." ty.Name case.Name f.Name f.Type.Name
             raise (JsonException msg)
-        v
+        else
+            JsonSerializer.Deserialize(&reader, f.Type, options)
 
     let readFieldsAsRestOfArray (reader: byref<Utf8JsonReader>) (case: Case) (options: JsonSerializerOptions) =
         let fieldCount = case.Fields.Length
         let fields = Array.zeroCreate fieldCount
         for i in 0..fieldCount-1 do
-            reader.Read() |> ignore
             fields.[i] <- readField &reader case case.Fields.[i] options
         readExpecting JsonTokenType.EndArray "end of array" &reader ty
         case.Ctor fields :?> 'T

tests/FSharp.SystemTextJson.Tests/FSharp.SystemTextJson.Tests.fsproj

@@ -9,6 +9,7 @@
     <Compile Include="Test.Collection.fs" />
     <Compile Include="Test.Record.fs" />
     <Compile Include="Test.Union.fs" />
+    <Compile Include="Test.Regression.fs" />
     <Compile Include="Program.fs" />
     <None Include="paket.references" />
   </ItemGroup>

tests/FSharp.SystemTextJson.Tests/Test.Regression.fs

@@ -0,0 +1,27 @@
+module Tests.Regression
+
+open Xunit
+open System.Text.Json.Serialization
+open System.Text.Json
+
+type Color = Red | Blue | Green
+
+[<Fact>]
+let ``regression #33`` () =
+    let serializerOptions = JsonSerializerOptions()
+    serializerOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.UnwrapFieldlessTags))
+    let actual = JsonSerializer.Deserialize<Color list>("""[ "Red", "Blue"] """, serializerOptions)
+    Assert.Equal<Color>([Red; Blue], actual)
+    let actual = JsonSerializer.Deserialize("""{"a":"Red","b":"Blue"}""", serializerOptions)
+    Assert.Equal({| a = Red; b = Blue |}, actual)
+
+type A = { A: int; B: string }
+
+[<Fact>]
+let ``regression #87`` () =
+    let options = JsonSerializerOptions()
+    options.Converters.Add(JsonFSharpConverter())
+    let ex1 = Assert.Throws<JsonException>(fun () -> JsonSerializer.Deserialize<A>("""{ "A": 2, "B": null }""", options) |> ignore)
+    Assert.Equal("A.B was expected to be of type String, but was null.", ex1.Message)
+    let ex2 = Assert.Throws<JsonException>(fun () -> JsonSerializer.Deserialize<A>("""{ "A": null, "B": "a" }""", options) |> ignore)
+    Assert.Equal("A.A was expected to be of type Int32, but was null.", ex2.Message)

tests/FSharp.SystemTextJson.Tests/Test.Union.fs

@@ -494,17 +494,6 @@ module NonStruct =
     let ``serialize non-unwrapped single-case`` () =
         Assert.Equal("""{"Case":"Unwrapped","Fields":["foo"]}""", JsonSerializer.Serialize(Unwrapped "foo", noNewtypeOptions))
 
-    type Color = Red | Blue | Green
-
-    [<Fact>]
-    let ``regression #33`` () =
-        let serializerOptions = JsonSerializerOptions()
-        serializerOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.UnwrapFieldlessTags))
-        let actual = JsonSerializer.Deserialize<Color list>("""[ "Red", "Blue"] """, serializerOptions)
-        Assert.Equal<Color>([Red; Blue], actual)
-        let actual = JsonSerializer.Deserialize("""{"a":"Red","b":"Blue"}""", serializerOptions)
-        Assert.Equal({| a = Red; b = Blue |}, actual)
-
     type UnionWithUnitField =
         | UWUF of int * unit