Skip to content

Commit 09216a8

Browse files
xperiandrixperiandri
committed
Merge pull request #490 from fsprojects/input_object_deserialization_fix
Check if input object CLR property type does not match the scalar's CLR type declared in GraphQL object definition
2 parents e2ff051 + 5da5dd9 commit 09216a8

File tree

4 files changed

+150
-27
lines changed

4 files changed

+150
-27
lines changed

‎src/FSharp.Data.GraphQL.Server/ReflectionHelper.fs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@ module internal ReflectionHelper =
132132

133133
let [<Literal>] OptionTypeName = "Microsoft.FSharp.Core.FSharpOption`1"
134134
let [<Literal>] ValueOptionTypeName = "Microsoft.FSharp.Core.FSharpValueOption`1"
135+
let [<Literal>] ListTypeName = "Microsoft.FSharp.Collections.FSharpList`1"
136+
let [<Literal>] ArrayTypeName = "System.Array`1"
137+
let [<Literal>] IEnumerableTypeName = "System.Collections.IEnumerable"
138+
let [<Literal>] IEnumerableGenericTypeName = "System.Collections.Generic.IEnumerable`1"
135139

136140
let isParameterOptional (p: ParameterInfo) =
137141
p.IsOptional
@@ -140,6 +144,50 @@ module internal ReflectionHelper =
140144

141145
let isPrameterMandatory = not << isParameterOptional
142146

147+
let unwrapOptions (ty : Type) =
148+
if ty.FullName.StartsWith OptionTypeName || ty.FullName.StartsWith ValueOptionTypeName then
149+
ty.GetGenericArguments().[0]
150+
else ty
151+
152+
let isAssignableWithUnwrap (from: Type) (``to``: Type) =
153+
154+
let checkCollections (from: Type) (``to``: Type) =
155+
if
156+
// TODO: Implement support of other types of collections using collection initializers
157+
(``to``.FullName.StartsWith ListTypeName || ``to``.FullName.StartsWith ArrayTypeName)
158+
&& (from.IsGenericType
159+
&& from.GenericTypeArguments[0].IsAssignableTo(``to``.GenericTypeArguments[0])
160+
&& from.GetInterfaces()
161+
|> Array.exists (
162+
fun i -> i.FullName.StartsWith IEnumerableGenericTypeName
163+
|| i.FullName = IEnumerableTypeName
164+
)
165+
)
166+
167+
then
168+
let fromType = from.GetGenericArguments()[0]
169+
let toType = ``to``.GetGenericArguments()[0]
170+
fromType.IsAssignableTo toType
171+
else
172+
false
173+
174+
let actualFrom =
175+
if from.FullName.StartsWith OptionTypeName || from.FullName.StartsWith ValueOptionTypeName then
176+
from.GetGenericArguments()[0]
177+
else from
178+
let actualTo =
179+
if ``to``.FullName.StartsWith OptionTypeName || ``to``.FullName.StartsWith ValueOptionTypeName then
180+
``to``.GetGenericArguments()[0]
181+
else ``to``
182+
183+
let result = actualFrom.IsAssignableTo actualTo || checkCollections actualFrom actualTo
184+
if result then result
185+
else
186+
if actualFrom.FullName.StartsWith OptionTypeName || actualFrom.FullName.StartsWith ValueOptionTypeName then
187+
let actualFrom = actualFrom.GetGenericArguments()[0]
188+
actualFrom.IsAssignableTo actualTo || checkCollections actualFrom actualTo
189+
else result
190+
143191
let matchConstructor (t: Type) (fields: string []) =
144192
if FSharpType.IsRecord(t, true) then FSharpValue.PreComputeRecordConstructorInfo(t, true)
145193
else

