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.

Sign up for GitHub

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

Jump to bottom

Improve Good faith introspection error handling #3546

Merged
merged 1 commit into from Mar 26, 2024
Merged

Improve Good faith introspection error handling #3546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/main/java/graphql/introspection/GoodFaithIntrospection.java
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@
import graphql.GraphQLContext;
import graphql.GraphQLError;
import graphql.PublicApi;
import graphql.execution.AbortExecutionException;
import graphql.execution.ExecutionContext;
import graphql.language.SourceLocation;
import graphql.normalized.ExecutableNormalizedField;
Expand Down Expand Up @@ -85,14 +86,20 @@ public static boolean enabledJvmWide(boolean flag) {

public static Optional<ExecutionResult> checkIntrospection(ExecutionContext executionContext) {
if (isIntrospectionEnabled(executionContext.getGraphQLContext())) {
ExecutableNormalizedOperation operation = mkOperation(executionContext);
ExecutableNormalizedOperation operation;
try {
operation = mkOperation(executionContext);
} catch (AbortExecutionException e) {
BadFaithIntrospectionError error = BadFaithIntrospectionError.tooBigOperation(e.getMessage());
return Optional.of(ExecutionResult.newExecutionResult().addError(error).build());
}
ImmutableListMultimap<FieldCoordinates, ExecutableNormalizedField> coordinatesToENFs = operation.getCoordinatesToNormalizedFields();
for (Map.Entry<FieldCoordinates, Integer> entry : ALLOWED_FIELD_INSTANCES.entrySet()) {
FieldCoordinates coordinates = entry.getKey();
Integer allowSize = entry.getValue();
ImmutableList<ExecutableNormalizedField> normalizedFields = coordinatesToENFs.get(coordinates);
if (normalizedFields.size() > allowSize) {
BadFaithIntrospectionError error = new BadFaithIntrospectionError(coordinates.toString());
BadFaithIntrospectionError error = BadFaithIntrospectionError.tooManyFields(coordinates.toString());
return Optional.of(ExecutionResult.newExecutionResult().addError(error).build());
}
}
Expand All @@ -108,7 +115,7 @@ public static Optional<ExecutionResult> checkIntrospection(ExecutionContext exec
*
* @return an executable operation
*/
private static ExecutableNormalizedOperation mkOperation(ExecutionContext executionContext) {
private static ExecutableNormalizedOperation mkOperation(ExecutionContext executionContext) throws AbortExecutionException {
Options options = Options.defaultOptions()
.maxFieldsCount(GOOD_FAITH_MAX_FIELDS_COUNT)
.maxChildrenDepth(GOOD_FAITH_MAX_DEPTH_COUNT)
Expand All @@ -133,8 +140,16 @@ private static boolean isIntrospectionEnabled(GraphQLContext graphQlContext) {
public static class BadFaithIntrospectionError implements GraphQLError {
private final String message;

public BadFaithIntrospectionError(String qualifiedField) {
this.message = String.format("This request is not asking for introspection in good faith - %s is present too often!", qualifiedField);
public static BadFaithIntrospectionError tooManyFields(String fieldCoordinate) {
return new BadFaithIntrospectionError(String.format("This request is not asking for introspection in good faith - %s is present too often!", fieldCoordinate));
}

public static BadFaithIntrospectionError tooBigOperation(String message) {
return new BadFaithIntrospectionError(String.format("This request is not asking for introspection in good faith - the query is too big: %s", message));
}

private BadFaithIntrospectionError(String message) {
this.message = message;
}

@Override
Expand All @@ -151,5 +166,12 @@ public ErrorClassification getErrorType() {
public List<SourceLocation> getLocations() {
return null;
}

@Override
public String toString() {
return "BadFaithIntrospectionError{" +
"message='" + message + '\'' +
'}';
}
}
}
17 changes: 8 additions & 9 deletions ...thIntrospectionInstrumentationTest.groovy → ...pection/GoodFaithIntrospectionTest.groovy
View file
Open in desktop
Expand Up @@ -3,13 +3,12 @@ package graphql.introspection
import graphql.ExecutionInput
import graphql.ExecutionResult
import graphql.TestUtil
import graphql.execution.AbortExecutionException
import graphql.execution.CoercedVariables
import graphql.language.Document
import graphql.normalized.ExecutableNormalizedOperationFactory
import spock.lang.Specification

class GoodFaithIntrospectionInstrumentationTest extends Specification {
class GoodFaithIntrospectionTest extends Specification {

def graphql = TestUtil.graphQL("type Query { normalField : String }").build()

Expand Down Expand Up @@ -170,7 +169,7 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification {
def query = createDeepQuery(depth)
def then = System.currentTimeMillis()
ExecutionResult er = graphql.execute(query)
def ms = System.currentTimeMillis()-then
def ms = System.currentTimeMillis() - then

then:
!er.errors.isEmpty()
Expand All @@ -181,12 +180,12 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification {
where:
depth | targetError
2 | GoodFaithIntrospection.BadFaithIntrospectionError.class
10 | AbortExecutionException.class
15 | AbortExecutionException.class
20 | AbortExecutionException.class
25 | AbortExecutionException.class
50 | AbortExecutionException.class
100 | AbortExecutionException.class
10 | GoodFaithIntrospection.BadFaithIntrospectionError.class
15 | GoodFaithIntrospection.BadFaithIntrospectionError.class
20 | GoodFaithIntrospection.BadFaithIntrospectionError.class
25 | GoodFaithIntrospection.BadFaithIntrospectionError.class
50 | GoodFaithIntrospection.BadFaithIntrospectionError.class
100 | GoodFaithIntrospection.BadFaithIntrospectionError.class
}

String createDeepQuery(int depth = 25) {
Expand Down