Django-rest-framework: Rendering of Nested Serializer fails if no dictionary is parsed

Created on 29 Jan 2021  路  10Comments  路  Source: encode/django-rest-framework

Checklist

  • [ ] I have verified that that issue exists against the master branch of Django REST framework.
  • [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • [x] This is not a usage question. (Those should be directed to the discussion group instead.)
  • [x] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • [x] I have reduced the issue to the simplest possible case.
  • [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

  1. Create a Serializer with a nested Serializer, e.g.:
class MySerializer(Serializer):
   payload = PayloadSerializer(default={}, write_only=True)

class PayloadSerializer(Serializer):
  some_field = CharField()
  1. Post a malformed payload that is not a dictionary:
{
   "payload": 1
}

Expected behavior

Nested Serializer should fail and throw a ValidationError, Browsable API should render empty fields.

Actual behavior

Server crashes in serializer_helpers.py at in as_form_field because integer is not iterable:

    def as_form_field(self):
        values = {}
        for key, value in self.value.items():
            if isinstance(value, (list, dict)):
                values[key] = value
            else:
                values[key] = '' if (value is None or value is False) else force_str(value)
        return self.__class__(self._field, values, self.errors, self._prefix)

My workaround, modify Parent serializer:

class PayloadNestedField(NestedBoundField):
    def __init__(self, field, value, errors, prefix=''):
        if value is not None and not isinstance(value, dict):
            value = {}
        super().__init__(field, value, errors, prefix)

class PayloadSerializer(Serializer):
   some_field = CharField()

   def __getitem__(self, key):
            field = self.fields[key]
            value = self.data.get(key)
            error = self.errors.get(key) if hasattr(self, '_errors') else None
            if key == 'payload':
                return PayloadNestedField(field, value, error)
            else:
                return super().__getitem__(key)

I think the issue here is that NestedBoundField only defaults the value to {} if it is None or the empty string here:

class NestedBoundField(BoundField):
    """
    This `BoundField` additionally implements __iter__ and __getitem__
    in order to support nested bound fields. This class is the type of
    `BoundField` that is used for serializer fields.
    """

    def __init__(self, field, value, errors, prefix=''):
        if value is None or value == '':
            value = {}
        super().__init__(field, value, errors, prefix)

All 10 comments

Hello @s4ke,

I was checking this issue and I tried to reproduce it with the following snippet:

from rest_framework import serializers 

class PayloadSerializer(serializers.Serializer): 
    some_field = serializers.CharField() 

class MySerializer(serializers.Serializer): 
    payload = PayloadSerializer(default={}, write_only=True) 

data = {'payload': 1}

serializer = MySerializer(data=data)
serializer.is_valid(raise_exception=True)

The following Exception is raised:

ValidationError: {'payload': {'non_field_errors': [ErrorDetail(string='Invalid data. Expected a dictionary, but got int.', code='invalid')]}}

I have tested this against the head of master and I have tested it against the version 3.12.2 that was already installed on my machine.

Do I miss anything in the snippet or in the reproduction steps?

This actually only happens in the browsable UI when rendering and is the reason we have not managed to reproduce it in our test suite. Re-reading the issue, I think I forgot to add this here.

I understand. Since it's also related with browsable UI, thus the endpoint, would it be possible to give more information regarding the views that can be used to reproduce this? Or does the issue happen with any Viewset extending APIView, GenericView, ModelViewSet, etc.?

This happens for GenericViewSet with the default html template.

I have created the following View that uses the serializers from the issue definition.

class NestedIssueView(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
    serializer_class = MySerializer
    queryset = Issue.objects.all()

    def testing(self, request, format=None):
        serializer = MySerializer(data=request.data)
        serializer.is_valid(raise_exception=True)
        return Response(serializer.data, status=status.HTTP_201_CREATED)

I have added it to the urls.py as following:

urlpatterns = [
    path('admin/', admin.site.urls),
    re_path('^issues/', NestedIssueView.as_view({'post': 'testing'}))
]

I sent the payload which can be seen in the following screenshot.
image

I am still receiving the validation error.
image

Could you maybe give more details to be able to reproduce the issue?

I will try to reproduce the error locally on our codebase and get back to you.

This is the error I get. I just checked with djangorestframework 3.12.2 and it also happens:

image

Let me try to extract more from our code.

Here is the stacktrace:

Traceback (most recent call last):
  File "/base/venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/base/venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 202, in _get_response
    response = response.render()
  File "/base/venv/lib/python3.8/site-packages/django/template/response.py", line 105, in render
    self.content = self.rendered_content
  File "/base/venv/lib/python3.8/site-packages/rest_framework/response.py", line 70, in rendered_content
    ret = renderer.render(self.data, accepted_media_type, context)
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 724, in render
    context = self.get_context(data, accepted_media_type, renderer_context)
  File "/base/<path-to-custom-renderer-that-only-overrides get_context>/renderers.py", line 25, in get_context
    base_ctx = super().get_context(data, accepted_media_type, renderer_context)
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 696, in get_context
    'post_form': self.get_rendered_html_form(data, view, 'POST', request),
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 493, in get_rendered_html_form
    return self.render_form_for_serializer(existing_serializer)
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 518, in render_form_for_serializer
    return form_renderer.render(
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 372, in render
    return template.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/backends/django.py", line 61, in render
    return self.template.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 170, in render
    return self._render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 162, in _render
    return self.nodelist.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 938, in render
    bit = node.render_annotated(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 905, in render_annotated
    return self.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/defaulttags.py", line 211, in render
    nodelist.append(node.render_annotated(context))
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 905, in render_annotated
    return self.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/defaulttags.py", line 312, in render
    return nodelist.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 938, in render
    bit = node.render_annotated(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/base.py", line 905, in render_annotated
    return self.render(context)
  File "/base/venv/lib/python3.8/site-packages/django/template/library.py", line 192, in render
    output = self.func(*resolved_args, **resolved_kwargs)
  File "/base/venv/lib/python3.8/site-packages/rest_framework/templatetags/rest_framework.py", line 87, in render_field
    return renderer.render_field(field, style)
  File "/base/venv/lib/python3.8/site-packages/rest_framework/renderers.py", line 339, in render_field
    field = field.as_form_field()
  File "/base/venv/lib/python3.8/site-packages/rest_framework/utils/serializer_helpers.py", line 122, in as_form_field
    for key, value in self.value.items():
AttributeError: 'int' object has no attribute 'items'

Here is the customized renderer we use (should not affect this, but nonetheless):

class CustomizableBrowsableAPIRenderer(BrowsableAPIRenderer):
    def get_context(self, data: Any, accepted_media_type: str, renderer_context: Mapping[str, Any]) -> Dict[str, Any]:
        base_ctx = super().get_context(data, accepted_media_type, renderer_context)

        view = renderer_context['view']
        if hasattr(view, 'get_description_string'):
            try:
                base_ctx['description'] = view.get_description_string()
            except APIException:
                pass
            except Http404:
                pass

        if hasattr(view, 'get_name_string'):
            try:
                base_ctx['name'] = view.get_name_string()
            except APIException:
                # PermissionDenied, NotFound are really relevant here, but
                # its better to not display anything than failing completely
                pass
            except Http404:
                pass

        return base_ctx

The PayloadSerializer class looks like this and is actually dynamically configured (see get_fields):

    class PayloadSerializer(serializers.Serializer):

        def create(self, validated_data: Any) -> Any:
            # we dont use this
            raise AssertionError()

        def update(self, instance: Any, validated_data: Any) -> Any:
            # we dont use this
            raise AssertionError()

        def to_internal_value(self, data: Dict[str, Any]) -> Dict[str, Any]:
            # from the base class, we need to check this before
            if not isinstance(data, Mapping):
                message = self.error_messages['invalid'].format(
                    datatype=type(data).__name__
                )
                raise ValidationError({
                    api_settings.NON_FIELD_ERRORS_KEY: [message]
                }, code='invalid')

            own_keys = payload_serializers.keys()
            extra_keys = set(data.keys()).difference(own_keys)

            if len(extra_keys) > 0:
                if data_series.allow_extra_fields:
                    for _extra_key in extra_keys:
                        if _extra_key in data:
                            # make sure downstream does not get the data
                            # if we ignore it
                            del data[_extra_key]
                else:
                    raise ValidationError(
                        f'{str(extra_keys)} were set, but were not recognized'
                    )

            return super().to_internal_value(data)  # type: ignore

        def get_fields(self) -> Dict[str, serializers.Field]:
            return payload_serializers

I was able to reproduce the issue just now. I have tried with CustomizableBrowsableAPIRenderer. I was not able to reproduce it. The code was executed in a bit different direction than was in the stacktrace shared above. I was using the following parser class configuration:

    'DEFAULT_PARSER_CLASSES': [
        'rest_framework.parsers.JSONParser',
    ],

I have changed it to the following:

    'DEFAULT_PARSER_CLASSES': [
        'rest_framework.parsers.JSONParser',
        'rest_framework.parsers.FormParser',
        'rest_framework.parsers.MultiPartParser',
    ],

Now, I was able to reproduce the issue. Then I have dropped CustomizableBrowsableAPIRenderer and used the regular BrowsableAPIRenderer. I am still able to reproduce the issue. Thank you for the detailed information. Now I will work on the issue.

Ah, something I also forgot. With this class it works for us:

class PayloadNestedField(NestedBoundField):
    def __init__(self, field, value, errors, prefix=''):  # type: ignore
        if value is not None and not isinstance(value, Mapping):
            value = {}
        super().__init__(field, value, errors, prefix)

We monkey patch this into the parent serializer class like so:

        def __getitem__(self, key):  # type: ignore
            # hack, return a PayloadNestedField to make rendering the browsable API not crash on non dict values
            field = self.fields[key]
            value = self.data.get(key)
            error = self.errors.get(key) if hasattr(self, '_errors') else None
            if key == 'payload':
                return PayloadNestedField(field, value, error)  # type: ignore
            else:
                return super().__getitem__(key)

I have created one PR to reproduce the issue via a test: #7723

I was checking the options on how to fix it. I believe your solution is the most proper one. Thus, I added it and one test regarding it.
I have created another PR to fix the issue: #7724

Was this page helpful?
0 / 5 - 0 ratings