From d7bc37d611b43d58be4b430faf0b9813bcde29c6 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Fri, 9 Jan 2015 22:18:34 +0100 Subject: [PATCH] Fixed #24097 -- Prevented AttributeError in redirect_to_login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- 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 | 2 +- 6 files changed, 48 insertions(+), 8 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 d406b436b3..e5538517d0 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 re @@ -10,9 +13,9 @@ from django.contrib.auth import SESSION_KEY, REDIRECT_FIELD_NAME from django.contrib.auth.forms import (AuthenticationForm, PasswordChangeForm, SetPasswordForm) from django.contrib.auth.models import User -from django.contrib.auth.views import login as login_view +from django.contrib.auth.views import login as login_view, redirect_to_login 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.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text @@ -648,6 +651,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): @@ -673,6 +680,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 b43c23a891..3bb2412526 100644 --- a/django/shortcuts.py +++ b/django/shortcuts.py @@ -18,6 +18,8 @@ from django.db.models.query import QuerySet from django.core import urlresolvers from django.utils import six from django.utils.deprecation import RemovedInDjango20Warning +from django.utils.encoding import force_text +from django.utils.functional import Promise def render_to_response(template_name, context=None, @@ -182,6 +184,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 ce550ea846..0fc3307026 100644 --- a/tests/resolve_url/tests.py +++ b/tests/resolve_url/tests.py @@ -1,10 +1,11 @@ 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, ignore_warnings, override_settings from django.utils.deprecation import RemovedInDjango20Warning +from django.utils import six from .models import UnimportantThing @@ -56,6 +57,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) + @ignore_warnings(category=RemovedInDjango20Warning) def test_valid_view_name(self): """ diff --git a/tests/resolve_url/urls.py b/tests/resolve_url/urls.py index eb3720a503..e78b833927 100644 --- a/tests/resolve_url/urls.py +++ b/tests/resolve_url/urls.py @@ -2,5 +2,5 @@ from django.conf.urls import url from django.contrib.auth import views urlpatterns = [ - url(r'^accounts/logout/$', views.logout), + url(r'^accounts/logout/$', views.logout, name='logout'), ]