Skip to content

Add max depth option to ExecutableNormalizedOperationFactory #3268

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

Merged
dondonz merged 4 commits into from
Jul 12, 2023
Merged

Add max depth option to ExecutableNormalizedOperationFactory #3268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c15da42
Add max depth option to ExecutableNormalizedOperationFactory
gnawf Jul 12, 2023
b704f0c
Add extra check on message
gnawf Jul 12, 2023
9fca81e
Add extra tests
gnawf Jul 12, 2023
d1dd724
Add docs
gnawf Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 134 additions & 26 deletions src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import graphql.GraphQLContext;
import graphql.PublicApi;
import graphql.collect.ImmutableKit;
import graphql.execution.AbortExecutionException;
import graphql.execution.CoercedVariables;
import graphql.execution.MergedField;
import graphql.execution.RawVariables;
Expand Down Expand Up @@ -64,6 +65,85 @@
*/
@PublicApi
public class ExecutableNormalizedOperationFactory {
public static class Options {
Copy link
Contributor Author

@gnawf gnawf Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this new Options class which mimicks the Options pattern seen in the repo.

This stores things not directly related to the operation info.

private final GraphQLContext graphQLContext;
private final Locale locale;
private final int maxChildrenDepth;

private Options(GraphQLContext graphQLContext,
Locale locale,
int maxChildrenDepth) {
this.graphQLContext = graphQLContext;
this.locale = locale;
this.maxChildrenDepth = maxChildrenDepth;
}

public static Options defaultOptions() {
return new Options(
GraphQLContext.getDefault(),
Locale.getDefault(),
Integer.MAX_VALUE);
}

/**
* Locale to use when parsing the query.
* <p>
* e.g. can be passed to {@link graphql.schema.Coercing} for parsing.
*
* @param locale the locale to use
* @return new options object to use
*/
public Options locale(Locale locale) {
return new Options(this.graphQLContext, locale, this.maxChildrenDepth);
}

/**
* Context object to use when parsing the operation.
* <p>
* Can be used to intercept input values e.g. using {@link graphql.execution.values.InputInterceptor}.
*
* @param graphQLContext the context to use
* @return new options object to use
*/
public Options graphQLContext(GraphQLContext graphQLContext) {
return new Options(graphQLContext, this.locale, this.maxChildrenDepth);
}

/**
* Controls the maximum depth of the operation. Can be used to prevent
* against malicious operations.
*
* @param maxChildrenDepth the max depth
* @return new options object to use
*/
public Options maxChildrenDepth(int maxChildrenDepth) {
return new Options(this.graphQLContext, this.locale, maxChildrenDepth);
}

/**
* @return context to use during operation parsing
* @see #graphQLContext(GraphQLContext)
*/
public GraphQLContext getGraphQLContext() {
return graphQLContext;
}

/**
* @return locale to use during operation parsing
* @see #locale(Locale)
*/
public Locale getLocale() {
return locale;
}

/**
* @return maximum children depth before aborting parsing
* @see #maxChildrenDepth(int)
*/
public int getMaxChildrenDepth() {
return maxChildrenDepth;
}
}
Copy link
Member

@bbakerman bbakerman Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

but can we have some javadoc on what they do please since its API

Copy link
Contributor Author

@gnawf gnawf Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 😄


private final ConditionalNodes conditionalNodes = new ConditionalNodes();

Expand All @@ -90,8 +170,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperation(
getOperationResult.fragmentsByName,
coercedVariableValues,
null,
GraphQLContext.getDefault(),
Locale.getDefault());
Options.defaultOptions());
}

/**
Expand All @@ -114,8 +193,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperation(
fragments,
coercedVariableValues,
null,
GraphQLContext.getDefault(),
Locale.getDefault());
Options.defaultOptions());
}

/**
Expand All @@ -137,8 +215,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW
document,
operationName,
rawVariables,
GraphQLContext.getDefault(),
Locale.getDefault());
Options.defaultOptions());
}


Expand All @@ -163,40 +240,64 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW
GraphQLContext graphQLContext,
Locale locale
) {
return createExecutableNormalizedOperationWithRawVariables(
graphQLSchema,
document,
operationName,
rawVariables,
Options.defaultOptions().graphQLContext(graphQLContext).locale(locale));
}


/**
* This will create a runtime representation of the graphql operation that would be executed
* in a runtime sense.
*
* @param graphQLSchema the schema to be used
* @param document the {@link Document} holding the operation text
* @param operationName the operation name to use
* @param rawVariables the raw variables that have not yet been coerced
* @param options the {@link Options} to use for parsing
*
* @return a runtime representation of the graphql operation.
*/
public static ExecutableNormalizedOperation createExecutableNormalizedOperationWithRawVariables(GraphQLSchema graphQLSchema,
Document document,
String operationName,
RawVariables rawVariables,
Options options) {
NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operationName);

return new ExecutableNormalizedOperationFactory().createExecutableNormalizedOperationImplWithRawVariables(graphQLSchema,
getOperationResult.operationDefinition,
getOperationResult.fragmentsByName,
rawVariables,
graphQLContext,
locale);
options
);
}

