Skip to content

Commit

Permalink
Make handling unexpected content types configurable
Browse files Browse the repository at this point in the history 
Adds some configuration options to determine whether or not to pass
through unexpected content types. Also does a more significant version
bump, since this is technically a backwards incompatible change
(although not one that changes _defined_ behavior).
  • Loading branch information
@mplanchard
mplanchard committed Sep 27, 2019
1 parent 23de1e1 commit d7de20a
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 26 deletions.
2 changes: 1 addition & 1 deletion falcon_marshmallow/_version.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@
unicode_literals,
)

__version_info__ = (0, 3, 1)
__version_info__ = (0, 4, 0)
__version__ = ".".join([str(ver) for ver in __version_info__])
61 changes: 49 additions & 12 deletions falcon_marshmallow/middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@


JSON_CONTENT_REQUIRED_METHODS = ("POST", "PUT", "PATCH")
JSON_CONTENT_TYPE = 'application/json'
JSON_CONTENT_TYPE = "application/json"
CONTENT_KEY = "content"
MARSHMALLOW_2 = marshmallow.__version_info__ < (3,)

Expand Down Expand Up @@ -142,14 +142,20 @@ def process_request(self, req, resp):
class Marshmallow:
"""Attempt to deserialize objects with any available schemas"""

def __init__(
# TODO: consider some sort of config object as the options here
# continue to grow in number.
def __init__( # pylint: disable=too-many-arguments
self,
req_key="json",
resp_key="result",
force_json=True,
# TODO: deprecate `json_module` param and change name to something
# more generic, e.g. `content_parser`, with a specified interface
json_module=simplejson,
expected_content_type=JSON_CONTENT_TYPE,
handle_unexpected_content_types=False,
):
# type: (str, str, bool, Any) -> None
# type: (str, str, bool, Any, str, bool) -> None
"""Instantiate the middleware object
:param req_key: (default ``'json'``) the key on the
Expand All @@ -168,6 +174,19 @@ def __init__(
module for your Marshmallow schemas, you will have to
specify using a schema metaclass, as defined in the
`Marshmallow documentation`_
:param expected_content_type: the expected CONTENT_TYPE header
corresponding to content that should be parsed by the
Marshmallow schema. By default, responses that
have a specified content type other than `expected_content_type`
will be ignored by this middleware. See
`handle_unexpected_content_types` for more options.
:param handle_unexpected_content_types: whether content types other
than the `expected_content_type` should be handled, or
whether they should be ignored and allowed to pass through
to the application or to other middlewares. This defaults to
True. If it is set to False, the middleware will attempt to
parse ALL requests with the provided json_module and/or
Marshmallow schema.
.. _marshmallow documentation: http://marshmallow.readthedocs.io/
en/latest/api_reference.html#marshmallow.Schema.Meta
Expand All @@ -184,6 +203,8 @@ def __init__(
self._resp_key = resp_key
self._force_json = force_json
self._json = json_module
self._expected_content_type = expected_content_type
self._handle_unexpected_content_types = handle_unexpected_content_types

@staticmethod
def _get_specific_schema(resource, method, msg_type):
Expand Down Expand Up @@ -264,24 +285,32 @@ def _get_schema(cls, resource, method, msg_type):
return specific_schema
return getattr(resource, "schema", None) # type: ignore

@classmethod
def _content_type_is_json(cls, content_type):
def _content_is_expected_type(self, content_type):
# type: (str) -> bool
"""Check if the provided content type is json. This uses similar code to client_accepts in falcon.request.
If content type is not provided, assume json for backwards compatibility.
"""Check if the provided content type is json.
This uses similar code to client_accepts in falcon.request.
If content type is not provided, assume json for backwards
compatibility.
:param content_type: a content type string from the request object
(e.g., 'application/json', 'text/csv', 'application/json;encoding=latin1')
(e.g., 'application/json', 'text/csv',
'application/json;encoding=latin1')
:return: true if the given content type represents JSON
"""
# PERF(kgriffs): Usually the following will be true, so
# try it first.
if content_type == JSON_CONTENT_TYPE or content_type is None:
if content_type == self._expected_content_type or content_type is None:
return True

# Fall back to full-blown parsing
try:
return mimeparse.quality(content_type, JSON_CONTENT_TYPE) != 0.0
return bool(
mimeparse.quality(content_type, self._expected_content_type)
!= 0.0
)
except ValueError:
return False

Expand Down Expand Up @@ -320,8 +349,16 @@ def process_resource(self, req, resp, resource, params):
if req.content_length in (None, 0):
return

if not self._content_type_is_json(req.content_type):
log.info("Non-json input, skipping deserialization")
if (
not self._handle_unexpected_content_types
and not self._content_is_expected_type(req.content_type)
):
log.info(
"Input type (%s) is not of expected type (%s), "
"skipping deserialization",
req.content_type,
self._expected_content_type,
)
return

sch = self._get_schema(resource, req.method, "request")
Expand Down
71 changes: 58 additions & 13 deletions tests/test_middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,12 @@ def __init__(self):
assert val == exp_value

@pytest.mark.parametrize(
"stream, content_type, schema, schema_err, bad_sch, force_json, json_err, " "exp_ret",
"stream, content_type, schema, schema_err, bad_sch, force_json, "
"json_err, exp_ret",
[
( # 0: Good schema
'{"foo": "test"}',
'application/json',
"application/json",
True,
False,
False,
Expand All @@ -152,7 +153,7 @@ def __init__(self):
),
( # 1: Schema errors on load
'{"foo": "test", "int": "test"}',
'application/json',
"application/json",
True,
True,
False,
Expand All @@ -162,7 +163,7 @@ def __init__(self):
),
( # 2: Good schema, bad unicode in body
'{"foo": "testé"}',
'application/json',
"application/json",
True,
False,
False,
Expand All @@ -172,7 +173,7 @@ def __init__(self):
),
( # 3: Bad schema
'{"foo": "test"}',
'application/json',
"application/json",
True,
False,
True,
Expand All @@ -182,7 +183,7 @@ def __init__(self):
),
( # 4: No schema, no force json (no change to req.context)
'{"foo": "test"}',
'application/json',
"application/json",
False,
False,
False,
Expand All @@ -192,7 +193,7 @@ def __init__(self):
),
( # 5: No schema, force json
'{"foo": "test"}',
'application/json',
"application/json",
False,
False,
False,
Expand All @@ -202,7 +203,7 @@ def __init__(self):
),
( # 6: No schema, force json, bad json
'{"foo": }',
'application/json',
"application/json",
False,
False,
False,
Expand All @@ -212,7 +213,7 @@ def __init__(self):
),
( # 7: No schema, force json, good json, bad unicode
'{"foo": "testé"}',
'application/json',
"application/json",
False,
False,
False,
Expand All @@ -222,7 +223,7 @@ def __init__(self):
),
( # 8: Good schema, extra info on content type
'{"foo": "test"}',
'application/json;encoding=latin1',
"application/json;encoding=latin1",
True,
False,
False,
Expand All @@ -241,8 +242,8 @@ def __init__(self):
{"bar": "test"},
),
( # 10: Non-json (no change to req.context)
'1,2,\'string\'',
'text/csv',
"1,2,'string'",
"text/csv",
False,
False,
False,
Expand All @@ -253,7 +254,15 @@ def __init__(self):
],
)
def test_process_resource(
self, stream, content_type, schema, schema_err, bad_sch, force_json, json_err, exp_ret
self,
stream,
content_type,
schema,
schema_err,
bad_sch,
force_json,
json_err,
exp_ret,
):
# type: (str, str, bool, bool, bool, bool, bool, dict) -> None
"""Test processing a resource
Expand Down Expand Up @@ -301,6 +310,42 @@ def test_process_resource(
else:
assert mw._req_key not in req.context

def test_process_resource_ignores_unexpected_content_by_default(self):
"""By default, we ignore unexpected content types."""
mw = mid.Marshmallow()
setattr(mw, "_get_schema", lambda *_, **__: self.FooSchema())
req = mock.Mock(method="GET", content_type="application/pdf")
req.bounded_stream.read.return_value = ""
req.context = {}

mw.process_resource(req, "foo", "foo", "foo") # type: ignore
assert mw._req_key not in req.context

def test_nondefault_unexpected_content_type(self):
"""We can specify any content type to handle."""
mw = mid.Marshmallow(expected_content_type="application/pdf")
setattr(mw, "_get_schema", lambda *_, **__: self.FooSchema())
req = mock.Mock(method="GET", content_type="application/pdf")
req.bounded_stream.read.return_value = '{"foo": "test"}'
req.context = {}

mw.process_resource(req, "foo", "foo", "foo") # type: ignore
assert req.context[mw._req_key] == {"bar": "test"}

def test_handle_unexpected_content_type(self):
"""We can specify to handle unexpected types."""
mw = mid.Marshmallow(
expected_content_type="application/pdf",
handle_unexpected_content_types=True,
)
setattr(mw, "_get_schema", lambda *_, **__: self.FooSchema())
req = mock.Mock(method="GET", content_type="text/plain")
req.bounded_stream.read.return_value = '{"foo": "test"}'
req.context = {}

mw.process_resource(req, "foo", "foo", "foo") # type: ignore
assert req.context[mw._req_key] == {"bar": "test"}

@pytest.mark.parametrize(
"res, schema, sch_err, bad_sch, force_json, json_err, exp_ret",
[
Expand Down

0 comments on commit d7de20a

Please sign in to comment.