InvalidRequestError: Invalid query produces an exception

[hroncok]

Consider a following query:

 GET /resource/?day=6

If the column day exists, everything goes fine.

Now (day is an integer), the following query produces a nice JSON encoded error:

GET /resource/?day=f
... "Not a valid integer type: f", error 400

But if I try with non-existent column:

GET /resource/?f=6
sqlalchemy.exc.InvalidRequestError: Entity '<class 'utvsapi.models.Course'>' has no property 'f'

It blows an exception to the client.

[timmartin19]

Out of curiosity, could I see the source code? Also, what are you running as your web framework? Flask? Is Debug on?

[hroncok]

https://github.com/hroncok/utvsapi-ripozo

Debug is on, do you want full traceback?

[hroncok]

127.0.0.1 - - [30/Apr/2016 23:16:59] "GET /courses/?f=1 HTTP/1.1" 500 -
Traceback (most recent call last):
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask_ripozo/dispatcher.py", line 216, in flask_dispatch
    return dispatcher.error_handler(dispatcher, accepted_mimetypes, e)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask_ripozo/dispatcher.py", line 51, in exception_handler
    raise exc
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/flask_ripozo/dispatcher.py", line 213, in flask_dispatch
    adapter = dispatcher.dispatch(f, accepted_mimetypes, ripozo_request)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/dispatch_base.py", line 212, in dispatch
    result = endpoint_func(request, *args, **kwargs)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/decorators.py", line 103, in newfunc
    return self.func(klass, *args)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/decorators.py", line 197, in wrapped
    resource = func(cls, request, *args, **kwargs)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/decorators.py", line 111, in __call__
    return self.__get__(None, klass=cls)(*args, **kwargs)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/decorators.py", line 103, in newfunc
    return self.func(klass, *args)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/decorators.py", line 369, in action
    return func(cls, request, *args, **kwargs)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo/resources/restmixins.py", line 171, in retrieve_list
    props, meta = cls.manager.retrieve_list(request.query_args)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo_sqlalchemy/alchemymanager.py", line 61, in wrapper
    raise exc
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo_sqlalchemy/alchemymanager.py", line 58, in wrapper
    resp = func(self, session, *args, **kwargs)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/ripozo_sqlalchemy/alchemymanager.py", line 194, in retrieve_list
    query = query.filter_by(**filters)
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/sqlalchemy/orm/query.py", line 1510, in filter_by
    for key, value in kwargs.items()]
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/sqlalchemy/orm/query.py", line 1510, in <listcomp>
    for key, value in kwargs.items()]
  File "utvsapi-ripozo/venv/lib/python3.4/site-packages/sqlalchemy/orm/base.py", line 383, in _entity_descriptor
    (description, key)
sqlalchemy.exc.InvalidRequestError: Entity '<class 'utvsapi.models.Course'>' has no property 'f'

[timmartin19]

Ah... It seems to be an issue with this line. It should be doing one of two things: Either ignoring fields not in the list_fields property or raising a ValidationException if a filter is not included in the list_fields. I'm leaning towards the exception personally, which would result in a well formatted exception and 400 response.

I'll try to get a fix out shortly (unless of course you want to jump on it). In the meantime, you can wrap it to get the correct response

def invalid_request_handler(f):
    def wrapper(*args, **kwargs):
        try:
            return f(*args, **kwargs)
        except InvalidRequestError as e:
            raise ValidationException('One of your filters doesn't exists!')
     return wrapper

class MyManager(AlchemyManager):
    model = MyModel
    ...

    retrieve_list = invalid_request_handler(AlchemyManager.retrieve_list)

[hroncok]

A 400 response seems like a right thing to do. Thanks for looking into it.

You could even do something like:

try:
    query = query.filter_by(**filters)
except InvalidRequestError as e:
    field = str(e).split("'")[-2]
    raise ValidationException(
        '{} is not a valid filter on this resource.'.format(field))

[timmartin19]

It presents a certain security issue actually, people could theoretically query on fields that are not exposed to them.

[hroncok]

Well in that case can a pre check be run to see if the filter is fine?

I'm not sure if that is OK either, because some fileds might get hidden in the postprocessor etc.

[timmartin19]

Yeah, that's probably what I'll end up doing. I may simply add another property that defaults to list_fields something like filter_fields so that you can explicitly state which fields you allow a user to filter on. I can't really prevent people from hiding stuff in the postprocessors, since the managers really have no concept of a resource.