graphql-spring-boot icon indicating copy to clipboard operation
graphql-spring-boot copied to clipboard

maxDepthQuery configuration is not enforced on 12.0.0

Open lhcopetti opened this issue 4 years ago • 2 comments

I am playing with graphql lately and I was trying to mimic an attack where a recursive query is sent by the client to the GraphQL server and it seems like the latest version doesn’t enforce the maxDepthQuery restriction while v11.1.0 works perfectly fine, aborting the execution with a descriptive error:

{
    "errors": [
        {
            "message": "maximum query depth exceeded 18 > 3",
            "extensions": {
                "classification": "ExecutionAborted"
            }
        }
    ],
    "data": null
}

From comparing the dependency:tree’s from my test project, I found that v11.1.0 pulls the spring-boot-starter-actuator on the classpath, while v12.0.0 doesn’t. This may explain why the MaxQueryDepthInstrumentation is not instantiated because the GraphQLInstrumentationAutoConfiguration actually requires MetricsAutoConfiguration to be on the classpath:

https://github.com/graphql-java-kickstart/graphql-spring-boot/blob/a7d2cb703255d38d571629c7208128ce56e36b82/graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/autoconfigure/web/servlet/GraphQLInstrumentationAutoConfiguration.java#L27

Adding the spring-boot-starter-actuator dependency manually to my test spring-boot project with v12.0.0 resolves the issue.

lhcopetti avatar Dec 03 '21 03:12 lhcopetti

v12 indeed has a breaking change that you now need to specify certain dependencies to use instead of pulling in all sorts of deps that you might not use.

oliemansm avatar Dec 03 '21 06:12 oliemansm

What strikes me as odd is the fact that the functionality to prevent cyclic queries is conditional on having the actuator dependency. Is this by design or is there actually some concrete dependency on the actuator for the depth check to run?

lhcopetti avatar Dec 04 '21 01:12 lhcopetti