From 7e65876b7c34d0c43c94ba01543a0050270a34fe Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Fri, 9 Jan 2015 22:18:34 +0100 Subject: [PATCH] [1.7.x] Fixed #24097 -- Prevented AttributeError in redirect_to_login Thanks Peter Schmidt for the report and the initial patch. Thanks to Oktay Sancak for writing the original failing test and Alvin Savoy for supporting contributing back to the community. Backport of d7bc37d61 from master. --- django/contrib/auth/decorators.py | 5 +---- django/contrib/auth/tests/test_views.py | 26 +++++++++++++++++++++++-- django/shortcuts.py | 7 +++++++ docs/releases/1.7.3.txt | 4 ++++ tests/resolve_url/tests.py | 12 +++++++++++- tests/resolve_url/urls.py | 5 +++-- 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/django/contrib/auth/decorators.py b/django/contrib/auth/decorators.py index 12a79c0918..99e2983e63 100644 --- a/django/contrib/auth/decorators.py +++ b/django/contrib/auth/decorators.py @@ -3,7 +3,6 @@ from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME from django.core.exceptions import PermissionDenied from django.utils.decorators import available_attrs -from django.utils.encoding import force_str from django.utils.six.moves.urllib.parse import urlparse from django.shortcuts import resolve_url @@ -21,9 +20,7 @@ def user_passes_test(test_func, login_url=None, redirect_field_name=REDIRECT_FIE if test_func(request.user): return view_func(request, *args, **kwargs) path = request.build_absolute_uri() - # urlparse chokes on lazy objects in Python 3, force to str - resolved_login_url = force_str( - resolve_url(login_url or settings.LOGIN_URL)) + resolved_login_url = resolve_url(login_url or settings.LOGIN_URL) # If the login url is the same scheme and net location then just # use the path as the "next" url. login_scheme, login_netloc = urlparse(resolved_login_url)[:2] diff --git a/django/contrib/auth/tests/test_views.py b/django/contrib/auth/tests/test_views.py index 1a73af50c7..d9fbd0be44 100644 --- a/django/contrib/auth/tests/test_views.py +++ b/django/contrib/auth/tests/test_views.py @@ -1,3 +1,6 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + from importlib import import_module import itertools import os @@ -9,7 +12,7 @@ from django.contrib.sites.requests import RequestSite from django.contrib.admin.models import LogEntry from django.contrib.auth.models import User from django.core import mail -from django.core.urlresolvers import reverse, NoReverseMatch +from django.core.urlresolvers import NoReverseMatch, reverse, reverse_lazy from django.http import QueryDict, HttpRequest from django.utils.encoding import force_text from django.utils.http import urlquote @@ -27,7 +30,7 @@ from django.contrib.auth.forms import (AuthenticationForm, PasswordChangeForm, # Needed so model is installed when tests are run independently: from django.contrib.auth.tests.custom_user import CustomUser # NOQA from django.contrib.auth.tests.utils import skipIfCustomUser -from django.contrib.auth.views import login as login_view +from django.contrib.auth.views import login as login_view, redirect_to_login @override_settings( @@ -652,6 +655,10 @@ class LoginURLSettings(AuthViewsTestCase): expected = 'http://remote.example.com/login/?next=%s' % quoted_next self.assertLoginURLEquals(expected) + @override_settings(LOGIN_URL=reverse_lazy('login')) + def test_lazy_login_url(self): + self.assertLoginURLEquals('/login/?next=/login_required/') + @skipIfCustomUser class LoginRedirectUrlTest(AuthViewsTestCase): @@ -677,6 +684,21 @@ class LoginRedirectUrlTest(AuthViewsTestCase): self.assertLoginRedirectURLEqual('http://remote.example.com/welcome/') +class RedirectToLoginTests(AuthViewsTestCase): + """Tests for the redirect_to_login view""" + @override_settings(LOGIN_URL=reverse_lazy('login')) + def test_redirect_to_login_with_lazy(self): + login_redirect_response = redirect_to_login(next='/else/where/') + expected = '/login/?next=/else/where/' + self.assertEqual(expected, login_redirect_response.url) + + @override_settings(LOGIN_URL=reverse_lazy('login')) + def test_redirect_to_login_with_lazy_and_unicode(self): + login_redirect_response = redirect_to_login(next='/else/where/झ/') + expected = '/login/?next=/else/where/%E0%A4%9D/' + self.assertEqual(expected, login_redirect_response.url) + + @skipIfCustomUser class LogoutTest(AuthViewsTestCase): diff --git a/django/shortcuts.py b/django/shortcuts.py index 62560472c1..d99581389a 100644 --- a/django/shortcuts.py +++ b/django/shortcuts.py @@ -11,6 +11,8 @@ from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.core import urlresolvers from django.utils import six +from django.utils.encoding import force_text +from django.utils.functional import Promise def render_to_response(*args, **kwargs): @@ -148,6 +150,11 @@ def resolve_url(to, *args, **kwargs): if hasattr(to, 'get_absolute_url'): return to.get_absolute_url() + if isinstance(to, Promise): + # Expand the lazy instance, as it can cause issues when it is passed + # further to some Python functions like urlparse. + to = force_text(to) + if isinstance(to, six.string_types): # Handle relative URLs if any(to.startswith(path) for path in ('./', '../')): diff --git a/docs/releases/1.7.3.txt b/docs/releases/1.7.3.txt index 964c654920..cd0c1efdf7 100644 --- a/docs/releases/1.7.3.txt +++ b/docs/releases/1.7.3.txt @@ -20,3 +20,7 @@ Bugfixes * Fixed a crash in the CSRF middleware when handling non-ASCII referer header (:ticket:`23815`). + +* Fixed a crash in the ``django.contrib.auth.redirect_to_login`` view when + passing a :func:`~django.core.urlresolvers.reverse_lazy` result on Python 3 + (:ticket:`24097`). diff --git a/tests/resolve_url/tests.py b/tests/resolve_url/tests.py index 4fdf2a9fe1..8f7c497304 100644 --- a/tests/resolve_url/tests.py +++ b/tests/resolve_url/tests.py @@ -1,9 +1,10 @@ from __future__ import unicode_literals -from django.core.urlresolvers import NoReverseMatch +from django.core.urlresolvers import NoReverseMatch, reverse_lazy from django.contrib.auth.views import logout from django.shortcuts import resolve_url from django.test import TestCase +from django.utils import six from .models import UnimportantThing @@ -55,6 +56,15 @@ class ResolveUrlTests(TestCase): resolved_url = resolve_url(logout) self.assertEqual('/accounts/logout/', resolved_url) + def test_lazy_reverse(self): + """ + Tests that passing the result of reverse_lazy is resolved to a real URL + string. + """ + resolved_url = resolve_url(reverse_lazy('logout')) + self.assertIsInstance(resolved_url, six.text_type) + self.assertEqual('/accounts/logout/', resolved_url) + def test_valid_view_name(self): """ Tests that passing a view function to ``resolve_url`` will result in diff --git a/tests/resolve_url/urls.py b/tests/resolve_url/urls.py index f03ac1a412..d558a9f037 100644 --- a/tests/resolve_url/urls.py +++ b/tests/resolve_url/urls.py @@ -1,6 +1,7 @@ -from django.conf.urls import patterns +from django.conf.urls import patterns, url +from django.contrib.auth import views urlpatterns = patterns('', - (r'^accounts/logout/$', 'django.contrib.auth.views.logout') + url(r'^accounts/logout/$', views.logout, name='logout'), )