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.

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

Fixed issue where empty selection sets lead to execution errors. #2432

Merged
merged 7 commits into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
Expand All @@ -6,6 +7,7 @@
using HotChocolate.Types;
using HotChocolate.Types.Descriptors.Definitions;
using static HotChocolate.Execution.ThrowHelper;
using static HotChocolate.Execution.Properties.Resources;

namespace HotChocolate.Execution.Processing
{
Expand All @@ -30,6 +32,36 @@ public sealed partial class OperationCompiler
ObjectType rootType,
IEnumerable<ISelectionOptimizer>? optimizers = null)
{
if (operationId == null)
{
throw new ArgumentNullException(nameof(operationId));
}

if (document == null)
{
throw new ArgumentNullException(nameof(document));
}

if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

if (schema == null)
{
throw new ArgumentNullException(nameof(schema));
}

if (rootType == null)
{
throw new ArgumentNullException(nameof(rootType));
}

if (operation.SelectionSet.Selections.Count == 0)
{
throw OperationCompiler_NoOperationSelections(operation);
}

var fragments = new FragmentCollection(schema, document);
var compiler = new OperationCompiler(schema, fragments);
var selectionSetLookup = new Dictionary<SelectionSetNode, SelectionVariants>();
Expand Down Expand Up @@ -177,6 +209,13 @@ private void CompleteSelectionSet(CompilerContext context)

if (context.Type.Fields.TryGetField(fieldName, out IObjectField? field))
{
if ((selection.SelectionSet is null ||
selection.SelectionSet.Selections.Count == 0) &&
field.Type.NamedType().IsCompositeType())
{
throw OperationCompiler_NoCompositeSelections(selection);
}

if (context.Fields.TryGetValue(responseName, out Selection? preparedSelection))
{
preparedSelection.AddSelection(selection, includeCondition);
Expand Down Expand Up @@ -226,6 +265,11 @@ private void CompleteSelectionSet(CompilerContext context)
{
FragmentDefinitionNode fragmentDefinition = fragmentInfo.FragmentDefinition!;

if (fragmentDefinition.SelectionSet.Selections.Count == 0)
{
throw OperationCompiler_FragmentNoSelections(fragmentDefinition);
}

if (fragmentSpread.IsDeferrable() &&
AllowFragmentDeferral(context, fragmentSpread, fragmentDefinition))
{
Expand Down Expand Up @@ -255,6 +299,11 @@ private void CompleteSelectionSet(CompilerContext context)
InlineFragmentNode inlineFragment,
SelectionIncludeCondition? includeCondition)
{
if (inlineFragment.SelectionSet.Selections.Count == 0)
{
throw OperationCompiler_FragmentNoSelections(inlineFragment);
}

if (_fragments.GetFragment(context.Type, inlineFragment) is { } fragmentInfo &&
DoesTypeApply(fragmentInfo.TypeCondition, context.Type))
{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/HotChocolate/Core/src/Execution/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,7 @@
<data name="ThrowHelper_OperationExecutionMiddleware_NoBatchDispatcher_Message" xml:space="preserve">
<value>Make sure that you have registered an IBatchDispatcher with your scoped request services.</value>
</data>
<data name="OperationCompiler_Compile_SelectionSetIsEmpty" xml:space="preserve">
<value>The operation selection set is empty.</value>
</data>
</root>
23 changes: 23 additions & 0 deletions src/HotChocolate/Core/src/Execution/ThrowHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,28 @@ internal static class ThrowHelper
public static GraphQLException OperationExecutionMiddleware_NoBatchDispatcher() =>
new GraphQLException(
ThrowHelper_OperationExecutionMiddleware_NoBatchDispatcher_Message);

public static GraphQLException OperationCompiler_FragmentNoSelections(
ISyntaxNode syntaxNode) =>
new GraphQLException(ErrorBuilder.New()
.SetMessage("Fragment selection set is empty.")
.AddLocation(syntaxNode)
.Build());

public static GraphQLException OperationCompiler_NoCompositeSelections(
FieldNode syntaxNode) =>
new GraphQLException(ErrorBuilder.New()
.SetMessage(
"The composite field `{0}` has no selections.",
syntaxNode.Alias?.Value ?? syntaxNode.Name.Value)
.AddLocation(syntaxNode)
.Build());

public static GraphQLException OperationCompiler_NoOperationSelections(
OperationDefinitionNode syntaxNode) =>
new GraphQLException(ErrorBuilder.New()
.SetMessage("The operation has no selections.")
.AddLocation(syntaxNode)
.Build());
}
}
17 changes: 17 additions & 0 deletions src/HotChocolate/Core/src/Validation/ErrorHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,23 @@ internal static class ErrorHelper
.Build();
}

public static IError NoSelectionOnRootType(
this IDocumentValidatorContext context,
OperationDefinitionNode node,
IType fieldType)
{
return ErrorBuilder.New()
.SetMessage(
Resources.ErrorHelper_NoSelectionOnRootType,
node.Name?.Value ?? "Unnamed")
.AddLocation(node)
.SetPath(context.CreateErrorPath())
.SetExtension("operation", node.Name?.Value ?? "Unnamed")
.SetExtension("type", fieldType.Print())
.SpecifiedBy("sec-Field-Selections-on-Objects-Interfaces-and-Unions-Types")
.Build();
}

public static IError FieldIsRequiredButNull(
this IDocumentValidatorContext context,
ISyntaxNode node,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@
<data name="ErrorHelper_NoSelectionOnCompositeField" xml:space="preserve">
<value>`{0}` is an object, interface or union type field. Leaf selections on objects, interfaces, and unions without subfields are disallowed.</value>
</data>
<data name="ErrorHelper_NoSelectionOnRootType" xml:space="preserve">
<value>Operation `{0}` has a empty selection set. Root types without subfields are disallowed.</value>
</data>
<data name="ErrorHelper_FieldIsRequiredButNull" xml:space="preserve">
<value>`{0}` is a required field and cannot be null.</value>
</data>
Expand Down
52 changes: 28 additions & 24 deletions src/HotChocolate/Core/src/Validation/Rules/FieldVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ internal sealed class FieldVisitor : TypeDocumentValidatorVisitor
TryMergeFieldsInSet(context, context.FieldSets[node.SelectionSet]);
}

if (node.SelectionSet.Selections.Count == 0)
{
context.Errors.Add(context.NoSelectionOnRootType(
node,
context.Schema.GetOperationType(node.Operation)));
}

return base.Leave(node, context);
}

Expand All @@ -64,27 +71,28 @@ internal sealed class FieldVisitor : TypeDocumentValidatorVisitor
fields.Add(new FieldInfo(context.Types.Peek(), context.NonNullString, node));
return Skip;
}
else if (context.Types.TryPeek(out IType type) &&

if (context.Types.TryPeek(out IType type) &&
type.NamedType() is IComplexOutputType ct)
{
if (ct.Fields.TryGetField(node.Name.Value, out IOutputField of))
{
fields.Add(new FieldInfo(context.Types.Peek(), of.Type, node));

if (node.SelectionSet is null)
if (node.SelectionSet is null || node.SelectionSet.Selections.Count == 0)
{
if (of.Type.NamedType().IsCompositeType())
{
context.Errors.Add(context.NoSelectionOnCompositeField(
node, ct, of.Type));
context.Errors.Add(
context.NoSelectionOnCompositeField(node, ct, of.Type));
}
}
else
{
if (of.Type.NamedType().IsLeafType())
{
context.Errors.Add(context.LeafFieldsCannotHaveSelections(
node, ct, of.Type));
context.Errors.Add
(context.LeafFieldsCannotHaveSelections(node, ct, of.Type));
return Skip;
}
}
Expand All @@ -93,17 +101,13 @@ internal sealed class FieldVisitor : TypeDocumentValidatorVisitor
context.Types.Push(of.Type);
return Continue;
}
else
{
context.Errors.Add(context.FieldDoesNotExist(node, ct));
return Skip;
}
}
else
{
context.UnexpectedErrorsDetected = true;

context.Errors.Add(context.FieldDoesNotExist(node, ct));
return Skip;
}

context.UnexpectedErrorsDetected = true;
return Skip;
}

protected override ISyntaxVisitorAction Leave(
Expand Down Expand Up @@ -177,7 +181,7 @@ internal sealed class FieldVisitor : TypeDocumentValidatorVisitor

private static bool HasFields(SelectionSetNode selectionSet)
{
for (int i = 0; i < selectionSet.Selections.Count; i++)
for (var i = 0; i < selectionSet.Selections.Count; i++)
{
ISelectionNode selection = selectionSet.Selections[i];
if (selection.Kind == SyntaxKind.Field)
Expand Down Expand Up @@ -210,13 +214,13 @@ private static bool IsTypeNameField(NameString fieldName)
}
else
{
for (int i = 0; i < fields.Count - 1; i++)
for (var i = 0; i < fields.Count - 1; i++)
{
FieldInfo fieldA = fields[i];
for (int j = i + 1; j < fields.Count; j++)
for (var j = i + 1; j < fields.Count; j++)
{
FieldInfo fieldB = fields[j];
if (!object.ReferenceEquals(fieldA.Field, fieldB.Field) &&
if (!ReferenceEquals(fieldA.Field, fieldB.Field) &&
string.Equals(
fieldA.ResponseName,
fieldB.ResponseName,
Expand Down Expand Up @@ -267,7 +271,7 @@ private static bool IsTypeNameField(NameString fieldName)

private static void CopyFieldInfos(IList<FieldInfo> from, IList<FieldInfo> to)
{
for (int i = 0; i < from.Count; i++)
for (var i = 0; i < from.Count; i++)
{
to.Add(from[i]);
}
Expand All @@ -276,7 +280,7 @@ private static void CopyFieldInfos(IList<FieldInfo> from, IList<FieldInfo> to)
private static bool IsParentTypeAligned(FieldInfo fieldA, FieldInfo fieldB)
{
return ReferenceEquals(fieldA.DeclaringType, fieldB.DeclaringType) ||
(!fieldA.DeclaringType.IsObjectType() && !fieldB.DeclaringType.IsObjectType());
!fieldA.DeclaringType.IsObjectType() && !fieldB.DeclaringType.IsObjectType();
}

private static bool AreArgumentsIdentical(FieldNode fieldA, FieldNode fieldB)
Expand All @@ -291,12 +295,12 @@ private static bool AreArgumentsIdentical(FieldNode fieldA, FieldNode fieldB)
return false;
}

int validPairs = 0;
var validPairs = 0;

for (int i = 0; i < fieldA.Arguments.Count; i++)
for (var i = 0; i < fieldA.Arguments.Count; i++)
{
ArgumentNode argumentA = fieldA.Arguments[i];
for (int j = 0; j < fieldB.Arguments.Count; j++)
for (var j = 0; j < fieldB.Arguments.Count; j++)
{
ArgumentNode argumentB = fieldB.Arguments[j];
if (argumentA.Name.Value.Equals(argumentB.Name.Value, StringComparison.Ordinal))
Expand Down