private ExecutableNormalizedOperation createExecutableNormalizedOperationImplWithRawVariables(GraphQLSchema graphQLSchema,
OperationDefinition operationDefinition,
Map<String, FragmentDefinition> fragments,
RawVariables rawVariables,
GraphQLContext graphQLContext,
Locale locale) {

Options options) {
List<VariableDefinition> variableDefinitions = operationDefinition.getVariableDefinitions();
CoercedVariables coercedVariableValues = ValuesResolver.coerceVariableValues(graphQLSchema,
variableDefinitions,
rawVariables,
graphQLContext,
locale);
options.getGraphQLContext(),
options.getLocale());
Map<String, NormalizedInputValue> normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema,
variableDefinitions,
rawVariables,
graphQLContext,
locale);
options.getGraphQLContext(),
options.getLocale());
return createNormalizedQueryImpl(graphQLSchema,
operationDefinition,
fragments,
coercedVariableValues,
normalizedVariableValues,
graphQLContext,
locale);
options);
}

/**
Expand All @@ -207,7 +308,7 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr
Map<String, FragmentDefinition> fragments,
CoercedVariables coercedVariableValues,
@Nullable Map<String, NormalizedInputValue> normalizedVariableValues,
GraphQLContext graphQLContext, Locale locale) {
Options options) {
FieldCollectorNormalizedQueryParams parameters = FieldCollectorNormalizedQueryParams
.newParameters()
.fragments(fragments)
Expand All @@ -226,8 +327,8 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr
ImmutableListMultimap.Builder<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields = ImmutableListMultimap.builder();

BiConsumer<ExecutableNormalizedField, MergedField> captureMergedField = (enf, mergedFld) -> {
//QueryDirectivesImpl is a lazy object and only computes itself when asked for
QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, coercedVariableValues.toMap(), graphQLContext, locale);
// QueryDirectivesImpl is a lazy object and only computes itself when asked for
QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, coercedVariableValues.toMap(), options.getGraphQLContext(), options.getLocale());
normalizedFieldToQueryDirectives.put(enf, queryDirectives);
normalizedFieldToMergedField.put(enf, mergedFld);
};
Expand All @@ -241,16 +342,17 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr
updateFieldToNFMap(topLevel, fieldAndAstParents, fieldToNormalizedField);
updateCoordinatedToNFMap(coordinatesToNormalizedFields, topLevel);

buildFieldWithChildren(topLevel,
buildFieldWithChildren(
topLevel,
fieldAndAstParents,
parameters,
fieldToNormalizedField,
captureMergedField,
coordinatesToNormalizedFields,
1);

1,
options.getMaxChildrenDepth());
}
for (FieldCollectorNormalizedQueryParams.PossibleMerger possibleMerger : parameters.possibleMergerList) {
for (FieldCollectorNormalizedQueryParams.PossibleMerger possibleMerger : parameters.getPossibleMergerList()) {
List<ExecutableNormalizedField> childrenWithSameResultKey = possibleMerger.parent.getChildrenWithSameResultKey(possibleMerger.resultKey);
ENFMerger.merge(possibleMerger.parent, childrenWithSameResultKey, graphQLSchema);
}
Expand All @@ -272,7 +374,12 @@ private void buildFieldWithChildren(ExecutableNormalizedField executableNormaliz
ImmutableListMultimap.Builder<Field, ExecutableNormalizedField> fieldNormalizedField,
BiConsumer<ExecutableNormalizedField, MergedField> captureMergedField,
ImmutableListMultimap.Builder<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields,
int curLevel) {
int curLevel,
int maxLevel) {
if (curLevel > maxLevel) {
throw new AbortExecutionException("Maximum query depth exceeded " + curLevel + " > " + maxLevel);
Copy link
Contributor Author

@gnawf gnawf Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied this from MaxQueryDepthInstrumentation but let me know if I should make a more specific exception class.

Copy link
Member

@bbakerman bbakerman Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense - this is our stop exception class

}

CollectNFResult nextLevel = collectFromMergedField(fieldCollectorNormalizedQueryParams, executableNormalizedField, fieldAndAstParents, curLevel + 1);

for (ExecutableNormalizedField childENF : nextLevel.children) {
Expand All @@ -291,7 +398,8 @@ private void buildFieldWithChildren(ExecutableNormalizedField executableNormaliz
fieldNormalizedField,
captureMergedField,
coordinatesToNormalizedFields,
curLevel + 1);
curLevel + 1,
maxLevel);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/main/java/graphql/normalized/FieldCollectorNormalizedQueryParams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class FieldCollectorNormalizedQueryParams {
private final GraphQLContext graphQLContext;
private final Locale locale;

public List<PossibleMerger> possibleMergerList = new ArrayList<>();
private final List<PossibleMerger> possibleMergerList = new ArrayList<>();
Copy link
Contributor Author

@gnawf gnawf Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason for it to be public or not-final. The class is marked as@Internal so shouldn't break anything.


public static class PossibleMerger {
ExecutableNormalizedField parent;
Expand All @@ -39,6 +39,10 @@ public void addPossibleMergers(ExecutableNormalizedField parent, String resultKe
possibleMergerList.add(new PossibleMerger(parent, resultKey));
}

public List<PossibleMerger> getPossibleMergerList() {
return possibleMergerList;
}

Copy link
Member

@bbakerman bbakerman Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one

public GraphQLSchema getGraphQLSchema() {
return graphQLSchema;
}
Expand Down
Loading