graphql: Make sure that undefined arguments are ignored

Author: lutter

graphql/src/execution/query.rs

@@ -106,6 +106,12 @@ impl<'a> std::fmt::Display for SelectedFields<'a> {
 /// A GraphQL query that has been preprocessed and checked and is ready
 /// for execution. Checking includes validating all query fields and, if
 /// desired, checking the query's complexity
+//
+// The implementation contains various workarounds to make it compatible
+// with the previous implementation when it comes to queries that are not
+// fully spec compliant and should be rejected through rigorous validation
+// against the GraphQL spec. Once we do validate queries, code that is
+// marked with `graphql-bug-compat` can be deleted.
 pub struct Query {
     /// The schema against which to execute the query
     pub schema: Arc<ApiSchema>,
@@ -791,6 +797,7 @@ impl Transform {
 
         let resolver = |name: &str| self.schema.get_named_type(name);
 
+        let mut defined_args: usize = 0;
         for argument_def in sast::get_argument_definitions(ty, field_name)
             .into_iter()
             .flatten()
@@ -799,6 +806,9 @@ impl Transform {
                 .iter_mut()
                 .find(|arg| &arg.0 == &argument_def.name)
                 .map(|arg| &mut arg.1);
+            if arg_value.is_some() {
+                defined_args += 1;
+            }
             match coercion::coerce_input_value(
                 arg_value.as_deref().cloned(),
                 &argument_def,
@@ -820,6 +830,18 @@ impl Transform {
             }
         }
 
+        // see: graphql-bug-compat
+        // avoids error 'unknown argument on field'
+        if defined_args < arguments.len() {
+            // `arguments` contains undefined arguments, remove them
+            match sast::get_argument_definitions(ty, field_name) {
+                None => arguments.clear(),
+                Some(arg_defs) => {
+                    arguments.retain(|(name, _)| arg_defs.iter().any(|def| &def.name == name))
+                }
+            }
+        }
+
         if errors.is_empty() {
             Ok(())
         } else {

graphql/tests/query.rs

@@ -1421,6 +1421,37 @@ fn can_use_nested_filter() {
     })
 }
 
+// see: graphql-bug-compat
+#[test]
+fn ignores_invalid_field_arguments() {
+    run_test_sequentially(|store| async move {
+        let deployment = setup(store.as_ref());
+        // This query has to return all the musicians since `id` is not a
+        // valid argument for the `musicians` field and must therefore be
+        // ignored
+        let result = execute_query_document(
+            &deployment.hash,
+            graphql_parser::parse_query("query { musicians(id: \"m1\") { id } } ")
+                .expect("invalid test query")
+                .into_static(),
+        )
+        .await;
+
+        let data = extract_data!(result).unwrap();
+        match data {
+            r::Value::Object(obj) => match obj.get("musicians").unwrap() {
+                r::Value::List(lst) => {
+                    assert_eq!(4, lst.len());
+                }
+                _ => panic!("expected a list of values"),
+            },
+            _ => {
+                panic!("expected an object")
+            }
+        }
+    })
+}
+
 async fn check_musicians_at(
     id: &DeploymentHash,
     query: &str,