Commit 056a3c6c authored by wrwrwr's avatar wrwrwr Committed by Tim Graham
Browse files

Fixed #23682 -- Enhanced circular redirects detection in tests. 

When the test client detects a redirect to a URL seen in the
currently followed chain it will now raise a RedirectCycleError
instead of just returning the first repeated response.

It will also complain when a single chain of redirects is longer
than 20, as this often means a redirect loop with varying URLs,
and even if it's not actually one, such long chains are likely
to be treated as loops by browsers.

Thanks Preston Timmons, Berker Peksag, and Tim Graham for reviews.
parent a973fb2d

django/test/client.py

+24 −7
Original line number Diff line number Diff line
@@ -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

docs/ref/exceptions.txt

+14 −0
Original line number Diff line number Diff line
@@ -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

=================

docs/releases/1.8.txt

+4 −0
Original line number Diff line number Diff line
@@ -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

tests/test_client_regress/tests.py

+12 −3
Original line number Diff line number Diff line
@@ -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)

tests/test_client_regress/urls.py

+1 −0
Original line number Diff line number Diff line
@@ -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/')),