Flask: get_json's silent option cache None and causes weird side-effect

Created on 5 Mar 2018  路  7Comments  路  Source: pallets/flask

Expected Behavior

Sample Code

app = Flask(__name__)

@app.route("/", methods=["POST"])
def broke_behvior():
    print(request.is_json)
    rv = request.get_json(silent=True)
    print(rv)
    rv = request.get_json()
    print(rv)
    return "", 200

client = app.test_client()
client.post("/", data="'invalid json'")

Output

False
None
<Response streamed [400 BAD REQUEST]>

Actual Behavior

False
None
None
<Response streamed [200 OK]>

The fact that calling get_json with silent and then calling it without silent seems to break expected behavior. When I don't call silent I am depending on the exception being thrown, however, because the code caches even on error, you get this weird behavior.

While this might be expected behavior by the authors, the docs don't make this clear.

I would think, invalid json should not be cached, yet I can see the value of it, however, it adds an layer of uncertainty and therefore, I my opinion, is a bug.

Environment

  • Python version: 3.4
  • Flask version: 0.12.2
  • Werkzeug version: 0.14.1
bug json

Most helpful comment

Thought about this more. I can see the issue where an extension or before_request might call get_json(slient=True), thus hiding the error from a user-written view when get_json is called.

All 7 comments

See also #2087 and #2181.

Why are you using silent then not silent in the same request? I don't think this behavior is inconsistent.

@davidism I'm not actually, it's a contrived example to show the behavior. We have some before and after hooks that call get_json(slient) because we didn't want to throw if there was an error, but there was no way to know this would cause invalid json to be cached as None, that's not very predictable behavior and it's not documented.

Then stop using silent if it's not appropriate for your use case. Catch the error yourself and ignore it.

I'm fine with a PR clarifying the docs, but I don't think the behavior should change.

I'll make a PR, but I disagree that the behavior isn't odd to the point of being a bug. Using silent anywhere else in the Request's life time changes the behavior of calling the function later in the Request's life. That's not a good thing. Weird side-effects like this introduce hard to find bugs, like this did for our usage of Flask.

Thought about this more. I can see the issue where an extension or before_request might call get_json(slient=True), thus hiding the error from a user-written view when get_json is called.

I'm marking this as 1.1, but if some can submit a PR in the next day or so I'll put it in 1.0.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

davidism picture davidism  路  3Comments

wadegilmer picture wadegilmer  路  4Comments

KeyC0de picture KeyC0de  路  4Comments

jab picture jab  路  4Comments