Sanic: Unparseable JSON should not be ignored

[jasonab]

request.py, line 64

Returning a None here causes breakage in application code further down the line. Generate a 400 error here on malformed JSON to protect the server.

[seemethere]

Should it just raise an Exception that then generates a 400 error?

[jasonab]

Yes, that would be my preference.

[seemethere]

The actual line is here now: https://github.com/channelcat/sanic/blob/master/sanic/request.py#L69

[jasonab]

Correct, that line should not log anything (as I don't want my logs littered with client-side problems), and should raise an InvalidUsage exception.

[r0fls]

@jasonab I am not able to reproduce this, am I misunderstanding the bug? I added this test and although it fails, there is a 500 internal server error (not the silent, ignoring result that you're referring to):

def test_post_invalid_json():
    app = Sanic('test_post_json')

    @app.route('/')
    async def handler(request):
        data = request.json()
        return json(data)

    payload = None
    headers = {'content-type': 'application/json'}

    request, response = sanic_endpoint_test(app, 
                                            data=json_dumps(payload), 
                                            headers=headers)

    assert response.status == '400'

[jasonab]

You're saying Sanic now throws a 500 error on malformed JSON? That doesn't sound right.

The initial issue was that if JSON parsing fails and throws an exception, Sanic catches that exception and returns None from the json() method. Any code of mine that assumes that we have a non-None value from json() breaks.

What stack trace are you now seeing?

[r0fls]

Ah, I was misunderstanding then. That test is malformed post data. I have observed the issue you're referring to as well.

[r0fls]

I don't think that request.py actually has anything to do with this, since we're talking about invalid json in the response. This looks like it's a result of the dump function in ujson:

>>> import ujson
>>> ujson.dumps(None)
'null'
>>> ujson.dumps("I am not valid json")
'"i am not valid json"'

See:
https://github.com/channelcat/sanic/blob/master/sanic/response.py#L5

and:
https://github.com/channelcat/sanic/blob/master/sanic/response.py#L135

[jasonab]

The line of code I cited (now at request.py line 69) is for incoming data, not response data. I'm sorry if there's been confusion.

To restate the problem: when you call request.json(), if the posted JSON data causes the parser to error out, Sanic returns a None to the controller/resource. This None will tend to blow up any code operating on that returned value.

What I believe should happen is that Sanic should raise an exception and return a 400-level error if malformed JSON is sent with the request.

[albanm]

This issue is marked as closed, but I have a very similar problem with sanic 0.6.0.

request.json contains a string instead of a dict when the incoming json is invalid.