From e264cf509dbea310c904d30933b22b1d11c3832c Mon Sep 17 00:00:00 2001 From: ilslv <47687266+ilslv@users.noreply.github.com> Date: Mon, 13 Dec 2021 15:27:14 +0300 Subject: [PATCH] Forbid `__typename` on subscription root (#1001, #1000) --- .../juniper_tests/src/issue_372.rs | 54 +++++++++++++------ juniper/CHANGELOG.md | 1 + juniper/src/types/subscriptions.rs | 11 ---- .../rules/fields_on_correct_type.rs | 48 +++++++++++++++++ 4 files changed, 86 insertions(+), 28 deletions(-) diff --git a/integration_tests/juniper_tests/src/issue_372.rs b/integration_tests/juniper_tests/src/issue_372.rs index 8d765176a..afe86a322 100644 --- 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 index 5c968de4b..914f4b911 100644 --- 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 = ""` attribute's argument renamed to `rename_all = ""`. ([#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 index c8ac53af3..e99a5aafa 100644 --- 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 index c15379aac..ad90cb257 100644 --- 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>, + ) { + // 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)], + )], + ); + } }