Expected Behavior

The following GraphQL query (link to PoC)

query ($account: Account) {
  balance(account:$account)
}

{
  "account": {
    "id": 314159,
    "balance": 250.47
  }
}

should respond with:

{
  "data": {
    "balance": 250.47
  }
}

Actual Behavior

The GraphQL query raises an error:

{
	"data": null,
	"errors": [
		{
			"message": "Invalid value for argument \"account.balance\", expected type \"Float\"",
			"locations": [
				{
					"line": 2,
					"column": 6
				}
			]
		}
	]
}

Steps to Reproduce the Problem

1. Create a simple GraphQL server which accepts and manipulates float inputs (or clone this repo)
2. Provide the inputs as GraphQL request variables

The same issue occurs with integers.

Analysis

Root cause

The problem occurs when using serde-json with the arbitrary_precision feature, either directly or through a dependency. serde-json uses special objects to represent numeric values when this feature is enabled:

• JSON input: {"pi": 3.14159}
• serde object: {Name("pi"): Object({Name("$serde_json::private::Number"): String("3.14159")})}
• serde object without arbitrary_precision feature: {Name("pi"): Number(3.14159)}.

When a HTTP request comes in, async-graphql deserializes the raw payload into a Request object whose attributes include the query variables. Each variable is stored as an async_graphql::Value.

The arbitrary-precision feature, and its object representation of numeric values, causes async-graphql to miscategorize variables types, thus the error: `Invalid value for argument x, expected type T".

On one hand, I think this is a problem with serde-json API, as there have been multiple reports of people having issues with the arbitrary_precision feature. On the other hand, the feature has been there for years so I don't see it going away soon. Even if async-graphql users don't enable the feature directly, they could still depend on libraries that do.

Fix

I have identified and quickly put to test two possible "fixes", but I am not familiar with this codebase so there might be better ones:

1. multiple GH issues such as this one mention a "hack" to properly convert arbitrary-precision numbers to Rust types by introducing an intermediate conversion step. Basically, you first convert your JSON string to a serde_json::value::Value object, and then deserialize it into your custom type (that would be Request). The double conversion is very inefficient (by what I read, I did not run any benchmarks) so we might consider putting it behind a feature flag and documenting it properly so users are aware of it. Practically speaking, I am suggesting we edit the receive_batch_json function in src/http/mod.rs replacing the current call to serde_json::from_slice<BatchRequest> with this code:

       // This intermediate deserialization into a Value object is required arbitrary-precision numbers
       // but it might slow down JSON parsing [1] compared to using serde_json::from_slice<BatchRequest>
       //
       // [1]: https://github.com/serde-rs/json/issues/896
       serde_json::from_slice::<serde_json::Value>(&data)
           .and_then(serde_json::from_value::<BatchRequest>)
           .map_err(|e| ParseRequestError::InvalidRequest(Box::new(e)))

2. alternatively, we make async-graphql aware of this internal detail of serde and check against it when implementing the ScalarType trait for Rust types. The main downside is that we expose ourselves to the internal specifics of the serde library. Additionally, numeric variables will still be stored as objects so other code in the library might have issues with it. Practically speaking, I am suggesting we edit external types definition files such as float's by adding an additional match clause to the parse and is_valid functions:

       Value::Object(n) => match n.get("$serde_json::private::Number") {
           Some(String(s)) => s
               .parse::<f64>()
               .map_err(|_| InputValueError::from("Invalid number")),
           _ => Err(InputValueError::from("Invalid number")),
       },

Overall, I am not fully happy with either solution. I would like to get your opinion on how to tackle this problem before I open a pull request.

Miscellaneous notes

• Although I did not check thoroughly, decimals and bigdecimals should not be affected as libraries such as rust_decimal and bigdecimal support the arbitrary-precision feature.
• This problem is not unique to async-graphql. Juniper is also affected by the same issue and so are many non-GraphQL Rust crates.

Specifications

• Version: 7.0.17
• Platform: Linux
• Subsystem: --