Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

Stop 500's on a non-JSON /auth request #75

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DavidCain
Copy link

When the request is not JSON (and the force flag is False), Flask's get_json() will return None.

If somebody POSTs to /auth with a non-JSON mimetype, the server will 500 with 'NoneType' object has no attribute 'get'.

This fixes this behavior in the default auth_request_handler.

When the request is not JSON (and the `force` flag is False), Flask's
`get_json()` will return None:
https://github.com/mitsuhiko/flask/blob/0.10.1/flask/wrappers.py#L127

If somebody POSTs to `/auth` with a non-JSON mimetype, the server will
500 with `'NoneType' object has no attribute 'get'`.

Additionally, the default second parameter to `get()` is already None -
it can be safely omitted.

And more than one criterion are criteria. =)
@@ -111,12 +111,12 @@ def _default_request_handler():


def _default_auth_request_handler():
data = request.get_json()
username = data.get(current_app.config.get('JWT_AUTH_USERNAME_KEY'), None)
password = data.get(current_app.config.get('JWT_AUTH_PASSWORD_KEY'), None)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second parameter to get() is already None - it can be safely omitted.

If the mimetype is indeed JSON, but strings or arrays are POSTed,
an `AttributeError` will be thrown when trying to call `get()`.
@timofurrer
Copy link

What about this one?

@DavidCain
Copy link
Author

@mattupstate, any input on this one? It closes a pretty easy mechanism for invoking 500's.

@dequis
Copy link

dequis commented Jun 23, 2016

This one is basically the same as #70

@DavidCain
Copy link
Author

@dequis it's similar, but actually catches errors not handled by #70. If you were to pass a string or array with a valid JSON mimetype, request.get_json() will return something truthy. Then, the later call to data.get would raise an AttributeError, as neither strings nor lists implement a get method.

That said, if I'd seen #70, I probably would have just left this as a comment instead of making a new PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants