From 6461c1b8a1dcb8820eeb32bbfbe8e3f22b3fa7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bisinger?= Date: Wed, 13 May 2015 19:08:56 +0200 Subject: [PATCH] Complex types should check unexpected attributes Check if a request is passing more attributes for complex objects than those defined in the API. WSME did not care if some unknown attribute was passed to an API with a complex type, only checking the required attributes. This is fixed raising a ValueError if more attributes are given than expected, resulting in an HTTP response with a 400 status. This helps check the validity of requests, which would otherwise unexpectedly work with (partially) invalid data. Closes-Bug: #1277571 Change-Id: Idf720a1c3fac8bdc8dca21a1ccdb126110dae62e --- wsme/exc.py | 28 +++++++++++++++++ wsme/rest/json.py | 56 +++++++++++++++++++++++---------- wsme/tests/test_restjson.py | 62 +++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 17 deletions(-) diff --git a/wsme/exc.py b/wsme/exc.py index 0dfc575..81dcbf8 100644 --- a/wsme/exc.py +++ b/wsme/exc.py @@ -62,3 +62,31 @@ class UnknownFunction(ClientSideError): @property def faultstring(self): return _(six.u("Unknown function name: %s")) % (self.name) + + +class UnknownAttribute(ClientSideError): + def __init__(self, fieldname, attributes, msg=''): + self.fieldname = fieldname + self.attributes = attributes + self.msg = msg + super(UnknownAttribute, self).__init__(self.msg) + + @property + def faultstring(self): + error = _("Unknown attribute for argument %(argn)s: %(attrs)s") + if len(self.attributes) > 1: + error = _("Unknown attributes for argument %(argn)s: %(attrs)s") + str_attrs = ", ".join(self.attributes) + return error % {'argn': self.fieldname, 'attrs': str_attrs} + + def add_fieldname(self, name): + """Add a fieldname to concatenate the full name. + + Add a fieldname so that the whole hierarchy is displayed. Successive + calls to this method will prepend ``name`` to the hierarchy of names. + """ + if self.fieldname is not None: + self.fieldname = "{}.{}".format(name, self.fieldname) + else: + self.fieldname = name + super(UnknownAttribute, self).__init__(self.msg) diff --git a/wsme/rest/json.py b/wsme/rest/json.py index abe3d4a..a7ba365 100644 --- a/wsme/rest/json.py +++ b/wsme/rest/json.py @@ -1,6 +1,4 @@ -""" -REST+Json protocol implementation. -""" +"""REST+Json protocol implementation.""" from __future__ import absolute_import import datetime import decimal @@ -9,10 +7,10 @@ import six from simplegeneric import generic -from wsme.types import Unset +import wsme.exc import wsme.types +from wsme.types import Unset import wsme.utils -from wsme.exc import ClientSideError, UnknownArgument, InvalidInput try: @@ -116,8 +114,7 @@ def datetime_tojson(datatype, value): @generic def fromjson(datatype, value): - """ - A generic converter from json base types to python datatype. + """A generic converter from json base types to python datatype. If a non-complex user specific type is to be used in the api, a specific fromjson should be added:: @@ -135,16 +132,31 @@ def fromjson(datatype, value): return None if wsme.types.iscomplex(datatype): obj = datatype() - for attrdef in wsme.types.list_attributes(datatype): + attributes = wsme.types.list_attributes(datatype) + + # Here we check that all the attributes in the value are also defined + # in our type definition, otherwise we raise an Error. + v_keys = set(value.keys()) + a_keys = set(adef.name for adef in attributes) + if not v_keys <= a_keys: + raise wsme.exc.UnknownAttribute(None, v_keys - a_keys) + + for attrdef in attributes: if attrdef.name in value: - val_fromjson = fromjson(attrdef.datatype, value[attrdef.name]) + try: + val_fromjson = fromjson(attrdef.datatype, + value[attrdef.name]) + except wsme.exc.UnknownAttribute as e: + e.add_fieldname(attrdef.name) + raise if getattr(attrdef, 'readonly', False): - raise InvalidInput(attrdef.name, val_fromjson, - "Cannot set read only field.") + raise wsme.exc.InvalidInput(attrdef.name, val_fromjson, + "Cannot set read only field.") setattr(obj, attrdef.key, val_fromjson) elif attrdef.mandatory: - raise InvalidInput(attrdef.name, None, - "Mandatory field missing.") + raise wsme.exc.InvalidInput(attrdef.name, None, + "Mandatory field missing.") + return wsme.types.validate_value(datatype, obj) elif wsme.types.isusertype(datatype): value = datatype.frombasetype( @@ -243,13 +255,18 @@ def parse(s, datatypes, bodyarg, encoding='utf8'): try: jdata = jload(s) except ValueError: - raise ClientSideError("Request is not in valid JSON format") + raise wsme.exc.ClientSideError("Request is not in valid JSON format") if bodyarg: argname = list(datatypes.keys())[0] try: kw = {argname: fromjson(datatypes[argname], jdata)} except ValueError as e: - raise InvalidInput(argname, jdata, e.args[0]) + raise wsme.exc.InvalidInput(argname, jdata, e.args[0]) + except wsme.exc.UnknownAttribute as e: + # We only know the fieldname at this level, not in the + # called function. We fill in this information here. + e.add_fieldname(argname) + raise else: kw = {} extra_args = [] @@ -260,9 +277,14 @@ def parse(s, datatypes, bodyarg, encoding='utf8'): try: kw[key] = fromjson(datatypes[key], jdata[key]) except ValueError as e: - raise InvalidInput(key, jdata[key], e.args[0]) + raise wsme.exc.InvalidInput(key, jdata[key], e.args[0]) + except wsme.exc.UnknownAttribute as e: + # We only know the fieldname at this level, not in the + # called function. We fill in this information here. + e.add_fieldname(key) + raise if extra_args: - raise UnknownArgument(', '.join(extra_args)) + raise wsme.exc.UnknownArgument(', '.join(extra_args)) return kw diff --git a/wsme/tests/test_restjson.py b/wsme/tests/test_restjson.py index 23731d6..0250a0c 100644 --- a/wsme/tests/test_restjson.py +++ b/wsme/tests/test_restjson.py @@ -100,6 +100,10 @@ class Obj(wsme.types.Base): name = wsme.types.text +class NestedObj(wsme.types.Base): + o = Obj + + class CRUDResult(object): data = Obj message = wsme.types.text @@ -476,6 +480,37 @@ class TestRestJson(wsme.tests.protocol.RestOnlyProtocolTestCase): "invalid literal for int() with base 10: '%s'" % value ) + def test_parse_unexpected_attribute(self): + o = { + "id": "1", + "name": "test", + "other": "unknown", + "other2": "still unknown", + } + for ba in True, False: + jd = o if ba else {"o": o} + try: + parse(json.dumps(jd), {'o': Obj}, ba) + raise AssertionError("Object should not parse correcty.") + except wsme.exc.UnknownAttribute as e: + self.assertEqual(e.attributes, set(['other', 'other2'])) + + def test_parse_unexpected_nested_attribute(self): + no = { + "o": { + "id": "1", + "name": "test", + "other": "unknown", + }, + } + for ba in False, True: + jd = no if ba else {"no": no} + try: + parse(json.dumps(jd), {'no': NestedObj}, ba) + except wsme.exc.UnknownAttribute as e: + self.assertEqual(e.attributes, set(['other'])) + self.assertEqual(e.fieldname, "no.o") + def test_nest_result(self): self.root.protocols[0].nest_result = True r = self.app.get('/returntypes/getint.json') @@ -659,6 +694,33 @@ class TestRestJson(wsme.tests.protocol.RestOnlyProtocolTestCase): assert result['data']['name'] == u("test") assert result['message'] == "read" + def test_unexpected_extra_arg(self): + headers = { + 'Content-Type': 'application/json', + } + data = {"id": 1, "name": "test"} + content = json.dumps({"data": data, "other": "unexpected"}) + res = self.app.put( + '/crud', + content, + headers=headers, + expect_errors=True) + self.assertEqual(res.status_int, 400) + + def test_unexpected_extra_attribute(self): + """Expect a failure if we send an unexpected object attribute.""" + headers = { + 'Content-Type': 'application/json', + } + data = {"id": 1, "name": "test", "other": "unexpected"} + content = json.dumps({"data": data}) + res = self.app.put( + '/crud', + content, + headers=headers, + expect_errors=True) + self.assertEqual(res.status_int, 400) + def test_body_arg(self): headers = { 'Content-Type': 'application/json',