Added protections to AttributeErrors raised within properties

Author: James Riley McHugh

rest_framework/request.py

@@ -78,6 +78,16 @@ def wrap_attributeerrors():
         raise exc.with_traceback(info[2])
 
 
+def safe_property(func):
+    """Property decorator to ensure AttributeErrors raised in properties are properly handled"""
+
+    @property
+    def new_func(self):
+        with wrap_attributeerrors():
+            return func(self)
+
+    return new_func
+
 class Empty:
     """
     Placeholder for unset attributes.
@@ -193,12 +203,12 @@ def __class_getitem__(cls, *args, **kwargs):
     def _default_negotiator(self):
         return api_settings.DEFAULT_CONTENT_NEGOTIATION_CLASS()
 
-    @property
+    @safe_property
     def content_type(self):
         meta = self._request.META
         return meta.get('CONTENT_TYPE', meta.get('HTTP_CONTENT_TYPE', ''))
 
-    @property
+    @safe_property
     def stream(self):
         """
         Returns an object that may be used to stream the request content.
@@ -207,28 +217,27 @@ def stream(self):
             self._load_stream()
         return self._stream
 
-    @property
+    @safe_property
     def query_params(self):
         """
         More semantically correct name for request.GET.
         """
         return self._request.GET
 
-    @property
+    @safe_property
     def data(self):
         if not _hasattr(self, '_full_data'):
             self._load_data_and_files()
         return self._full_data
 
-    @property
+    @safe_property
     def user(self):
         """
         Returns the user associated with the current request, as authenticated
         by the authentication classes provided to the request.
         """
         if not hasattr(self, '_user'):
-            with wrap_attributeerrors():
-                self._authenticate()
+            self._authenticate()
         return self._user
 
     @user.setter
@@ -244,15 +253,14 @@ def user(self, value):
         self._user = value
         self._request.user = value
 
-    @property
+    @safe_property
     def auth(self):
         """
         Returns any non-user authentication information associated with the
         request, such as an authentication token.
         """
         if not hasattr(self, '_auth'):
-            with wrap_attributeerrors():
-                self._authenticate()
+            self._authenticate()
         return self._auth
 
     @auth.setter
@@ -264,15 +272,14 @@ def auth(self, value):
         self._auth = value
         self._request.auth = value
 
-    @property
+    @safe_property
     def successful_authenticator(self):
         """
         Return the instance of the authentication instance class that was used
         to authenticate the request, or `None`.
         """
         if not hasattr(self, '_authenticator'):
-            with wrap_attributeerrors():
-                self._authenticate()
+            self._authenticate()
         return self._authenticator
 
     def _load_data_and_files(self):
@@ -420,9 +427,9 @@ def __getattr__(self, attr):
             _request = self.__getattribute__("_request")
             return getattr(_request, attr)
         except AttributeError:
-            return self.__getattribute__(attr)
+            raise AttributeError(f"'{self.__class__.__name__}' object has no attribute '{attr}'")
 
-    @property
+    @safe_property
     def POST(self):
         # Ensure that request.POST uses our request parsing.
         if not _hasattr(self, '_data'):
@@ -431,7 +438,7 @@ def POST(self):
             return self._data
         return QueryDict('', encoding=self._request._encoding)
 
-    @property
+    @safe_property
     def FILES(self):
         # Leave this one alone for backwards compat with Django's request.FILES
         # Different from the other two cases, which are not valid property

tests/test_request.py

@@ -347,6 +347,22 @@ def test_duplicate_request_form_data_access(self):
         assert request.content_type.startswith('multipart/form-data')
         assert response.data == {'a': ['b']}
 
+    def test_parser_attribute_error(self):
+        """Ensure attribute errors raised when parsing are properly re-raised"""
+        expected_message = "Internal error"
+
+        class BrokenParser:
+            media_type = "application/json"
+
+            def parse(self, *args, **kwargs):
+                raise AttributeError(expected_message)
+
+        http_request = factory.post('/', data={}, format="json")
+        request = Request(http_request, parsers=[BrokenParser()])
+
+        with self.assertRaisesMessage(WrappedAttributeError, expected_message):
+            request.data
+
 
 class TestDeepcopy(TestCase):