Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request json raises error if no arguments passed #3302

Closed
dmytrostriletskyi opened this issue Sep 29, 2018 · 6 comments
Closed

Request json raises error if no arguments passed #3302

dmytrostriletskyi opened this issue Sep 29, 2018 · 6 comments
Labels
invalid This doesn't seem right outdated question StackOverflow server wontfix

Comments

@dmytrostriletskyi
Copy link

dmytrostriletskyi commented Sep 29, 2018

Long story short

The following code:

arguments = await request.json()
print(arguments)

Raises an exception:

[2018-09-29 11:42:33 +0000] [93] [ERROR] Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/aiohttp/web_protocol.py", line 390, in start
    resp = await self._request_handler(request)
  File "/usr/local/lib/python3.6/site-packages/aiohttp/web_app.py", line 366, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.6/site-packages/aiohttp/web_middlewares.py", line 106, in impl
    return await handler(request)
  File "/project/main/rest/middlewares/authorization.py", line 39, in auth_token_validation_middleware
    return await handler(request)
  File "/usr/local/lib/python3.6/site-packages/aiohttp/web_urldispatcher.py", line 740, in _iter
     resp = await method()
  File "/project/main/rest/endpoints/service/services/authentication/single.py", line 26, in put
    return await authentication.create_token(request=self.request)
  File "/project/main/core/service/services/authentication.py", line 99, in create_token
    arguments = await request.json()
  File "/usr/local/lib/python3.6/site-packages/aiohttp/web_request.py", line 551, in json
    return loads(body)
  File "/usr/local/lib/python3.6/json/__init__.py", line 354, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.6/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.6/json/decoder.py", line 357, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

After my http request without (!) PUT parameters:

http PUT :8000/some/endpoint

Expected behaviour

I expect, if length of request.text()equals 0, request.json() will return empty dictionary {} instead of raising.

Actual behaviour

While I pass parameter, my request.text() equals {"key": "value"} and request.json() equals {'key': 'value'}. When I do not pass parameter, JSONDecodeError is raised.

Steps to reproduce

from aiohttp import web


class SomeView(web.View):


    async def put(self):
        arguments = self.request.json()
        ...

Your environment

  1. aiohttp (3.4.2)
  2. Python 3.6
  3. MacOs
  4. Server
@aio-libs-bot
Copy link

GitMate.io thinks possibly related issues are #3242 (Error), #2174 (Insecure way to handle json requests), #2206 (json request for certain media types fail), #256 (More information provided when errors raised), and #2878 (Add argument 'raise_for_status' to 'ClientSession' request method.).

@asvetlov
Copy link
Member

I disagree.
request.json() returns a JSON decoded payload.
An empty string is invalid JSON.
Raising an exception for invalid JSON sounds reasonable to me.

@dmytrostriletskyi
Copy link
Author

dmytrostriletskyi commented Sep 29, 2018

@asvetlov, seems we consider the problem from different sides. Does aiohttp has simple features/ways to handle incoming arguments (if they are, we get dictionary, if not, if get something empty like {} without any exceptions) like request.POST in Django? Sorry for my bad knowledge of framework.

@asvetlov
Copy link
Member

await request.post() returns an empty multidict if BODY doesn't exists but JSON is different.

@webknjaz webknjaz added invalid This doesn't seem right question StackOverflow wontfix server and removed bug labels Sep 30, 2018
@webknjaz
Copy link
Member

I agree with Andrew. If body is empty, then it's an empty string, but you can't decode it as a JSON, because it's invalid.
OTOH you cannot choose a fallback, because it's app-specific and won't represent actual payload in any way. You expect dict, but someone else will expect list and I'll ask why not empty string then. See? It's overly ambiguous to try magically replace invalid input. And it's not a framework's job to do so.
If you need such a fallback you'll have to build your own helper for your app.

