Fix invalid query parameters were not handled (#3927)

* Fix handling of invalid query parameter

* Add release file

* Use more precise error message

Author: DoctorJohn

RELEASE.md

@@ -0,0 +1,6 @@
+Release type: patch
+
+In this release, we updated Strawberry to gracefully handle requests containing
+an invalid `query` parameter. Previously, such requests could result in internal
+server errors, but now they will return a 400 Bad Request response with an
+appropriate error message, conforming to the GraphQL over HTTP specification.

strawberry/http/async_base_view.py

@@ -529,8 +529,15 @@ async def parse_http_body(
         else:
             raise HTTPException(400, "Unsupported content type")
 
+        query = data.get("query")
+        if not isinstance(query, (str, type(None))):
+            raise HTTPException(
+                400,
+                "The GraphQL operation's `query` must be a string or null, if provided.",
+            )
+
         return GraphQLRequestData(
-            query=data.get("query"),
+            query=query,
             variables=data.get("variables"),
             operation_name=data.get("operationName"),
             extensions=data.get("extensions"),

strawberry/http/sync_base_view.py

@@ -154,8 +154,15 @@ def parse_http_body(self, request: SyncHTTPRequestAdapter) -> GraphQLRequestData
         else:
             raise HTTPException(400, "Unsupported content type")
 
+        query = data.get("query")
+        if not isinstance(query, (str, type(None))):
+            raise HTTPException(
+                400,
+                "The GraphQL operation's `query` must be a string or null, if provided.",
+            )
+
         return GraphQLRequestData(
-            query=data.get("query"),
+            query=query,
             variables=data.get("variables"),
             operation_name=data.get("operationName"),
             extensions=data.get("extensions"),

tests/http/test_graphql_over_http_spec.py

@@ -214,10 +214,6 @@ async def test_423l(http_client):
     assert response.status_code == 400
 
 
-@pytest.mark.xfail(
-    reason="OPTIONAL - Currently results in lots of TypeErrors",
-    raises=AssertionError,
-)
 @pytest.mark.parametrize(
     "invalid",
     [{"obj": "ect"}, 0, False, ["array"]],

tests/http/test_query.py

@@ -196,6 +196,30 @@ async def test_missing_query(http_client: HttpClient):
     assert "No GraphQL query found in the request" in response.text
 
 
+@pytest.mark.parametrize(
+    "not_stringified_json",
+    [{"obj": "ect"}, 0, False, ["array"]],
+)
+async def test_requests_with_invalid_query_parameter_are_rejected(
+    http_client: HttpClient, not_stringified_json
+):
+    response = await http_client.query(
+        query=not_stringified_json,
+    )
+
+    assert response.status_code == 400
+    message = "The GraphQL operation's `query` must be a string or null, if provided."
+
+    if isinstance(http_client, ChaliceHttpClient):
+        # Our Chalice integration purposely wraps errors messages with a JSON object
+        assert response.json == {
+            "Code": "BadRequestError",
+            "Message": message,
+        }
+    else:
+        assert response.data == message.encode()
+
+
 @pytest.mark.parametrize("method", ["get", "post"])
 async def test_query_context(method: Literal["get", "post"], http_client: HttpClient):
     response = await http_client.query(