feat: ast input support added

[Ceres6]

closes #804

JSON ast can be now used as input.

The change can be tested with the code of the test added at test/app-decorator.js

Comments are welcomed for both performance and security on the use of JSON.parse function

It should be decided whether this feature is desired.

[mcollina]

lgtm

[simoneb]

lgtm

[simoneb]

@Ceres6 can you please check if there's a better, more secure (possibly faster) way to handle the JSON parsing? E.g. I came across this https://github.com/fastify/secure-json-parse, but I would check how this is done elsewhere in the codebase, including fastify

[mcollina]

Could you add a test where the source of the query is a JSON but it's missing some key properties (not a valid ast?)

[Ceres6]

@Ceres6 can you please check if there's a better, more secure (possibly faster) way to handle the JSON parsing? E.g. I came across this https://github.com/fastify/secure-json-parse, but I would check how this is done elsewhere in the codebase, including fastify

Fastify uses secure-json-parse, which adds some validation and options to the built-in JSON.parse this would be more secure, definitely not more efficient, but the trade might be worth.

Other options would be using other json-schema validation libraries as joi, ajv, fluent-json-schema, which are used in the Fastify ecosystem. This would require more code to be as precise as possible with the values accepted by the schema, and if we're too strict it could become harder to maintain.

WDYT? @simoneb

[Ceres6]

Could you add a test where the source of the query is a JSON but it's missing some key properties (not a valid ast?)

Of course. I'll see what error throws in that case to see if we need better error handling to guide the client debug it.

[smolinari]

I thought GraphQL does its own validation. Is this direct AST input bypassing that?

Scott

[Ceres6]

I thought GraphQL does its own validation. Is this direct AST input bypassing that?

Scott

No, it is still be done some lines below in the validation phase. It's just the query -> ast conversion that we're skipping

[Ceres6]

Test for the invalid AST added. Let me know if you want me to add an example to the examples folder or something to the documentation.

[mcollina]

lgtm

[Ceres6]

How is it possible that there is a fail in windows 16.x if the only change between this commit and the previous is that I added a test? Besides it is passing in ubuntu 16.x and windows 18.x

[mcollina]

Flaky CI maybe?

[simoneb]

@mcollina wdyt about json parsing in this PR? is it ok to do it or shall we use a json parsing library such as https://github.com/fastify/secure-json-parse?

[mcollina]

Maybe it's better to add a test with an external request.

[Ceres6]

Maybe it's better to add a test with an external request.

Would app.inject be enough or should I listen to a port and then use any library to fetch the response?

[mcollina]

lgtm