Skip to content

Commit

Permalink
[1.7.x] Fixed #24097 -- Prevented AttributeError in redirect_to_login
Browse files Browse the repository at this point in the history
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 d7bc37d from master.
  • Loading branch information
claudep committed Jan 10, 2015
1 parent bbcbacf commit 7e65876
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 9 deletions.
5 changes: 1 addition & 4 deletions django/contrib/auth/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
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

Expand All @@ -21,9 +20,7 @@ def _wrapped_view(request, *args, **kwargs):
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]
Expand Down
26 changes: 24 additions & 2 deletions django/contrib/auth/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from importlib import import_module
import itertools
import os
Expand All @@ -9,7 +12,7 @@
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
Expand All @@ -27,7 +30,7 @@
# 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(
Expand Down Expand Up @@ -652,6 +655,10 @@ def test_remote_login_url_with_next_querystring(self):
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):
Expand All @@ -677,6 +684,21 @@ def test_remote(self):
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):

Expand Down
7 changes: 7 additions & 0 deletions django/shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
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):
Expand Down Expand Up @@ -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 ('./', '../')):
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/1.7.3.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
12 changes: 11 additions & 1 deletion tests/resolve_url/tests.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -55,6 +56,15 @@ def test_view_function(self):
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
Expand Down
5 changes: 3 additions & 2 deletions tests/resolve_url/urls.py
Original file line number Diff line number Diff line change
@@ -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'),
)

0 comments on commit 7e65876

Please sign in to comment.