Skip to content
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

[GraphQL October 2021] Forbid __typename on subscription root #1001

Merged
merged 10 commits into from
Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 37 additions & 15 deletions integration_tests/juniper_tests/src/issue_372.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
//! Checks that `__typename` field queries 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,
};

use crate::util::extract_next;

pub struct Query;

#[graphql_object]
Expand Down Expand Up @@ -98,28 +96,52 @@ async fn explicit_mutation_typename() {

#[tokio::test]
async fn subscription_typename() {
use juniper::GraphQLError;

let query = r#"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]
async fn explicit_subscription_typename() {
use juniper::GraphQLError;

let query = r#"subscription 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"),
};
}
1 change: 1 addition & 0 deletions juniper/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` [accordingly to the 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

Expand Down
11 changes: 0 additions & 11 deletions juniper/src/types/subscriptions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use futures::{future, stream};
use serde::Serialize;

use crate::{
Expand Down Expand Up @@ -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(|| {
Expand Down
22 changes: 22 additions & 0 deletions juniper/src/validation/rules/fields_on_correct_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
schema::meta::MetaType,
validation::{ValidatorContext, Visitor},
value::ScalarValue,
Operation, OperationType, Selection,
};

pub struct FieldsOnCorrectType;
Expand All @@ -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" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add tests fully covering this in this module below along with others.

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>,
Expand Down