[GraphQL October 2021] Forbid __typename on subscription root

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,22 @@ 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 error = errors.pop().unwrap();
+            assert_eq!(
+                error.message(),
+                "`__typename` may not be included as a root field in a \
+                 subscription operation",
+            );
+            assert_eq!(error.locations()[0].index(), 15);
+            assert_eq!(error.locations()[0].line(), 0);
+            assert_eq!(error.locations()[0].column(), 15);
+        }
+        _ => panic!("Expected ValidationError"),
+    };
 }
 
 #[tokio::test]
@@ -116,10 +124,20 @@ 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 error = errors.pop().unwrap();
+            assert_eq!(
+                error.message(),
+                "`__typename` may not be included as a root field in a \
+                 subscription operation"
+            );
+            assert_eq!(error.locations()[0].index(), 28);
+            assert_eq!(error.locations()[0].line(), 0);
+            assert_eq!(error.locations()[0].column(), 28);
+        }
+        _ => panic!("Expected ValidationError"),
+    };
 }

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 query 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
 

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(|| {

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,28 @@ mod tests {
         "#,
         );
     }
+
+    #[test]
+    fn 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 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)],
+            )],
+        );
+    }
 }

juniper_actix/Cargo.toml

@@ -13,10 +13,10 @@ subscriptions = ["juniper_graphql_ws", "tokio"]
 
 [dependencies]
 actix = "0.12"
-actix-http = "3.0.0-beta.13"
+actix-http = "3.0.0-beta.15"
 http = "0.2.4"
-actix-web = "4.0.0-beta.12"
-actix-web-actors = "4.0.0-beta.7"
+actix-web = "4.0.0-beta.14"
+actix-web-actors = "4.0.0-beta.8"
 
 juniper = { version = "0.15.7", path = "../juniper", default-features = false }
 juniper_graphql_ws = { version = "0.3.0", path = "../juniper_graphql_ws", optional = true }
@@ -30,11 +30,11 @@ tokio = { version = "1.0", features = ["sync"], optional = true }
 
 [dev-dependencies]
 actix-rt = "2"
-actix-cors = "0.6.0-beta.4"
-actix-identity = "0.4.0-beta.4"
+actix-cors = "0.6.0-beta.6"
+actix-identity = "0.4.0-beta.5"
 tokio = "1.0"
 async-stream = "0.3"
-actix-test = "0.1.0-beta.6"
+actix-test = "0.1.0-beta.8"
 
 juniper = { version = "0.15.7", path = "../juniper", features = ["expose-test-schema"] }
 

juniper_actix/src/lib.rs

