Skip to content

Commit

Permalink
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.
  • Loading branch information
claudep committed Jan 10, 2015
1 parent f5c3a8b commit d7bc37d
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 8 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 re
Expand All @@ -10,9 +13,9 @@
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
Expand Down Expand Up @@ -648,6 +651,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 @@ -673,6 +680,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 @@ -18,6 +18,8 @@
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,
Expand Down Expand Up @@ -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 ('./', '../')):
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,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

Expand Down Expand Up @@ -56,6 +57,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)

@ignore_warnings(category=RemovedInDjango20Warning)
def test_valid_view_name(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/resolve_url/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
from django.contrib.auth import views

urlpatterns = [
url(r'^accounts/logout/$', views.logout),
url(r'^accounts/logout/$', views.logout, name='logout'),
]

0 comments on commit d7bc37d

Please sign in to comment.