Repository: graphql-rust/juniper (language: rust)

Issue:
Literal Scalar Parsing should not panic
**Describe the bug**
Literal Scalar Parsing should not panic. Sending badly shaped data is a thing that Denial-Of-Service attackers like to do -- and being able to panic the thread this easy is a smell in my opinion.

**To Reproduce**
Steps to reproduce the behavior:
1. Define a schema with a scalar String, Int, or Float type
2. Query the schema with query literals which violate the contract in step (1)
3. Notice that the thread panics
4. Code reference here: https://github.com/graphql-rust/juniper/blob/master/juniper/src/parser/value.rs ... look for `parse_scalar_literal_by_infered_type` and then look for the panics.

**Expected behavior**
The framework should handle this class of error gracefully without a panic. It should return in this case a proper `400 Bad Request` HTTP Response with the standard graphql json error reporting array format: `{ "errors": [ "... must be an Int" ] }`

**Additional context**
Violating the api shape is one of the easiest things a would-be denial-of-service attacker can try to attempt to crash your service and/or waste cpu cycles/compute resources/network resources.


Discussion / hints:
We should probably add this case to `ParseError` and bubble it up.
@dreamcodez  I am trying to reproduce panic [here](https://github.com/andy128k/juniper/pull/1/files) but I cannot. Could you provide more details?
sure. sorry i have been busy shlepping my stuff from place to place.

I am going to create a small repo and post it here. give me an hour or so

I haven't had time to understand junipers test suite, but i can isolate it for you...
I added more context to 434, and I have also posted a bounty for this issue (427)

$50 out of my own pocket if someone can just get this one thing fixed, its the only thing which is causing me to not want to use juniper right now :)

https://www.bountysource.com/issues/80699749-literal-scalar-parsing-should-not-panic


Thanks !

(and unit tests of course)
@andy128k @dreamcodez I tried to reproduce this, but haven't been able to.

All seems to work as expected, without a panic.

Can you please provide a simple reproduction with a sample schema definition and query?

Fix patch:
diff --git a/juniper/src/parser/parser.rs b/juniper/src/parser/parser.rs
index e010180da..095fc1e7e 100644
--- a/juniper/src/parser/parser.rs
+++ b/juniper/src/parser/parser.rs
@@ -13,6 +13,9 @@ pub enum ParseError<'a> {
 
     /// An error during tokenization occurred
     LexerError(LexerError),
+
+    /// A scalar of unexpected type occurred in the source
+    ExpectedScalarError(&'static str),
 }
 
 #[doc(hidden)]
@@ -196,6 +199,7 @@ impl<'a> fmt::Display for ParseError<'a> {
             ParseError::UnexpectedToken(ref token) => write!(f, "Unexpected \"{}\"", token),
             ParseError::UnexpectedEndOfFile => write!(f, "Unexpected end of input"),
             ParseError::LexerError(ref err) => err.fmt(f),
+            ParseError::ExpectedScalarError(err) => err.fmt(f),
         }
     }
 }
diff --git a/juniper/src/parser/value.rs b/juniper/src/parser/value.rs
index 74dde9f99..260f9ec06 100644
--- a/juniper/src/parser/value.rs
+++ b/juniper/src/parser/value.rs
@@ -210,33 +210,36 @@ fn parse_scalar_literal_by_infered_type<'a, 'b, S>(
 where
     S: ScalarValue,
 {
-    match token {
+    let result = match token {
         ScalarToken::String(_) => {
             if let Some(&MetaType::Scalar(ref s)) = schema.concrete_type_by_name("String") {
-                (s.parse_fn)(token)
-                    .map(|s| Spanning::start_end(start, end, InputValue::Scalar(s)))
-                    .map_err(|e| Spanning::start_end(start, end, e))
+                (s.parse_fn)(token).map(InputValue::Scalar)
             } else {
-                panic!("There needs to be a String type")
+                Err(ParseError::ExpectedScalarError(
+                    "There needs to be a String type",
+                ))
             }
         }
         ScalarToken::Int(_) => {
             if let Some(&MetaType::Scalar(ref s)) = schema.concrete_type_by_name("Int") {
-                (s.parse_fn)(token)
-                    .map(|s| Spanning::start_end(start, end, InputValue::Scalar(s)))
-                    .map_err(|e| Spanning::start_end(start, end, e))
+                (s.parse_fn)(token).map(InputValue::Scalar)
             } else {
-                panic!("There needs to be a Int type")
+                Err(ParseError::ExpectedScalarError(
+                    "There needs to be an Int type",
+                ))
             }
         }
         ScalarToken::Float(_) => {
             if let Some(&MetaType::Scalar(ref s)) = schema.concrete_type_by_name("Float") {
-                (s.parse_fn)(token)
-                    .map(|s| Spanning::start_end(start, end, InputValue::Scalar(s)))
-                    .map_err(|e| Spanning::start_end(start, end, e))
+                (s.parse_fn)(token).map(InputValue::Scalar)
             } else {
-                panic!("There needs to be a Float type")
+                Err(ParseError::ExpectedScalarError(
+                    "There needs to be a Float type",
+                ))
             }
         }
-    }
+    };
+    result
+        .map(|s| Spanning::start_end(start, end, s))
+        .map_err(|e| Spanning::start_end(start, end, e))
 }


Test patch:
diff --git a/juniper/src/parser/tests/document.rs b/juniper/src/parser/tests/document.rs
index 97b6c31d3..cfe18d648 100644
--- a/juniper/src/parser/tests/document.rs
+++ b/juniper/src/parser/tests/document.rs
@@ -4,6 +4,7 @@ use crate::{
     },
     parser::{document::parse_document_source, ParseError, SourcePosition, Spanning, Token},
     schema::model::SchemaType,
+    types::scalars::EmptyMutation,
     validation::test_harness::{MutationRoot, QueryRoot},
     value::{DefaultScalarValue, ScalarRefValue, ScalarValue},
 };
@@ -145,3 +146,23 @@ fn errors() {
         )
     );
 }
+
+#[test]
+fn issue_427_panic_is_not_expected() {
+    struct QueryWithoutFloat;
+
+    #[crate::object_internal]
+    impl QueryWithoutFloat {
+        fn echo(value: String) -> String {
+            value
+        }
+    }
+
+    let schema = SchemaType::new::<QueryWithoutFloat, EmptyMutation<()>>(&(), &());
+    let parse_result = parse_document_source(r##"{ echo(value: 123.0) }"##, &schema);
+
+    assert_eq!(
+        parse_result.unwrap_err().item,
+        ParseError::ExpectedScalarError("There needs to be a Float type")
+    );
+}