Attribute Errors raised in Parser Silently Ignored

[james-mchugh]

Hello.

First, thank you for the hard work on DRF. It has been great workin with it.

There seems to be an issue where an AttributeError raised in a parser is being swallowed by the Request class's __getattr__ method. I've noticed a similar issue reported previously (#4896), but it was closed as the reporter associated with an error from a separate package.

I encountered this because I wrote the server and tested using Python 3.11, and another developer attempted to test using 3.10. My custom parser used a function that was not available in Python 3.10 (hashlib.file_digest), which caused an AttributeError when hitting certain endpoints. Instead of seeing a 500 response and a traceback in the server logs, we were getting a 200 response with an empty dictionary in the response body. We were scratching our heads for a while, as there was no indication anything was going wrong.

Environment

OS: Darwin 23.4.0 Darwin Kernel Version 23.4.0 x86_64
Python: 3.10 + 3.11
DRF: 3.15.1
Django: 5.0.6

Expected Behavior

500 Internal server error response and traceback in server logs.

Actual Behavior

200 response and no errors in server logs.

$ curl --json '{"foo": "bar"}' http://localhost:8000/foos/
{}

[04/Jun/2024 02:59:15] "POST /foos/ HTTP/1.1" 200 2

Code to Reproduce

from rest_framework import viewsets, response, parsers, routers


class BrokenParser(parsers.JSONParser):

    def parse(self, stream, media_type=None, parser_context=None):
        raise AttributeError


class TestViewSet(viewsets.ViewSet):

    parser_classes = (BrokenParser,)

    def create(self, request, **kwargs):
        return response.Response(request.data)

router = routers.DefaultRouter()
router.register("foos", TestViewSet, "foo")

urlpatterns = router.urls

Investigation

This appears to be happening because accessing the data property lazily parses data. If the parsing raises an AttributeError, this is raised up to https://github.com/encode/django-rest-framework/blob/master/rest_framework/request.py#L359 where self._full_data is set to an empty dictionary before the error is re-raised.

This error then raises up and causes the attribute access to fallback to the Request.__getattr__ method via Python's data model. Here, the data attribute is attempted to be retrieved from the WSGIRequest stored in Request._request, and once again, this raises an AttributeError which is caught by https://github.com/encode/django-rest-framework/blob/master/rest_framework/request.py#L423. From here, the original Request.__getattribute__ handling runs again. Now when the data getter runs, self._full_data is already set to an empty dictionary which is then returned.

In the end, an empty response is returned and the error is silently ignored.

Noob question, but why does Request.__getattr__ call self.__getattribute__ when an AttributeError occurs? Based on the Python Data model linked above, __getattr__ should only run if __getattribute__ fails in the first place, so by the time __getattr__ runs, __getattribute__ already tried and failed.

[james-mchugh]

As this hasn't really gotten any attention from the maintainers, I have created an issue (#9433) regarding this bug. Since the issue was created, two PRs have been opened to attempt to resolve it. #9453 was opened by a DRF contributor and #9455 was opened by myself. I'm going to go ahead and just say this is a confirmed bug at this point.

As for my final question:

Noob question, but why does Request.getattr call self.getattribute when an AttributeError occurs? Based on the Python Data model linked above, getattr should only run if getattribute fails in the first place, so by the time getattr runs, getattribute already tried and failed.

This appears to have been done to recreate the original AttributeError that occurred the first time __getattribute__ runs, as described here. At the time, I'm assuming the side-effects of doing this (namely re-running property getters that already ran) were not taken into account. A more detailed comment describing these side-effects can be found here.