Skip to content

Commit

Permalink
Raise 400 Bad Request on server-side await request.json() if incorr…
Browse files Browse the repository at this point in the history
…ect content-type received (aio-libs#3889)
  • Loading branch information
Artem Yushkovskiy authored and asvetlov committed Jul 13, 2019
1 parent c8dbe75 commit 637db70
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGES/2174.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Raise 400 Bad Request on server-side `await request.json()` if incorrect content-type received.
13 changes: 2 additions & 11 deletions aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
BasicAuth,
HeadersMixin,
TimerNoop,
is_expected_content_type,
noop,
reify,
set_result,
Expand Down Expand Up @@ -80,9 +81,6 @@
from .tracing import Trace # noqa


json_re = re.compile(r'^application/(?:[\w.+-]+?\+)?json')


@attr.s(frozen=True, slots=True)
class ContentDisposition:
type = attr.ib(type=str) # type: Optional[str]
Expand Down Expand Up @@ -155,13 +153,6 @@ class ConnectionKey:
proxy_headers_hash = attr.ib(type=int) # type: Optional[int] # noqa # hash(CIMultiDict)


def _is_expected_content_type(response_content_type: str,
expected_content_type: str) -> bool:
if expected_content_type == 'application/json':
return json_re.match(response_content_type) is not None
return expected_content_type in response_content_type


class ClientRequest:
GET_METHODS = {
hdrs.METH_GET,
Expand Down Expand Up @@ -974,7 +965,7 @@ async def json(self, *, encoding: str=None,

if content_type:
ctype = self.headers.get(hdrs.CONTENT_TYPE, '').lower()
if not _is_expected_content_type(ctype, content_type):
if not is_expected_content_type(ctype, content_type):
raise ContentTypeError(
self.request_info,
self.history,
Expand Down
9 changes: 9 additions & 0 deletions aiohttp/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ async def noop(*args: Any, **kwargs: Any) -> None:

coroutines._DEBUG = old_debug # type: ignore

json_re = re.compile(r'^application/(?:[\w.+-]+?\+)?json')


class BasicAuth(namedtuple('BasicAuth', ['login', 'password', 'encoding'])):
"""Http basic authentication helper."""
Expand Down Expand Up @@ -366,6 +368,13 @@ def content_disposition_header(disptype: str,
return value


def is_expected_content_type(response_content_type: str,
expected_content_type: str) -> bool:
if expected_content_type == 'application/json':
return json_re.match(response_content_type) is not None
return expected_content_type in response_content_type


class reify:
"""Use as a class method decorator. It operates almost exactly like
the Python `@property` decorator, but it puts the result of the
Expand Down
25 changes: 22 additions & 3 deletions aiohttp/web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,14 @@

from . import hdrs
from .abc import AbstractStreamWriter
from .helpers import DEBUG, ChainMapProxy, HeadersMixin, reify, sentinel
from .helpers import (
DEBUG,
ChainMapProxy,
HeadersMixin,
is_expected_content_type,
reify,
sentinel,
)
from .http_parser import RawRequestMessage
from .multipart import MultipartReader
from .streams import EmptyStreamReader, StreamReader
Expand All @@ -41,7 +48,11 @@
RawHeaders,
StrOrURL,
)
from .web_exceptions import HTTPRequestEntityTooLarge, HTTPUnsupportedMediaType
from .web_exceptions import (
HTTPBadRequest,
HTTPRequestEntityTooLarge,
HTTPUnsupportedMediaType,
)
from .web_response import StreamResponse

__all__ = ('BaseRequest', 'FileField', 'Request')
Expand Down Expand Up @@ -581,9 +592,17 @@ async def text(self) -> str:
except LookupError:
raise HTTPUnsupportedMediaType()

async def json(self, *, loads: JSONDecoder=DEFAULT_JSON_DECODER) -> Any:
async def json(self, *,
loads: JSONDecoder=DEFAULT_JSON_DECODER,
content_type: Optional[str]='application/json') -> Any:
"""Return BODY as JSON."""
body = await self.text()
if content_type:
ctype = self.headers.get(hdrs.CONTENT_TYPE, '').lower()
if not is_expected_content_type(ctype, content_type):
raise HTTPBadRequest(text=('Attempt to decode JSON with '
'unexpected mimetype: %s' % ctype))

return loads(body)

async def multipart(self) -> MultipartReader:
Expand Down
16 changes: 7 additions & 9 deletions docs/web_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -377,21 +377,19 @@ and :ref:`aiohttp-web-signals` handlers.
The method **does** store read data internally, subsequent
:meth:`~Request.text` call will return the same value.

.. comethod:: json(*, loads=json.loads)
.. comethod:: json(*, loads=json.loads, \
content_type='application/json')

Read request body decoded as *json*.

The method is just a boilerplate :ref:`coroutine <coroutine>`
implemented as::

async def json(self, *, loads=json.loads):
body = await self.text()
return loads(body)
Read request body decoded as *json*. If request's content-type does not
match `content_type` parameter, :class:`web.HTTPBadRequest` get raised.
To disable content type check pass ``None`` value.

:param callable loads: any :term:`callable` that accepts
:class:`str` and returns :class:`dict`
with parsed JSON (:func:`json.loads` by
default).
:param str content_type: expected value of Content-Type header or ``None``
('application/json' by default)

.. note::

Expand Down
29 changes: 29 additions & 0 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from yarl import URL

from aiohttp import helpers
from aiohttp.helpers import is_expected_content_type

IS_PYPY = platform.python_implementation() == 'PyPy'

Expand Down Expand Up @@ -563,3 +564,31 @@ def test_repr(self) -> None:
cp = helpers.ChainMapProxy([d1, d2])
expected = "ChainMapProxy({!r}, {!r})".format(d1, d2)
assert expected == repr(cp)


def test_is_expected_content_type_json_match_exact():
expected_ct = 'application/json'
response_ct = 'application/json'
assert is_expected_content_type(response_content_type=response_ct,
expected_content_type=expected_ct)


def test_is_expected_content_type_json_match_partially():
expected_ct = 'application/json'
response_ct = 'application/alto-costmap+json' # mime-type from rfc7285
assert is_expected_content_type(response_content_type=response_ct,
expected_content_type=expected_ct)


def test_is_expected_content_type_non_json_match_exact():
expected_ct = 'text/javascript'
response_ct = 'text/javascript'
assert is_expected_content_type(response_content_type=response_ct,
expected_content_type=expected_ct)


def test_is_expected_content_type_non_json_not_match():
expected_ct = 'application/json'
response_ct = 'text/plain'
assert not is_expected_content_type(response_content_type=response_ct,
expected_content_type=expected_ct)
41 changes: 40 additions & 1 deletion tests/test_web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from multidict import CIMultiDict, MultiDict
from yarl import URL

from aiohttp import HttpVersion
from aiohttp import HttpVersion, web
from aiohttp.helpers import DEBUG
from aiohttp.streams import StreamReader
from aiohttp.test_utils import make_mocked_request
Expand Down Expand Up @@ -675,3 +675,42 @@ async def test_loop_prop() -> None:
req = make_mocked_request('GET', '/path', loop=loop)
with pytest.warns(DeprecationWarning):
assert req.loop is loop


async def test_json(aiohttp_client) -> None:
async def handler(request):
body_text = await request.text()
assert body_text == '{"some": "data"}'
assert request.headers['Content-Type'] == 'application/json'
body_json = await request.json()
assert body_json == {'some': 'data'}
return web.Response()

app = web.Application()
app.router.add_post('/', handler)
client = await aiohttp_client(app)

json_data = {'some': 'data'}
async with client.post('/', json=json_data) as resp:
assert 200 == resp.status


async def test_json_invalid_content_type(aiohttp_client) -> None:
async def handler(request):
body_text = await request.text()
assert body_text == '{"some": "data"}'
assert request.headers['Content-Type'] == 'text/plain'
await request.json() # raises HTTP 400
return web.Response()

app = web.Application()
app.router.add_post('/', handler)
client = await aiohttp_client(app)

json_data = {'some': 'data'}
headers = {'Content-Type': 'text/plain'}
async with client.post('/', json=json_data, headers=headers) as resp:
assert 400 == resp.status
resp_text = await resp.text()
assert resp_text == ('Attempt to decode JSON with '
'unexpected mimetype: text/plain')

0 comments on commit 637db70

Please sign in to comment.