sirosen added a commit to sirosen/webargs that referenced this issue Sep 4, 2019
Fixes as the location loading code undergoes refactoring for the
flaskparser:
- Restore missing mimetype check for JSON payloads
- Treat form data as a multidict without extraneous AttributeError check
  (after testing, flask will return '{}' for form data when it is
  missing or empty -- just respect/allow this)
- req.headers is a multidict type as well
- fix some mistakes with new argument ordering

Changes to the base test app at src/webargs/testing.py of note:
- Because each route can only specify that it loads data from a single
  location (unless you pull extra tricks), several testing cases have
  had their routes modified, e.g. "/echo" -> "/echo_json"
- New test, test_parse_json_list_error_malformed_data added to check
  that webargs does not incorrectly treat non-list data as singleton
  lists when passing data to a schema with list fields. The previous
  behavior was questionable before, but with full data going to schemas
  it definitely becomes incorrect.
- Many tests which previously expected success with malformed data
  ignored now expect 422 errors. For example, sending a non-dict JSON
  value (e.g. `1`) should fail because it cannot be loaded by the
  schema. It should *not* be silently ignored because data was provided
  and failed to parse. This is a major behavior change for webargs.
- Reverse the decision on marshmallow-code#297 . After reading the referent material, I
  agree with the aiohttp maintainers in aio-libs/aiohttp#3302 . "" and
  "{}" should not be treated equivalently -- they are different inputs.
  Additionally, I foresee that supporting "" as "{}" will pose an issue
  if the proposal for "json_or_form" in marshmallow-code#419 is accepted.
  More detailed explanation is provided in the inline comment, and the
  test for this case is preserved with its expectations reversed

Supporting modifications were made in tests/apps/flask_app.py ,
including the addition of `ma.EXCLUDE` when the detected marshmallow
version is >= 3.

Minor fix in core: restore the cache clearing step. This is apparently
used by the flaskparser.
Possibly this only shows up under test usage? The main parse function
should always be cloning the parser before its cache could be
populated.
sirosen added a commit to sirosen/webargs that referenced this issue Sep 4, 2019
Fixes as the location loading code undergoes refactoring for the
flaskparser:
- Restore missing mimetype check for JSON payloads
- Treat form data as a multidict without extraneous AttributeError check
  (after testing, flask will return '{}' for form data when it is
  missing or empty -- just respect/allow this)
- req.headers is a multidict type as well
- fix some mistakes with new argument ordering

Changes to the base test app at src/webargs/testing.py of note:
- Because each route can only specify that it loads data from a single
  location (unless you pull extra tricks), several testing cases have
  had their routes modified, e.g. "/echo" -> "/echo_json"
- New test, test_parse_json_list_error_malformed_data added to check
  that webargs does not incorrectly treat non-list data as singleton
  lists when passing data to a schema with list fields. The previous
  behavior was questionable before, but with full data going to schemas
  it definitely becomes incorrect.
- Many tests which previously expected success with malformed data
  ignored now expect 422 errors. For example, sending a non-dict JSON
  value (e.g. `1`) should fail because it cannot be loaded by the
  schema. It should *not* be silently ignored because data was provided
  and failed to parse. This is a major behavior change for webargs.
- Reverse the decision on marshmallow-code#297 . After reading the referent material, I
  agree with the aiohttp maintainers in aio-libs/aiohttp#3302 . "" and
  "{}" should not be treated equivalently -- they are different inputs.
  Additionally, I foresee that supporting "" as "{}" will pose an issue
  if the proposal for "json_or_form" in marshmallow-code#419 is accepted.
  More detailed explanation is provided in the inline comment, and the
  test for this case is preserved with its expectations reversed

Supporting modifications were made in tests/apps/flask_app.py ,
including the addition of `ma.EXCLUDE` when the detected marshmallow
version is >= 3.
@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
invalid This doesn't seem right outdated question StackOverflow server wontfix
Projects
None yet
Development

No branches or pull requests

4 participants