Skip to content

Fix misleading AttributeError tracebacks on Request objects. #2530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
21 changes: 15 additions & 6 deletions rest_framework/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
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


Expand Down Expand Up @@ -362,7 +363,7 @@ def _load_stream(self):
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):
"""
Expand Down Expand Up @@ -404,7 +405,7 @@ def _perform_form_overloading(self):
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):
Expand Down Expand Up @@ -485,8 +486,16 @@ def _not_authenticated(self):
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)
4 changes: 3 additions & 1 deletion rest_framework/templatetags/rest_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I'd feel better if these conditional and y or x lines were replaced with explicit Python conditional assignment statements.

return ('%s...' % x[:max(0, limit - 3)]) if limit and len(x) > limit else x

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. We're just cloning from the Django codebase here so I don't particularly mind that it's a bit rubbish in this case.


safe_input = isinstance(text, SafeData)
words = word_split_re.split(force_text(text))
for i, word in enumerate(words):
Expand Down
5 changes: 4 additions & 1 deletion tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ def test_post_json_passing_token_auth(self):
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):
Expand Down
4 changes: 3 additions & 1 deletion tests/test_relations_hyperlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
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(
'',
Expand Down
9 changes: 7 additions & 2 deletions tests/test_renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,13 @@
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 = [
Expand Down
22 changes: 21 additions & 1 deletion tests/test_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,29 @@ def test_logged_in_user_is_set_on_wrapped_request(self):
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'
Expand Down
9 changes: 7 additions & 2 deletions tests/test_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
8 changes: 6 additions & 2 deletions tests/test_throttling.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ def setUp(self):
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,)
Expand Down Expand Up @@ -290,7 +292,9 @@ def setUp(self):
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,)
Expand Down