Merge pull request #2530 from tomchristie/attribute-proxying-fix

Fix misleading `AttributeError` tracebacks on `Request` objects.

9 files changed, 73 insertions, 27 deletions

diff --git a/.gitignore b/.gitignore
index 2bdf8f7e..3d5f1043 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,18 +3,14 @@
 *~
 .*
 
-site/
-htmlcov/
-coverage/
-build/
-dist/
-*.egg-info/
+/site/
+/htmlcov/
+/coverage/
+/build/
+/dist/
+/*.egg-info/
+/env/
 MANIFEST
 
-bin/
-include/
-lib/
-local/
-
 !.gitignore
 !.travis.yml
diff --git a/rest_framework/request.py b/rest_framework/request.py
index cfbbdecc..c4de9424 100644
--- a/rest_framework/request.py
+++ b/rest_framework/request.py
@@ -12,12 +12,13 @@ from __future__ import unicode_literals
 from django.conf import settings
 from django.http import QueryDict
 from django.http.multipartparser import parse_header
+from django.utils import six
 from django.utils.datastructures import MultiValueDict
 from django.utils.datastructures import MergeDict as DjangoMergeDict
-from django.utils.six import BytesIO
 from rest_framework import HTTP_HEADER_ENCODING
 from rest_framework import exceptions
 from rest_framework.settings import api_settings
+import sys
 import warnings
 
 
@@ -362,7 +363,7 @@ class Request(object):
         elif hasattr(self._request, 'read'):
             self._stream = self._request
         else:
-            self._stream = BytesIO(self.raw_post_data)
+            self._stream = six.BytesIO(self.raw_post_data)
 
     def _perform_form_overloading(self):
         """
@@ -404,7 +405,7 @@ class Request(object):
             self._CONTENTTYPE_PARAM in self._data
         ):
             self._content_type = self._data[self._CONTENTTYPE_PARAM]
-            self._stream = BytesIO(self._data[self._CONTENT_PARAM].encode(self.parser_context['encoding']))
+            self._stream = six.BytesIO(self._data[self._CONTENT_PARAM].encode(self.parser_context['encoding']))
             self._data, self._files, self._full_data = (Empty, Empty, Empty)
 
     def _parse(self):
@@ -485,8 +486,16 @@ class Request(object):
         else:
             self.auth = None
 
-    def __getattr__(self, attr):
+    def __getattribute__(self, attr):
         """
-        Proxy other attributes to the underlying HttpRequest object.
+        If an attribute does not exist on this instance, then we also attempt
+        to proxy it to the underlying HttpRequest object.
         """
-        return getattr(self._request, attr)
+        try:
+            return super(Request, self).__getattribute__(attr)
+        except AttributeError:
+            info = sys.exc_info()
+            try:
+                return getattr(self._request, attr)
+            except AttributeError:
+                six.reraise(info[0], info[1], info[2].tb_next)
diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py
index 69e03af4..d66ffb33 100644
--- a/rest_framework/templatetags/rest_framework.py
+++ b/rest_framework/templatetags/rest_framework.py
@@ -154,7 +154,9 @@ def urlize_quoted_links(text, trim_url_limit=None, nofollow=True, autoescape=Tru
 
     If autoescape is True, the link text and URLs will get autoescaped.
     """
-    trim_url = lambda x, limit=trim_url_limit: limit is not None and (len(x) > limit and ('%s...' % x[:max(0, limit - 3)])) or x
+    def trim_url(x, limit=trim_url_limit):
+        return limit is not None and (len(x) > limit and ('%s...' % x[:max(0, limit - 3)])) or x
+
     safe_input = isinstance(text, SafeData)
     words = word_split_re.split(force_text(text))
     for i, word in enumerate(words):
diff --git a/tests/test_authentication.py b/tests/test_authentication.py
index caabcc21..19fe6043 100644
--- a/tests/test_authentication.py
+++ b/tests/test_authentication.py
@@ -205,7 +205,10 @@ class TokenAuthTests(TestCase):
     def test_post_json_makes_one_db_query(self):
         """Ensure that authenticating a user using a token performs only one DB query"""
         auth = "Token " + self.key
-        func_to_test = lambda: self.csrf_client.post('/token/', {'example': 'example'}, format='json', HTTP_AUTHORIZATION=auth)
+
+        def func_to_test():
+            return self.csrf_client.post('/token/', {'example': 'example'}, format='json', HTTP_AUTHORIZATION=auth)
+
         self.assertNumQueries(1, func_to_test)
 
     def test_post_form_failing_token_auth(self):
diff --git a/tests/test_relations_hyperlink.py b/tests/test_relations_hyperlink.py
index f1b882ed..2230c275 100644
--- a/tests/test_relations_hyperlink.py
+++ b/tests/test_relations_hyperlink.py
@@ -12,7 +12,9 @@ factory = APIRequestFactory()
 request = factory.get('/')  # Just to ensure we have a request in the serializer context
 
 
-dummy_view = lambda request, pk: None
+def dummy_view(request, pk):
+    pass
+
 
 urlpatterns = patterns(
     '',
diff --git a/tests/test_renderers.py b/tests/test_renderers.py
index 54eea8ce..4f41144e 100644
--- a/tests/test_renderers.py
+++ b/tests/test_renderers.py
@@ -28,8 +28,13 @@ import re
 DUMMYSTATUS = status.HTTP_200_OK
 DUMMYCONTENT = 'dummycontent'
 
-RENDERER_A_SERIALIZER = lambda x: ('Renderer A: %s' % x).encode('ascii')
-RENDERER_B_SERIALIZER = lambda x: ('Renderer B: %s' % x).encode('ascii')
+
+def RENDERER_A_SERIALIZER(x):
+    return ('Renderer A: %s' % x).encode('ascii')
+
+
+def RENDERER_B_SERIALIZER(x):
+    return ('Renderer B: %s' % x).encode('ascii')
 
 
 expected_results = [
diff --git a/tests/test_request.py b/tests/test_request.py
index 02a9b1e2..c274ab69 100644
--- a/tests/test_request.py
+++ b/tests/test_request.py
@@ -249,9 +249,29 @@ class TestUserSetter(TestCase):
         login(self.request, self.user)
         self.assertEqual(self.wrapped_request.user, self.user)
 
+    def test_calling_user_fails_when_attribute_error_is_raised(self):
+        """
+        This proves that when an AttributeError is raised inside of the request.user
+        property, that we can handle this and report the true, underlying error.
+        """
+        class AuthRaisesAttributeError(object):
+            def authenticate(self, request):
+                import rest_framework
+                rest_framework.MISSPELLED_NAME_THAT_DOESNT_EXIST
 
-class TestAuthSetter(TestCase):
+        self.request = Request(factory.get('/'), authenticators=(AuthRaisesAttributeError(),))
+        SessionMiddleware().process_request(self.request)
 
+        login(self.request, self.user)
+        try:
+            self.request.user
+        except AttributeError as error:
+            self.assertEqual(str(error), "'module' object has no attribute 'MISSPELLED_NAME_THAT_DOESNT_EXIST'")
+        else:
+            assert False, 'AttributeError not raised'
+
+
+class TestAuthSetter(TestCase):
     def test_auth_can_be_set(self):
         request = Request(factory.get('/'))
         request.auth = 'DUMMY'
diff --git a/tests/test_response.py b/tests/test_response.py
index f233ae33..4a9deaa2 100644
--- a/tests/test_response.py
+++ b/tests/test_response.py
@@ -38,8 +38,13 @@ class MockTextMediaRenderer(BaseRenderer):
 DUMMYSTATUS = status.HTTP_200_OK
 DUMMYCONTENT = 'dummycontent'
 
-RENDERER_A_SERIALIZER = lambda x: ('Renderer A: %s' % x).encode('ascii')
-RENDERER_B_SERIALIZER = lambda x: ('Renderer B: %s' % x).encode('ascii')
+
+def RENDERER_A_SERIALIZER(x):
+    return ('Renderer A: %s' % x).encode('ascii')
+
+
+def RENDERER_B_SERIALIZER(x):
+    return ('Renderer B: %s' % x).encode('ascii')
 
 
 class RendererA(BaseRenderer):
diff --git a/tests/test_throttling.py b/tests/test_throttling.py
index cc36a004..50a53b3e 100644
--- a/tests/test_throttling.py
+++ b/tests/test_throttling.py
@@ -188,7 +188,9 @@ class ScopedRateThrottleTests(TestCase):
         class XYScopedRateThrottle(ScopedRateThrottle):
             TIMER_SECONDS = 0
             THROTTLE_RATES = {'x': '3/min', 'y': '1/min'}
-            timer = lambda self: self.TIMER_SECONDS
+
+            def timer(self):
+                return self.TIMER_SECONDS
 
         class XView(APIView):
             throttle_classes = (XYScopedRateThrottle,)
@@ -290,7 +292,9 @@ class XffTestingBase(TestCase):
         class Throttle(ScopedRateThrottle):
             THROTTLE_RATES = {'test_limit': '1/day'}
             TIMER_SECONDS = 0
-            timer = lambda self: self.TIMER_SECONDS
+
+            def timer(self):
+                return self.TIMER_SECONDS
 
         class View(APIView):
             throttle_classes = (Throttle,)