‎src/FSharp.Data.GraphQL.Server/Values.fs‎

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ let private normalizeOptional (outputType : Type) value =
3636
| value ->
3737
let inputType = value.GetType ()
3838
if inputType.Name <> outputType.Name then
39-
let expectedOutputType = outputType.GenericTypeArguments[0]
39+
// Use only when option or voption so must not be null
40+
let expectedOutputType = outputType.GenericTypeArguments.FirstOrDefault()
4041
if
4142
outputType.FullName.StartsWith ReflectionHelper.OptionTypeName
4243
&& expectedOutputType.IsAssignableFrom inputType
@@ -50,19 +51,20 @@ let private normalizeOptional (outputType : Type) value =
5051
let valuesome, _, _ = ReflectionHelper.vOptionOfType expectedOutputType
5152
valuesome value
5253
else
53-
let realInputType = inputType.GenericTypeArguments[0]
54+
// Use only when option or voption so must not be null
55+
let actualInputType = inputType.GenericTypeArguments.FirstOrDefault()
5456
if
5557
inputType.FullName.StartsWith ReflectionHelper.OptionTypeName
56-
&& outputType.IsAssignableFrom realInputType
58+
&& outputType.IsAssignableFrom actualInputType
5759
then
58-
let _, _, getValue = ReflectionHelper.optionOfType realInputType
60+
let _, _, getValue = ReflectionHelper.optionOfType actualInputType
5961
// none is null so it is already covered above
6062
getValue value
6163
elif
6264
inputType.FullName.StartsWith ReflectionHelper.ValueOptionTypeName
63-
&& outputType.IsAssignableFrom realInputType
65+
&& outputType.IsAssignableFrom actualInputType
6466
then
65-
let _, valueNone, getValue = ReflectionHelper.vOptionOfType realInputType
67+
let _, valueNone, getValue = ReflectionHelper.vOptionOfType actualInputType
6668
if value = valueNone then null else getValue value
6769
else
6870
value
@@ -107,10 +109,17 @@ let rec internal compileByType
107109
let objtype = objDef.Type
108110
let ctor = ReflectionHelper.matchConstructor objtype (objDef.Fields |> Array.map (fun x -> x.Name))
109111

110-
let struct (mapper, nullableMismatchParameters, missingParameters) =
112+
let struct (mapper, typeMismatchParameters, nullableMismatchParameters, missingParameters) =
111113
ctor.GetParameters ()
112114
|> Array.fold
113-
(fun struct (all : ResizeArray<_>, areNullable : HashSet<_>, missing : HashSet<_>) param ->
115+
(fun struct (
116+
all : ResizeArray<_>,
117+
mismatch : HashSet<_>,
118+
areNullable : HashSet<_>,
119+
missing : HashSet<_>
120+
)
121+
param
122+
->
114123
match
115124
objDef.Fields
116125
|> Array.tryFind (fun field -> field.Name = param.Name)
@@ -122,27 +131,41 @@ let rec internal compileByType
122131
&& field.DefaultValue.IsNone
123132
->
124133
areNullable.Add param.Name |> ignore
125-
| _ -> all.Add (struct (ValueSome field, param)) |> ignore
134+
| inputDef ->
135+
if ReflectionHelper.isAssignableWithUnwrap inputDef.Type param.ParameterType then
136+
all.Add (struct (ValueSome field, param)) |> ignore
137+
else
138+
// TODO: Consider improving by specifying type mismatches
139+
mismatch.Add param.Name |> ignore
126140
| None ->
127141
if ReflectionHelper.isParameterOptional param then
128142
all.Add <| struct (ValueNone, param) |> ignore
129143
else
130144
missing.Add param.Name |> ignore
131-
struct (all, areNullable, missing))
132-
struct (ResizeArray (), HashSet (), HashSet ())
133-
134-
if missingParameters.Any () then
135-
raise
136-
<| InvalidInputTypeException (
137-
$"Input object '%s{objDef.Name}' refers to type '%O{objtype}', but mandatory constructor parameters '%A{missingParameters}' don't match any of the defined input fields",
138-
missingParameters.ToImmutableHashSet ()
139-
)
140-
if nullableMismatchParameters.Any () then
141-
raise
142-
<| InvalidInputTypeException (
143-
$"Input object %s{objDef.Name} refers to type '%O{objtype}', but optional fields '%A{missingParameters}' are not optional parameters of the constructor",
144-
nullableMismatchParameters.ToImmutableHashSet ()
145-
)
145+
struct (all, mismatch, areNullable, missing))
146+
struct (ResizeArray (), HashSet (), HashSet (), HashSet ())
147+
148+
let exceptions : exn list = [
149+
if missingParameters.Any () then
150+
InvalidInputTypeException (
151+
$"Input object '%s{objDef.Name}' refers to type '%O{objtype}', but mandatory constructor parameters '%A{missingParameters}' don't match any of the defined input fields",
152+
missingParameters.ToImmutableHashSet ()
153+
)
154+
if nullableMismatchParameters.Any () then
155+
InvalidInputTypeException (
156+
$"Input object %s{objDef.Name} refers to type '%O{objtype}', but optional fields '%A{missingParameters}' are not optional parameters of the constructor",
157+
nullableMismatchParameters.ToImmutableHashSet ()
158+
)
159+
if typeMismatchParameters.Any () then
160+
InvalidInputTypeException (
161+
$"Input object %s{objDef.Name} refers to type '%O{objtype}', but fields '%A{typeMismatchParameters}' have different types than constructor parameters",
162+
typeMismatchParameters.ToImmutableHashSet ()
163+
)
164+
]
165+
match exceptions with
166+
| [] -> ()
167+
| [ ex ] -> raise ex
168+
| _ -> raise (AggregateException ($"Invalid input object '%O{objtype}'", exceptions))
146169