@@ -466,7 +466,9 @@ pub mod subscriptions {
 
 #[cfg(test)]
 mod tests {
-    use actix_http::body::AnyBody;
+    use std::pin::Pin;
+
+    use actix_http::body::MessageBody;
     use actix_web::{
         dev::ServiceResponse,
         http,
@@ -475,6 +477,7 @@ mod tests {
         web::Data,
         App,
     };
+    use futures::future;
     use juniper::{
         http::tests::{run_http_test_suite, HttpIntegration, TestResponse},
         tests::fixtures::starwars::schema::{Database, Query},
@@ -487,11 +490,16 @@ mod tests {
     type Schema =
         juniper::RootNode<'static, Query, EmptyMutation<Database>, EmptySubscription<Database>>;
 
-    async fn take_response_body_string(resp: &mut ServiceResponse) -> String {
-        match resp.response().body() {
-            AnyBody::Bytes(body) => String::from_utf8(body.to_vec()).unwrap(),
-            _ => String::from(""),
-        }
+    async fn take_response_body_string(resp: ServiceResponse) -> String {
+        let mut body = resp.into_body();
+        String::from_utf8(
+            future::poll_fn(|cx| Pin::new(&mut body).poll_next(cx))
+                .await
+                .unwrap()
+                .unwrap()
+                .to_vec(),
+        )
+        .unwrap()
     }
 
     async fn index(
@@ -537,13 +545,13 @@ mod tests {
             .append_header((ACCEPT, "text/html"))
             .to_request();
 
-        let mut resp = test::call_service(&mut app, req).await;
-        let body = take_response_body_string(&mut resp).await;
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), http::StatusCode::OK);
         assert_eq!(
             resp.headers().get(CONTENT_TYPE).unwrap().to_str().unwrap(),
             "text/html; charset=utf-8"
         );
+        let body = take_response_body_string(resp).await;
         assert!(body.contains("<script>var GRAPHQL_URL = '/dogs-api/graphql';</script>"));
         assert!(body.contains(
             "<script>var GRAPHQL_SUBSCRIPTIONS_URL = '/dogs-api/subscriptions';</script>"
@@ -578,13 +586,13 @@ mod tests {
             .append_header((ACCEPT, "text/html"))
             .to_request();
 
-        let mut resp = test::call_service(&mut app, req).await;
-        let body = take_response_body_string(&mut resp).await;
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), http::StatusCode::OK);
         assert_eq!(
             resp.headers().get(CONTENT_TYPE).unwrap().to_str().unwrap(),
             "text/html; charset=utf-8"
         );
+        let body = take_response_body_string(resp).await;
         assert!(body.contains("GraphQLPlayground.init(root, { endpoint: '/dogs-api/graphql', subscriptionEndpoint: '/dogs-api/subscriptions' })"));
     }
 
@@ -611,17 +619,17 @@ mod tests {
         )
         .await;
 
-        let mut resp = test::call_service(&mut app, req).await;
-        dbg!(take_response_body_string(&mut resp).await);
+        let resp = test::call_service(&mut app, req).await;
         assert_eq!(resp.status(), http::StatusCode::OK);
-        assert_eq!(
-            take_response_body_string(&mut resp).await,
-            r#"{"data":{"hero":{"name":"R2-D2"}}}"#
-        );
         assert_eq!(
             resp.headers().get("content-type").unwrap(),
             "application/json",
         );
+
+        assert_eq!(
+            take_response_body_string(resp).await,
+            r#"{"data":{"hero":{"name":"R2-D2"}}}"#
+        );
     }
 
     #[actix_web::rt::test]
@@ -644,17 +652,18 @@ mod tests {
         )
         .await;
 
-        let mut resp = test::call_service(&mut app, req).await;
+        let resp = test::call_service(&mut app, req).await;
 
         assert_eq!(resp.status(), http::StatusCode::OK);
-        assert_eq!(
-            take_response_body_string(&mut resp).await,
-            r#"{"data":{"hero":{"name":"R2-D2"}}}"#
-        );
         assert_eq!(
             resp.headers().get("content-type").unwrap(),
             "application/json",
         );
+
+        assert_eq!(
+            take_response_body_string(resp).await,
+            r#"{"data":{"hero":{"name":"R2-D2"}}}"#
+        );
     }
 
     #[actix_web::rt::test]
@@ -688,17 +697,17 @@ mod tests {
         )
         .await;
 
-        let mut resp = test::call_service(&mut app, req).await;
+        let resp = test::call_service(&mut app, req).await;
 
         assert_eq!(resp.status(), http::StatusCode::OK);
-        assert_eq!(
-            take_response_body_string(&mut resp).await,
-            r#"[{"data":{"hero":{"name":"R2-D2"}}},{"data":{"hero":{"id":"1000","name":"Luke Skywalker"}}}]"#
-        );
         assert_eq!(
             resp.headers().get("content-type").unwrap(),
             "application/json",
         );
+        assert_eq!(
+            take_response_body_string(resp).await,
+            r#"[{"data":{"hero":{"name":"R2-D2"}}},{"data":{"hero":{"id":"1000","name":"Luke Skywalker"}}}]"#
+        );
     }
 
     #[test]
@@ -757,14 +766,20 @@ mod tests {
         }
     }
 
-    async fn make_test_response(mut resp: ServiceResponse) -> TestResponse {
-        let body = take_response_body_string(&mut resp).await;
+    async fn make_test_response(resp: ServiceResponse) -> TestResponse {
         let status_code = resp.status().as_u16();
-        let content_type = resp.headers().get(CONTENT_TYPE).unwrap();
+        let content_type = resp
+            .headers()
+            .get(CONTENT_TYPE)
+            .unwrap()
+            .to_str()
+            .unwrap()
+            .to_string();
+        let body = take_response_body_string(resp).await;
         TestResponse {
             status_code: status_code as i32,
             body: Some(body),
-            content_type: content_type.to_str().unwrap().to_string(),
+            content_type,
         }
     }