diff --git a/django/test/client.py b/django/test/client.py index fb8ff75e95..b2633bd7e7 100644 --- a/django/test/client.py +++ b/django/test/client.py @@ -27,7 +27,7 @@ from django.utils import six from django.utils.six.moves.urllib.parse import urlparse, urlsplit from django.test.utils import ContextList -__all__ = ('Client', 'RequestFactory', 'encode_file', 'encode_multipart') +__all__ = ('Client', 'RedirectCycleError', 'RequestFactory', 'encode_file', 'encode_multipart') BOUNDARY = 'BoUnDaRyStRiNg' @@ -35,6 +35,16 @@ MULTIPART_CONTENT = 'multipart/form-data; boundary=%s' % BOUNDARY CONTENT_TYPE_RE = re.compile('.*; charset=([\w\d-]+);?') +class RedirectCycleError(Exception): + """ + The test client has been asked to follow a redirect loop. + """ + def __init__(self, message, last_response): + super(RedirectCycleError, self).__init__(message) + self.last_response = last_response + self.redirect_chain = last_response.redirect_chain + + class FakePayload(object): """ A wrapper around BytesIO that restricts what can be read since data from @@ -630,11 +640,11 @@ class Client(RequestFactory): response.redirect_chain = [] while response.status_code in (301, 302, 303, 307): - url = response.url + response_url = response.url redirect_chain = response.redirect_chain - redirect_chain.append((url, response.status_code)) + redirect_chain.append((response_url, response.status_code)) - url = urlsplit(url) + url = urlsplit(response_url) if url.scheme: extra['wsgi.url_scheme'] = url.scheme if url.hostname: @@ -645,7 +655,14 @@ class Client(RequestFactory): response = self.get(url.path, QueryDict(url.query), follow=False, **extra) response.redirect_chain = redirect_chain - # Prevent loops - if response.redirect_chain[-1] in response.redirect_chain[0:-1]: - break + if redirect_chain[-1] in redirect_chain[:-1]: + # Check that we're not redirecting to somewhere we've already + # been to, to prevent loops. + raise RedirectCycleError("Redirect loop detected.", last_response=response) + if len(redirect_chain) > 20: + # Such a lengthy chain likely also means a loop, but one with + # a growing path, changing view, or changing query argument; + # 20 is the value of "network.http.redirection-limit" from Firefox. + raise RedirectCycleError("Too many redirects.", last_response=response) + return response diff --git a/docs/ref/exceptions.txt b/docs/ref/exceptions.txt index e74e0991e1..bbd0229610 100644 --- a/docs/ref/exceptions.txt +++ b/docs/ref/exceptions.txt @@ -217,6 +217,20 @@ Transaction exceptions are defined in :mod:`django.db.transaction`. The :exc:`TransactionManagementError` is raised for any and all problems related to database transactions. +.. currentmodule:: django.test + +Testing Framework Exceptions +============================ + +Exceptions provided by the :mod:`django.test` package. + +.. exception:: client.RedirectCycleError + + .. versionadded:: 1.8 + + :exc:`~client.RedirectCycleError` is raised when the test client detects a + loop or an overly long chain of redirects. + Python Exceptions ================= diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index ba0b0cfa2b..73ad244cfb 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -799,6 +799,10 @@ Miscellaneous when both the ``fields`` and ``form_class`` attributes are specified. Previously, ``fields`` was silently ignored. +* When following redirects, the test client now raises + :exc:`~django.test.client.RedirectCycleError` if it detects a loop or hits a + maximum redirect limit (rather than passing silently). + .. _deprecated-features-1.8: Features deprecated in 1.8 diff --git a/tests/test_client_regress/tests.py b/tests/test_client_regress/tests.py index 63e60ee8bd..0d63ee9429 100644 --- a/tests/test_client_regress/tests.py +++ b/tests/test_client_regress/tests.py @@ -10,7 +10,7 @@ import itertools from django.core.urlresolvers import reverse, NoReverseMatch from django.template import TemplateSyntaxError, Context, Template from django.test import Client, TestCase, override_settings -from django.test.client import encode_file, RequestFactory +from django.test.client import RedirectCycleError, RequestFactory, encode_file from django.test.utils import ContextList, str_prefix from django.template.response import SimpleTemplateResponse from django.utils._os import upath @@ -377,15 +377,24 @@ class AssertRedirectsTests(TestCase): def test_redirect_chain_to_self(self): "Redirections to self are caught and escaped" - response = self.client.get('/redirect_to_self/', {}, follow=True) + with self.assertRaises(RedirectCycleError) as context: + self.client.get('/redirect_to_self/', {}, follow=True) + response = context.exception.last_response # The chain of redirects stops once the cycle is detected. self.assertRedirects(response, '/redirect_to_self/', status_code=301, target_status_code=301) self.assertEqual(len(response.redirect_chain), 2) + def test_redirect_to_self_with_changing_query(self): + "Redirections don't loop forever even if query is changing" + with self.assertRaises(RedirectCycleError): + self.client.get('/redirect_to_self_with_changing_query_view/', {'counter': '0'}, follow=True) + def test_circular_redirect(self): "Circular redirect chains are caught and escaped" - response = self.client.get('/circular_redirect_1/', {}, follow=True) + with self.assertRaises(RedirectCycleError) as context: + self.client.get('/circular_redirect_1/', {}, follow=True) + response = context.exception.last_response # The chain of redirects will get back to the starting point, but stop there. self.assertRedirects(response, '/circular_redirect_2/', status_code=301, target_status_code=301) diff --git a/tests/test_client_regress/urls.py b/tests/test_client_regress/urls.py index 1bd0e0bc5b..a69a774555 100644 --- a/tests/test_client_regress/urls.py +++ b/tests/test_client_regress/urls.py @@ -21,6 +21,7 @@ urlpatterns = [ url(r'^redirect_to_non_existent_view/$', RedirectView.as_view(url='/non_existent_view/')), url(r'^redirect_to_non_existent_view2/$', RedirectView.as_view(url='/redirect_to_non_existent_view/')), url(r'^redirect_to_self/$', RedirectView.as_view(url='/redirect_to_self/')), + url(r'^redirect_to_self_with_changing_query_view/$', views.redirect_to_self_with_changing_query_view), url(r'^circular_redirect_1/$', RedirectView.as_view(url='/circular_redirect_2/')), url(r'^circular_redirect_2/$', RedirectView.as_view(url='/circular_redirect_3/')), url(r'^circular_redirect_3/$', RedirectView.as_view(url='/circular_redirect_1/')), diff --git a/tests/test_client_regress/views.py b/tests/test_client_regress/views.py index 656d7a4651..b752d1b9f7 100644 --- a/tests/test_client_regress/views.py +++ b/tests/test_client_regress/views.py @@ -11,6 +11,7 @@ from django.template.loader import render_to_string from django.test import Client from django.test.client import CONTENT_TYPE_RE from django.test.utils import setup_test_environment +from django.utils.six.moves.urllib.parse import urlencode class CustomTestException(Exception): @@ -85,6 +86,12 @@ def login_protected_redirect_view(request): login_protected_redirect_view = login_required(login_protected_redirect_view) +def redirect_to_self_with_changing_query_view(request): + query = request.GET.copy() + query['counter'] += '0' + return HttpResponseRedirect('/redirect_to_self_with_changing_query_view/?%s' % urlencode(query)) + + def set_session_view(request): "A view that sets a session variable" request.session['session_var'] = 'YES'