147170
let attachErrorExtensionsIfScalar inputSource path objDef (fieldDef : InputFieldDef) result =
148171

‎tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ type InputObjectNested (homeAddress : InputObject, workAddress : InputObject vop
7676
let InputObjectNestedType =
7777
Define.InputObject<InputObjectNested> (
7878
"InputObjectOptional",
79-
[ Define.Input ("homeAddress", InputRecordType)
80-
Define.Input ("workAddress", Nullable InputRecordType)
81-
Define.Input ("mailingAddress", Nullable InputRecordType) ],
79+
[ Define.Input ("homeAddress", InputObjectType)
80+
Define.Input ("workAddress", Nullable InputObjectType)
81+
Define.Input ("mailingAddress", Nullable InputObjectType) ],
8282
fun (inputObject: InputObjectNested) ->
8383
match inputObject.MailingAddress, inputObject.WorkAddress with
8484
| None, ValueNone -> ValidationError <| createSingleError "MailingAddress or WorkAddress must be provided"

‎tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputScalarAndAutoFieldScalarTests.fs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module FSharp.Data.GraphQL.Tests.InputScalarAndAutoFieldScalarTests
66

77
open Xunit
88
open System
9+
open System.Text.Json.Serialization
910

1011
open FSharp.Data.GraphQL
1112
open FSharp.Data.GraphQL.Types
@@ -210,3 +211,54 @@ let ``Execute handles nullable auto-fields in input and output object fields coe
210211
empty errors
211212
data |> equals (upcast expected)
212213

214+
215+
open FSharp.Data.GraphQL.Tests.OptionalsNormalizationTests
216+
217+
[<RequireQualifiedAccess>]
218+
module ConsoleLoginProviders =
219+
220+
let [<Literal>] Microsoft365 = "microsoft365"
221+
let [<Literal>] GoogleWorkspace = "google_workspace"
222+
223+
type ConsoleLoginProvider =
224+
| [<JsonName(ConsoleLoginProviders.Microsoft365)>] Microsoft365
225+
| [<JsonName(ConsoleLoginProviders.GoogleWorkspace)>] GoogleWorkspace
226+
and ApplicationTenantId = ValidString<InputRecord>
227+
and WrongInput = {
228+
Id : ApplicationTenantId
229+
/// Legal entity name
230+
Name : string
231+
LoginProvider : ConsoleLoginProvider
232+
AllowedDomains : string list
233+
/// Tenants visible to a management company
234+
AuthorizedTenants : ApplicationTenantId list
235+
}
236+
237+
// Checks that IndexOutOfRangeException no longer happens in normalizeOptional
238+
[<Fact>]
239+
let ``Schema cannot be created for unmatched input field types on record`` () =
240+
241+
let ``InputRecord without proper scalars Type`` =
242+
Define.InputObject<WrongInput> (
243+
"InputRecordWithoutProperScalars",
244+
[ Define.Input ("id", StringType)
245+
Define.Input ("name", StringType)
246+
Define.Input ("loginProvider", StringType)
247+
Define.Input ("allowedDomains", ListOf StringType)
248+
Define.Input ("authorizedTenants", ListOf GuidType) ]
249+
)
250+
251+
Assert.Throws<InvalidInputTypeException> (fun () ->
252+
Schema (
253+
query =
254+
Define.Object (
255+
"Query",
256+
[ Define.Field (
257+
"wrongRecord",
258+
StringType,
259+
[ Define.Input ("record", ``InputRecord without proper scalars Type``) ],
260+
stringifyInput
261+
) ]
262+
)
263+
) |> Executor :> obj
264+
)

0 commit comments

Comments
 (0)