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.

3 files changed, 20 insertions, 3 deletions

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'