Validation of include parameter causes unhandled exception

[brettdh]

Description of the Bug Report

When requesting an API endpoint with the include query param, specifying an unsupported value results in an uncaught exception and traceback (below). Note: this only seems to happen when querying using the browseable API. When I use Insomnia, I get a 400 response as I'd expect.

Versions:

• Python: 3.6.15
• Django: 3.1.8
• DRF: 3.12.4
• DRF-JA: 3.1.0

Stack trace (browseable API)

Traceback (most recent call last):
  File "/opt/python/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/opt/python/django/core/handlers/base.py", line 204, in _get_response
    response = response.render()
  File "/opt/python/django/template/response.py", line 105, in render
    self.content = self.rendered_content
  File "/opt/python/rest_framework/response.py", line 70, in rendered_content
    ret = renderer.render(self.data, accepted_media_type, context)
  File "/opt/python/rest_framework/renderers.py", line 724, in render
    context = self.get_context(data, accepted_media_type, renderer_context)
  File "/opt/python/rest_framework/renderers.py", line 655, in get_context
    raw_data_post_form = self.get_raw_data_form(data, view, 'POST', request)
  File "/opt/python/rest_framework/renderers.py", line 554, in get_raw_data_form
    serializer = view.get_serializer()
  File "/opt/python/rest_framework/generics.py", line 110, in get_serializer
    return serializer_class(*args, **kwargs)
  File "/opt/python/rest_framework_json_api/serializers.py", line 113, in __init__
    validate_path(this_serializer_class, included_field_path, included_field_name)
  File "/opt/python/rest_framework_json_api/serializers.py", line 99, in validate_path
    path

400 response (Insomnia)

{
  "errors": [
    {
      "detail": "This endpoint does not support the include parameter for path foo",
      "status": "400",
      "source": {
        "pointer": "/data"
      },
      "code": "parse_error"
    }
  ],
  "meta": {
    "version": "unversioned"
  }
}

Checklist

• [x] Certain that this is a bug (if unsure or you have a question use discussions instead)
• [ ] Code snippet or unit test added to reproduce bug
  • Should be reproducible in any minimal app with no include serializers configured

[sliverc]

thanks for reporting. While handling the exception a serializer is retrieved and initalizing the serializer causes the error to be raised again ending up in the 500 error message.

It is not a good idea to do validation in init... The logic of include path validation should be split up. The default included query parameters be checked in the serializer meta class and the include parameter validation moved to either QueryParameterValidationFilter or its own filter backend.

[yardensachs]

I see this error as well.

[avacaru]

class MyView(GenericAPIView):
# ...
def get_serializer_context(self):
    context = super().get_serializer_context()
    context["some_object"] = get_object_or_404(MyModel, id=self.kwargs["some_object_id"])
    return context

Do you see anything wrong with the above snippet? This is perfectly fine in DRF, but the DJA exception handler is causing an HTTP 500 instead of HTTP 404.

I think this line should be:

try:
    serializer = context["view"].get_serializer()
except (Http404, PermissionDenied): # maybe other exceptions, or generic?
    # clearly something went wrong during serializer init
    # so don't rely on the serializer fields because there was no validation done
    fields = dict()
# ...

[sliverc]

@avacaru Thanks for the additional info. The issue might not just be with the include param as initially reported here, but also the example you provided.

I have an idea, let me see whether I will get to it next week to look into it.

[sliverc]

def format_drf_errors(response, context, exc):
    errors = []
    # handle generic errors. ValidationError('test') in a view for example
    if isinstance(response.data, list):
        for message in response.data:
            errors.extend(format_error_object(message, "/data", response))
    # handle all errors thrown from serializers
    else:
        try:
            serializer = context["view"].get_serializer()
            fields = get_serializer_fields(serializer) or dict()
            relationship_fields = [
                format_field_name(name)
                for name, field in fields.items()
                if is_relationship_field(field)
            ]
        except Exception:
            # ignore potential errors when retrieving serializer
            # as it might shadow error which is currently being
            # formatted
            serializer = None

        for field, error in response.data.items():
            non_field_error = field == api_settings.NON_FIELD_ERRORS_KEY
            field = format_field_name(field)
            pointer = None
            if non_field_error:
                # Serializer error does not refer to a specific field.
                pointer = "/data"
            elif serializer:
                # pointer can be determined only if there's a serializer.
                rel = "relationships" if field in relationship_fields else "attributes"
                pointer = f"/data/{rel}/{field}"
            if isinstance(exc, Http404) and isinstance(error, str):
                # 404 errors don't have a pointer
                errors.extend(format_error_object(error, None, response))
            elif isinstance(error, str):
                classes = inspect.getmembers(exceptions, inspect.isclass)
                # DRF sets the `field` to 'detail' for its own exceptions
                if isinstance(exc, tuple(x[1] for x in classes)):
                    pointer = "/data"
                errors.extend(format_error_object(error, pointer, response))
            elif isinstance(error, list):
                errors.extend(format_error_object(error, pointer, response))
            else:
                errors.extend(format_error_object(error, pointer, response))

    context["view"].resource_name = "errors"
    response.data = errors

    return response

I think this should fix the issue. When it is not possible to get a serializer it will simply not write a pointer which is optional anyway.

Not sure when I get around creating a PR. If someone has time to do it, feel free. I think the most time will be to write a test with the invalid include parameter and the serializer_context example of @avacaru so we can make sure we do not introduce these errors again in the future.

[ntessman-capsule]

+1 on this issue, ran into this when attempting to upgrade from an older version.