From 27408f88d167b177cc8007adf32277f0d3b41339 Mon Sep 17 00:00:00 2001
From: Benjie Gillam <benjie@jemjie.com>
Date: Sun, 25 Apr 2021 12:00:11 +0100
Subject: [PATCH] Prevent @skip and @include on root subscription selection set

---
 spec/Section 5 -- Validation.md | 32 ++++++++++++++++++++++++--------
 spec/Section 6 -- Execution.md  | 23 +++++++++++++++++------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md
index c33157705..724abc7f2 100644
--- a/spec/Section 5 -- Validation.md	
+++ b/spec/Section 5 -- Validation.md	
@@ -249,19 +249,22 @@ query getName {
 
 **Formal Specification**
 
-* For each subscription operation definition {subscription} in the document
-* Let {subscriptionType} be the root Subscription type in {schema}.
-* Let {selectionSet} be the top level selection set on {subscription}.
-* Let {variableValues} be the empty set.
-* Let {groupedFieldSet} be the result of
-  {CollectFields(subscriptionType, selectionSet, variableValues)}.
-* {groupedFieldSet} must have exactly one entry, which must not be an
-  introspection field.
+* For each subscription operation definition {subscription} in the document:
+  * Let {subscriptionType} be the root Subscription type in {schema}.
+  * Let {selectionSet} be the top level selection set on {subscription}.
+  * Let {groupedFieldSet} be the result of
+    {CollectFields(subscriptionType, selectionSet, null)}.
+  * {groupedFieldSet} must have exactly one entry, which must not be an
+    introspection field.
 
 **Explanatory Text**
 
 Subscription operations must have exactly one root field.
 
+To enable us to determine this without access to runtime variables, we must
+forbid the `@skip` and `@include` directives in the root selection set (by
+passing `null` as the `variableValues` argument to {CollectFields()}).
+
 Valid examples:
 
 ```graphql example
@@ -312,6 +315,19 @@ fragment multipleSubscriptions on Subscription {
 }
 ```
 
+We do not allow the `@skip` and `@include` directives at the root of the
+subscription operation. The following example is also invalid:
+
+```graphql counter-example
+subscription requiredRuntimeValidation($bool: Boolean!) {
+  newMessage @include(if: $bool) {
+    body
+    sender
+  }
+  disallowedSecondRootField @skip(if: $bool)
+}
+```
+
 The root field of a subscription operation must not be an introspection field.
 The following example is also invalid:
 
diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md
index ace837f2e..ed62666e2 100644
--- a/spec/Section 6 -- Execution.md	
+++ b/spec/Section 6 -- Execution.md	
@@ -479,17 +479,28 @@ The depth-first-search order of the field groups produced by {CollectFields()}
 is maintained through execution, ensuring that fields appear in the executed
 response in a stable and predictable order.
 
+When {CollectFields()} is used during validation (see for example the
+[single root field](#sec-Single-root-field) subscription operation validation
+rule), the runtime value for {variableValues} will not be available - in this
+case we set {variableValues} to {null} and forbid the use of the `@skip` and
+`@include` directives. During execution, {variableValues} will always be
+non-null.
+
 CollectFields(objectType, selectionSet, variableValues, visitedFragments):
 
   * If {visitedFragments} is not provided, initialize it to the empty set.
   * Initialize {groupedFields} to an empty ordered map of lists.
   * For each {selection} in {selectionSet}:
-    * If {selection} provides the directive `@skip`, let {skipDirective} be that directive.
-      * If {skipDirective}'s {if} argument is {true} or is a variable in {variableValues} with the value {true}, continue with the next
-      {selection} in {selectionSet}.
-    * If {selection} provides the directive `@include`, let {includeDirective} be that directive.
-      * If {includeDirective}'s {if} argument is not {true} and is not a variable in {variableValues} with the value {true}, continue with the next
-      {selection} in {selectionSet}.
+    * If {variableValues} is {null}:
+      * {selection} must not provide the `@skip` directive.
+      * {selection} must not provide the `@include` directive.
+    * Otherwise:
+      * If {selection} provides the directive `@skip`, let {skipDirective} be that directive.
+        * If {skipDirective}'s {if} argument is {true} or is a variable in {variableValues} with the value {true}, continue with the next
+        {selection} in {selectionSet}.
+      * If {selection} provides the directive `@include`, let {includeDirective} be that directive.
+        * If {includeDirective}'s {if} argument is not {true} and is not a variable in {variableValues} with the value {true}, continue with the next
+        {selection} in {selectionSet}.
     * If {selection} is a {Field}:
       * Let {responseKey} be the response key of {selection} (the alias if defined, otherwise the field name).
       * Let {groupForResponseKey} be the list in {groupedFields} for