Skip to content

Commit

Permalink
Add a guard around dagit's JSON parsing of incoming requests (#7714)
Browse files Browse the repository at this point in the history
Summary:
Before, malformed JSON bubbled all the way up to the top and manifested as a 500/Internal Server Error. Now, it is caught and raises a 400.
  • Loading branch information
gibsondan committed May 3, 2022
1 parent 52ff71f commit d4293b8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
9 changes: 8 additions & 1 deletion python_modules/dagit/dagit/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,14 @@ async def graphql_http_endpoint(self, request: Request):
content_type = request.headers.get("Content-Type", "")

if "application/json" in content_type:
data = await request.json()
try:
data = await request.json()
except json.JSONDecodeError:
body = await request.body()
return PlainTextResponse(
f"GraphQL request is invalid JSON:\n{body.decode()}",
status_code=status.HTTP_400_BAD_REQUEST,
)
elif "application/graphql" in content_type:
body = await request.body()
text = body.decode()
Expand Down
12 changes: 12 additions & 0 deletions python_modules/dagit/dagit_tests/webserver/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ def test_graphql_get(instance, test_client: TestClient): # pylint: disable=unus
assert response.status_code == 400, response.text


def test_graphql_invalid_json(instance, test_client: TestClient): # pylint: disable=unused-argument
# base case
response = test_client.post(
"/graphql", data='{"query": "foo}', headers={"Content-Type": "application/json"}
)

print(str(response.text))

assert response.status_code == 400, response.text
assert 'GraphQL request is invalid JSON:\n{"query": "foo}' in response.text


def test_graphql_post(test_client: TestClient):
# base case
response = test_client.post(
Expand Down

0 comments on commit d4293b8

Please sign in to comment.