django-json-api / django-rest-framework-json-api

JSON:API support for Django REST framework
https://django-rest-framework-json-api.readthedocs.org/
BSD 2-Clause "Simplified" License
1.19k stars 295 forks source link

500 error on invalid parameter when using QueryParameterValidationFilter with BrowsableAPIRenderer #665

Open CDavantzis opened 5 years ago

CDavantzis commented 5 years ago

If a viewset is using the filter backend rest_framework_json_api.filters.QueryParameterValidationFilter the server will throw a 500 error when an invalid parameter is provided and the response is being rendered using rest_framework.renderers.BrowsableAPIRenderer.

The below code snippet is one way to fix the issue.

import rest_framework.renderers
import rest_framework.exceptions

class BrowsableAPIRenderer(rest_framework.renderers.BrowsableAPIRenderer):

    def get_filter_form(self, *args, **kwargs):
        """
        override `get_filter_form` so that if a user uses
        uses a filter backend that throws a `ValidationError`
        such as `QueryParameterValidationFilter` the server
        will return the proper 4XX error with incorrect query
        parameters, rather than a 500 error.
        """
        try:
            return super(BrowsableAPIRenderer, self).get_filter_form(*args, **kwargs)
        except rest_framework.exceptions.ValidationError:
            return ""

It may be in the scope of this project to add this fix to rest_framework_json_api.renderers or mention it in the documentation.

There are other issues with allowing the BrowsableApiRenderer with the QueryParameterValidationFilter such as the 'format' option the browsable api provides.

class QueryParameterValidationFilter(rest_framework_json_api.filters.QueryParameterValidationFilter):

    query_regex = re.compile(r'^(sort|include|format)$|^(filter|fields|page)(\[[\w\.\-]+\])?$')

Users can use the above fix to allow format, however they may want to do something more complex such as overriding QueryParameterValidationFilter.validate_query_params so that it uses a different query_regex based on the if the view is a subclass of rest_framework.renderers.BrowsableAPIRenderer

sliverc commented 5 years ago

As DJA we support BrowsableAPIRenderer so using the different filter backend should work. I haven't looked into it in detail so not sure whether you fix is the only solution. Feel free thought to work on a PR also with a reproducing test.

In terms of format parameter see discussion in #535