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

ArgumentException thrown with invalid passed value, when Upload scalar is expected #14

Open
MantasK-DTS opened this issue Oct 22, 2020 · 2 comments

Comments

@MantasK-DTS
Copy link

MantasK-DTS commented Oct 22, 2020

If a query is passed any other value (for example: a string) where an Upload scalar is expected, the IDocumentExecutor throws an unhandled ArgumentException exception. The problem is that the Upload scalar parsing accepts any value, even though it shouldn't.

Here's a relevant stacktrace when trying to pass a variable with value "foo" as the value for the Upload scalar:

System.ArgumentException: Unable to convert 'StringValue{value=foo}' to 'Upload'
   at GraphQL.Execution.ExecutionHelper.CoerceValue(ISchema schema, IGraphType type, IValue input, Variables variables) in C:\projects\graphql-dotnet\src\GraphQL\Execution\ExecutionHelper.cs:line 321
   at GraphQL.Execution.ExecutionHelper.CoerceValue(ISchema schema, IGraphType type, IValue input, Variables variables) in C:\projects\graphql-dotnet\src\GraphQL\Execution\ExecutionHelper.cs:line 306
   at GraphQL.Execution.ExecutionHelper.GetVariableValue(Document document, ISchema schema, VariableDefinition variable, Object input) in C:\projects\graphql-dotnet\src\GraphQL\Execution\ExecutionHelper.cs:line 117
   at GraphQL.Execution.ExecutionHelper.GetVariableValues(Document document, ISchema schema, VariableDefinitions variableDefinitions, Inputs inputs) in C:\projects\graphql-dotnet\src\GraphQL\Execution\ExecutionHelper.cs:line 89
   at GraphQL.DocumentExecuter.BuildExecutionContext(ISchema schema, Object root, Document document, Operation operation, Inputs inputs, IDictionary`2 userContext, CancellationToken cancellationToken, Metrics metrics, List`1 listeners, Boolean throwOnUnhandledException, Action`1 unhandledExceptionDelegate, Nullable`1 maxParallelExecutionCount, IServiceProvider requestServices) in C:\projects\graphql-dotnet\src\GraphQL\Execution\DocumentExecuter.cs:line 247
   at GraphQL.DocumentExecuter.ExecuteAsync(ExecutionOptions options) in C:\projects\graphql-dotnet\src\GraphQL\Execution\DocumentExecuter.cs:line 110

Luckily, there is an easy fix: add a cast to IFormFile in the UploadScalarType.ParseValue method:

public override object ParseValue(object value)
{
	return value as IFormFile;
}

This will make it, so that only an IFormValue is accepted. When ParseValue method returns null, graphql-dotnet will detect this as an invalid value for the type and handle it accordingly.

Another fix might be to throw exceptions on invalid values. When graphql-dotnet detects an exception in the ParseValue method, it assumes that the conversion is not possible and handles it accordingly.

@JannikLassahn
Copy link
Owner

Hi!
I feel like both approaches would make sense. Since graphql-dotnet can handle both cases we can keep it simple and use casting as you suggested.
If you would like to make a PR go right ahead, otherwise I'll fix this over the weekend.

@MantasK-DTS
Copy link
Author

Thank you for responding! I will leave this fix up to you.

ganhammar added a commit to ganhammar/graphql-dotnet-upload that referenced this issue Jun 12, 2022
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