Stronger redirect loop detection in the test client

[Wojtek Ruszczewski]

The test client has a mechanism to detect circular redirects. I'd like to propose two changes to it;

• when a loop is detected, throw an exception rather than break silently;
• detect and complain about very (possibly infinitely) long redirect chains with differing URLs (at least when they only differ by a view argument value or query string).

​A few tests that are not part of the proposed changes, but demonstrate how one could try to test for loops (currently 3 of these just past, while a browser would run into a loop, and 2 actually loop indefinitely).

[Wojtek Ruszczewski]

​Proposed changes:

• defined a new RedirectCycleError;
• changed the loop detection to throw the exception on repeated URLs and chains longer than 20;
• added a "redirect to self" test with a changing query argument.

Could all the loop prevention tests maybe live in test_client rather than test_client_regress?

[Tim Graham]

It seems reasonable to me as infinite redirects could indeed be something a developer would want to be notified about. Skimming the original ticket where this was added (#4476), I couldn't find anything about why the decision was made.

Combining the test_client and test_client_regress tests would probably be welcome, but making it a separate commit/ticket is best.

[Preston Timmons]

I reviewed and tested this patch. It looks good to me, although the import statements should reflect Django's convention of being alphabetized.

Do you want to create a pull request from your branch? This has the nice side-effect of triggering an automated test run and verifying the merge status.

[Wojtek Ruszczewski]

Thanks. Did some imports reordering (see also #23860) and created ​a pull request.

[Preston Timmons]

Thanks for adding the pull request. I left a few comments on it.

[Wojtek Ruszczewski]

Thanks again; rebased, undid the unnecessary reordering. added docs on the new exception and a point to the 1.8 release notes.

[Preston Timmons]

This patch looks good to me.

[Tim Graham <timograham@…>]

In 056a3c6c374f15e23746ea8568cd5b11bfe7d442:

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.