Skip to content

Commit 0eec91e

Browse files
bbakermanbbakerman
authored
Merge pull request #3551 from jbellenger/jbellenger-validate-dir-args
validate non-nullable directive args
2 parents c4df085 + b906880 commit 0eec91e

2 files changed

Lines changed: 78 additions & 18 deletions

File tree

‎src/main/java/graphql/schema/validation/AppliedDirectiveArgumentsAreValid.java‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
import graphql.execution.ValuesResolver;
77
import graphql.language.Value;
88
import graphql.schema.CoercingParseValueException;
9+
import graphql.schema.GraphQLAppliedDirective;
10+
import graphql.schema.GraphQLAppliedDirectiveArgument;
911
import graphql.schema.GraphQLArgument;
1012
import graphql.schema.GraphQLDirective;
1113
import graphql.schema.GraphQLInputType;
1214
import graphql.schema.GraphQLSchema;
1315
import graphql.schema.GraphQLSchemaElement;
16+
import graphql.schema.GraphQLTypeUtil;
1417
import graphql.schema.GraphQLTypeVisitorStub;
1518
import graphql.schema.InputValueWithState;
1619
import graphql.util.TraversalControl;
@@ -32,29 +35,56 @@ public TraversalControl visitGraphQLDirective(GraphQLDirective directive, Traver
3235
// if there is no parent it means it is just a directive definition and not an applied directive
3336
if (context.getParentNode() != null) {
3437
for (GraphQLArgument graphQLArgument : directive.getArguments()) {
35-
checkArgument(directive, graphQLArgument, context);
38+
checkArgument(
39+
directive.getName(),
40+
graphQLArgument.getName(),
41+
graphQLArgument.getArgumentValue(),
42+
graphQLArgument.getType(),
43+
context
44+
);
3645
}
3746
}
3847
return TraversalControl.CONTINUE;
3948
}
4049

41-
private void checkArgument(GraphQLDirective directive, GraphQLArgument argument, TraverserContext<GraphQLSchemaElement> context) {
42-
if (!argument.hasSetValue()) {
43-
return;
50+
@Override
51+
public TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective directive, TraverserContext<GraphQLSchemaElement> context) {
52+
// if there is no parent it means it is just a directive definition and not an applied directive
53+
if (context.getParentNode() != null) {
54+
for (GraphQLAppliedDirectiveArgument graphQLArgument : directive.getArguments()) {
55+
checkArgument(
56+
directive.getName(),
57+
graphQLArgument.getName(),
58+
graphQLArgument.getArgumentValue(),
59+
graphQLArgument.getType(),
60+
context
61+
);
62+
}
4463
}
64+
return TraversalControl.CONTINUE;
65+
}
66+
67+
private void checkArgument(
68+
String directiveName,
69+
String argumentName,
70+
InputValueWithState argumentValue,
71+
GraphQLInputType argumentType,
72+
TraverserContext<GraphQLSchemaElement> context
73+
) {
4574
GraphQLSchema schema = context.getVarFromParents(GraphQLSchema.class);
4675
SchemaValidationErrorCollector errorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class);
47-
InputValueWithState argumentValue = argument.getArgumentValue();
4876
boolean invalid = false;
4977
if (argumentValue.isLiteral() &&
50-
!validationUtil.isValidLiteralValue((Value<?>) argumentValue.getValue(), argument.getType(), schema, GraphQLContext.getDefault(), Locale.getDefault())) {
78+
!validationUtil.isValidLiteralValue((Value<?>) argumentValue.getValue(), argumentType, schema, GraphQLContext.getDefault(), Locale.getDefault())) {
5179
invalid = true;
5280
} else if (argumentValue.isExternal() &&
53-
!isValidExternalValue(schema, argumentValue.getValue(), argument.getType(), GraphQLContext.getDefault(), Locale.getDefault())) {
81+
!isValidExternalValue(schema, argumentValue.getValue(), argumentType, GraphQLContext.getDefault(), Locale.getDefault())) {
82+
invalid = true;
83+
} else if (argumentValue.isNotSet() && GraphQLTypeUtil.isNonNull(argumentType)) {
5484
invalid = true;
5585
}
5686
if (invalid) {
57-
String message = format("Invalid argument '%s' for applied directive of name '%s'", argument.getName(), directive.getName());
87+
String message = format("Invalid argument '%s' for applied directive of name '%s'", argumentName, directiveName);
5888
errorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.InvalidAppliedDirectiveArgument, message));
5989
}
6090
}

‎src/test/groovy/graphql/schema/GraphQLArgumentTest.groovy‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -196,23 +196,53 @@ class GraphQLArgumentTest extends Specification {
196196
resolvedDefaultValue == null
197197
}
198198

199-
def "Applied schema directives arguments are validated for programmatic schemas"() {
199+
def "schema directive arguments are validated for programmatic schemas"() {
200200
given:
201201
def arg = newArgument().name("arg").type(GraphQLInt).valueProgrammatic(ImmutableKit.emptyMap()).build() // Retain for test coverage
202202
def directive = mkDirective("cached", ARGUMENT_DEFINITION, arg)
203203
def field = newFieldDefinition()
204-
.name("hello")
205-
.type(GraphQLString)
206-
.argument(arg)
207-
.withDirective(directive)
208-
.build()
204+
.name("hello")
205+
.type(GraphQLString)
206+
.argument(arg)
207+
.withDirective(directive)
208+
.build()
209209
when:
210-
newSchema().query(
210+
newSchema()
211+
.query(
211212
newObject()
212-
.name("Query")
213-
.field(field)
214-
.build())
213+
.name("Query")
214+
.field(field)
215+
.build()
216+
)
217+
.additionalDirective(directive)
218+
.build()
219+
then:
220+
def e = thrown(InvalidSchemaException)
221+
e.message.contains("Invalid argument 'arg' for applied directive of name 'cached'")
222+
}
223+
224+
def "applied directive arguments are validated for programmatic schemas"() {
225+
given:
226+
def arg = newArgument()
227+
.name("arg")
228+
.type(GraphQLNonNull.nonNull(GraphQLInt))
215229
.build()
230+
def directive = mkDirective("cached", ARGUMENT_DEFINITION, arg)
231+
def field = newFieldDefinition()
232+
.name("hello")
233+
.type(GraphQLString)
234+
.withAppliedDirective(directive.toAppliedDirective())
235+
.build()
236+
when:
237+
newSchema()
238+
.query(
239+
newObject()
240+
.name("Query")
241+
.field(field)
242+
.build()
243+
)
244+
.additionalDirective(directive)
245+
.build()
216246
then:
217247
def e = thrown(InvalidSchemaException)
218248
e.message.contains("Invalid argument 'arg' for applied directive of name 'cached'")

0 commit comments

Comments
 (0)