Forbid __typename on subscription root (#1001, #1000)

graphql-rust · Dec 13, 2021 · e264cf5 · e264cf5
1 parent 09da50b
commit e264cf5
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 28 deletions.
diff --git a/integration_tests/juniper_tests/src/issue_372.rs b/integration_tests/juniper_tests/src/issue_372.rs
@@ -1,14 +1,12 @@
-//! Checks that `__typename` field queries okay on root types.
+//! Checks that `__typename` field queries okay (and not okay) on root types.
 //! See [#372](https://github.com/graphql-rust/juniper/issues/372) for details.
 
-use futures::{stream, FutureExt as _};
+use futures::stream;
 use juniper::{
     execute, graphql_object, graphql_subscription, graphql_value, graphql_vars,
-    resolve_into_stream, RootNode,
+    resolve_into_stream, GraphQLError, RootNode,
 };
 
-use crate::util::extract_next;
-
 pub struct Query;
 
 #[graphql_object]
@@ -102,12 +100,23 @@ async fn subscription_typename() {
 
     let schema = RootNode::new(Query, Mutation, Subscription);
 
-    assert_eq!(
-        resolve_into_stream(query, None, &schema, &graphql_vars! {}, &())
-            .then(|s| extract_next(s))
-            .await,
-        Ok((graphql_value!({"__typename": "Subscription"}), vec![])),
-    );
+    match resolve_into_stream(query, None, &schema, &graphql_vars! {}, &()).await {
+        Err(GraphQLError::ValidationError(mut errors)) => {
+            assert_eq!(errors.len(), 1);
+
+            let err = errors.pop().unwrap();
+
+            assert_eq!(
+                err.message(),
+                "`__typename` may not be included as a root field in a \
+                 subscription operation",
+            );
+            assert_eq!(err.locations()[0].index(), 15);
+            assert_eq!(err.locations()[0].line(), 0);
+            assert_eq!(err.locations()[0].column(), 15);
+        }
+        _ => panic!("Expected ValidationError"),
+    };
 }
 
 #[tokio::test]
@@ -116,10 +125,21 @@ async fn explicit_subscription_typename() {
 
     let schema = RootNode::new(Query, Mutation, Subscription);
 
-    assert_eq!(
-        resolve_into_stream(query, None, &schema, &graphql_vars! {}, &())
-            .then(|s| extract_next(s))
-            .await,
-        Ok((graphql_value!({"__typename": "Subscription"}), vec![])),
-    );
+    match resolve_into_stream(query, None, &schema, &graphql_vars! {}, &()).await {
+        Err(GraphQLError::ValidationError(mut errors)) => {
+            assert_eq!(errors.len(), 1);
+
+            let err = errors.pop().unwrap();
+
+            assert_eq!(
+                err.message(),
+                "`__typename` may not be included as a root field in a \
+                 subscription operation"
+            );
+            assert_eq!(err.locations()[0].index(), 28);
+            assert_eq!(err.locations()[0].line(), 0);
+            assert_eq!(err.locations()[0].column(), 28);
+        }
+        _ => panic!("Expected ValidationError"),
+    };
 }
diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md
@@ -7,6 +7,7 @@
 - `#[graphql_object]` and `#[graphql_subscription]` macros expansion now preserves defined `impl` blocks "as is" and reuses defined methods in opaque way. ([#971](https://github.com/graphql-rust/juniper/pull/971))
 - `rename = "<policy>"` attribute's argument renamed to `rename_all = "<policy>"`. ([#971](https://github.com/graphql-rust/juniper/pull/971))
 - Upgrade `bson` feature to [2.0 version of its crate](https://github.com/mongodb/bson-rust/releases/tag/v2.0.0). ([#979](https://github.com/graphql-rust/juniper/pull/979))
+- Forbid `__typename` field on `subscription` operations [accordingly to October 2021 spec](https://spec.graphql.org/October2021/#note-bc213). ([#1001](https://github.com/graphql-rust/juniper/pull/1001), [#1000](https://github.com/graphql-rust/juniper/pull/1000))
 
 ## Features
 

diff --git a/juniper/src/types/subscriptions.rs b/juniper/src/types/subscriptions.rs
@@ -1,4 +1,3 @@
-use futures::{future, stream};
 use serde::Serialize;
 
 use crate::{
@@ -293,16 +292,6 @@ where
 
                 let response_name = f.alias.as_ref().unwrap_or(&f.name).item;
 
-                if f.name.item == "__typename" {
-                    let typename =
-                        Value::scalar(instance.concrete_type_name(executor.context(), info));
-                    object.add_field(
-                        response_name,
-                        Value::Scalar(Box::pin(stream::once(future::ok(typename)))),
-                    );
-                    continue;
-                }
-
                 let meta_field = meta_type
                     .field_by_name(f.name.item)
                     .unwrap_or_else(|| {

diff --git a/juniper/src/validation/rules/fields_on_correct_type.rs b/juniper/src/validation/rules/fields_on_correct_type.rs
@@ -4,6 +4,7 @@ use crate::{
     schema::meta::MetaType,
     validation::{ValidatorContext, Visitor},
     value::ScalarValue,
+    Operation, OperationType, Selection,
 };
 
 pub struct FieldsOnCorrectType;
@@ -16,6 +17,27 @@ impl<'a, S> Visitor<'a, S> for FieldsOnCorrectType
 where
     S: ScalarValue,
 {
+    fn enter_operation_definition(
+        &mut self,
+        context: &mut ValidatorContext<'a, S>,
+        operation: &'a Spanning<Operation<S>>,
+    ) {
+        // https://spec.graphql.org/October2021/#note-bc213
+        if let OperationType::Subscription = operation.item.operation_type {
+            for selection in &operation.item.selection_set {
+                if let Selection::Field(field) = selection {
+                    if field.item.name.item == "__typename" {
+                        context.report_error(
+                            "`__typename` may not be included as a root \
+                             field in a subscription operation",
+                            &[field.item.name.start],
+                        );
+                    }
+                }
+            }
+        }
+    }
+
     fn enter_field(
         &mut self,
         context: &mut ValidatorContext<'a, S>,
@@ -357,4 +379,30 @@ mod tests {
         "#,
         );
     }
+
+    #[test]
+    fn forbids_typename_on_subscription() {
+        expect_fails_rule::<_, _, DefaultScalarValue>(
+            factory,
+            r#"subscription { __typename }"#,
+            &[RuleError::new(
+                "`__typename` may not be included as a root field in a \
+                 subscription operation",
+                &[SourcePosition::new(15, 0, 15)],
+            )],
+        );
+    }
+
+    #[test]
+    fn forbids_typename_on_explicit_subscription() {
+        expect_fails_rule::<_, _, DefaultScalarValue>(
+            factory,
+            r#"subscription SubscriptionRoot { __typename }"#,
+            &[RuleError::new(
+                "`__typename` may not be included as a root field in a \
+                 subscription operation",
+                &[SourcePosition::new(32, 0, 32)],
+            )],
+        );
+    }
 }