Skip to content

Updated validation to disallow skip/include on root subscription fields #8276

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,27 @@ public static T SingleOrDefault<T>(this IDirectiveCollection directives)
return default!;
}

internal static bool HasSkipOrIncludeDirective(this IReadOnlyList<DirectiveNode> directives)
{
if (directives.Count == 0)
{
return false;
}

for (var i = 0; i < directives.Count; i++)
{
var directive = directives[i];

if (directive.Name.Value.EqualsOrdinal(WellKnownDirectives.Skip) ||
directive.Name.Value.EqualsOrdinal(WellKnownDirectives.Include))
{
return true;
}
}

return false;
}

internal static IValueNode? SkipValue(this IReadOnlyList<DirectiveNode> directives)
{
var directive = directives.GetSkipDirectiveNode();
Expand Down
8 changes: 8 additions & 0 deletions src/HotChocolate/Core/src/Validation/ErrorHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,14 @@ public static IError OneOfVariablesMustBeNonNull(
.SpecifiedBy("sec-Oneof–Input-Objects-Have-Exactly-One-Field", rfc: 825)
.Build();

public static IError SkipAndIncludeNotAllowedOnSubscriptionRootField(
ISelectionNode selection)
=> ErrorBuilder.New()
.SetMessage(Resources.ErrorHelper_SkipAndIncludeNotAllowedOnSubscriptionRootField)
.SetLocations([selection])
.SpecifiedBy("sec-Single-Root-Field", rfc: 860)
.Build();

public static IError DeferAndStreamNotAllowedOnMutationOrSubscriptionRoot(
ISelectionNode selection)
=> ErrorBuilder.New()
Expand Down
342 changes: 250 additions & 92 deletions src/HotChocolate/Core/src/Validation/Properties/Resources.Designer.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@
<data name="ErrorHelper_OneOfVariablesMustBeNonNull" xml:space="preserve">
<value>The variable `${0}` assigned to the field `{1}` of the Oneof Input Object `{2}` must be non-null.</value>
</data>
<data name="ErrorHelper_SkipAndIncludeNotAllowedOnSubscriptionRootField" xml:space="preserve">
<value>The skip and include directives are not allowed to be used on root fields of the subscription type.</value>
</data>
<data name="ErrorHelper_DeferAndStreamNotAllowedOnMutationOrSubscriptionRoot" xml:space="preserve">
<value>The defer and stream directives are not allowed to be used on root fields of the mutation or subscription type.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ protected override ISyntaxVisitorAction Enter(
{
context.Names.Add((node.Alias ?? node.Name).Value);

if (context.OperationType is OperationType.Subscription &&
node.Directives.HasSkipOrIncludeDirective())
{
context.ReportError(SkipAndIncludeNotAllowedOnSubscriptionRootField(node));
}

if (context.OperationType is OperationType.Mutation or OperationType.Subscription &&
node.Directives.HasStreamOrDeferDirective())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,124 @@ fragment multipleSubscriptions on Subscription {
"have exactly one root field.", t.Message));
}

[Fact]
public void DisallowedSkipDirectiveOnRootField()
{
ExpectErrors(@"
subscription requiredRuntimeValidation($bool: Boolean!) {
newMessage @skip(if: $bool) {
body
sender
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedIncludeDirectiveOnRootField()
{
ExpectErrors(@"
subscription requiredRuntimeValidation($bool: Boolean!) {
newMessage @include(if: $bool) {
body
sender
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedSkipDirectiveOnRootFieldWithinFragment()
{
// arrange
ExpectErrors(@"
subscription sub {
...newMessageFields
}

fragment newMessageFields on Subscription {
newMessage @skip(if: true) {
body
sender
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedIncludeDirectiveOnRootFieldWithinFragment()
{
// arrange
ExpectErrors(@"
subscription sub {
...newMessageFields
}

fragment newMessageFields on Subscription {
newMessage @include(if: true) {
body
sender
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedSkipDirectiveOnRootFieldWithinInlineFragment()
{
// arrange
ExpectErrors(@"
subscription sub {
...on Subscription {
newMessage @skip(if: true) {
body
sender
}
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedIncludeDirectiveOnRootFieldWithinInlineFragment()
{
// arrange
ExpectErrors(@"
subscription sub {
...on Subscription {
newMessage @include(if: true) {
body
sender
}
}
}
",
t => Assert.Equal(
"The skip and include directives are not allowed to be used on root fields of " +
"the subscription type.",
t.Message));
}

[Fact]
public void DisallowedIntrospectionField()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 3,
"Column": 21
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 7,
"Column": 21
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 4,
"Column": 25
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 3,
"Column": 21
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 7,
"Column": 21
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"Message": "The skip and include directives are not allowed to be used on root fields of the subscription type.",
"Code": null,
"Path": null,
"Locations": [
{
"Line": 4,
"Column": 25
}
],
"Extensions": {
"specifiedBy": "https://spec.graphql.org/draft/#sec-Single-Root-Field",
"rfc": "https://github.com/graphql/graphql-spec/pull/860"
},
"Exception": null
}
]
Loading