Redirect loop detection in test client is incorrect

[Mounir Messelmeni]

This is related to the ticket #24713, I'm trying to fix this issue and I was thinking about using a variable to know how much time a single url can be repeated on the redirects.

Is two time sufficient or we will have another case that need 3 times.

If you agree using a variable is it okay to pass it like the follow on the get/post/head/trace/put method of the test client (max_page_redirect) and make it by default 2 or store it on the settings ?

def put(self, path, data='', content_type='application/octet-stream',

            follow=False, max_page_redirect=2,

            secure=False, **extra):

        """

        Send a resource to the server using PUT.

        """

        response = super(Client, self).put(path, data=data,

                                           content_type=content_type,

                                           secure=secure, **extra)

        if follow:

            response = self._handle_redirects(response, max_page_redirect,

                                              **extra)

        return response

[Tim Graham]

Is there any disadvantage to making it some "large" number like 10? In that case it probably wouldn't need to be configurable. The additional time to do 10 loops instead of 2 is probably trivial (and it's an unexpected condition anyway).

[Mounir Messelmeni]

I think 10 is already too much, it about redirecting to the same page N times. Page1 to Page, Page2 to Page3 and Page3 to the Page1: N=2 redirects to the Page1.

Do you think 10 times make senses? In my opinion it's too much and the need to raise an error to make developers know about something wrong in their redirects else they can override it for an extreme purpose.

[Mounir Messelmeni]

We've already the max total redirects counts it's 20

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)

[Tim Graham]

My thinking is that a redirect loop of n>2 doesn't seem likely to exit on its own if the developer doesn't intend it to, thus the n we pick shouldn't really matter.

[Collin Anderson]

If it helps, I believe Chrome and Firefox have a limit of 20, and Internet Explorer have a limit of 10.

[Luke Scott]

Not sure if this is where I should comment on this issue but I just upgraded to Django 1.8 and am encountering this error:

    RedirectCycleError: Redirect loop detected.

I have a view that looks for a certain condition. If that condition is not met then I redirect to a second view. That second view does something then most of the time will redirect back to the first view. This works well in reality, but in testing results in  'RedirectCycleError: Redirect loop detected' when the second view redirects back to the first.

[Shai Berger]

Hi Scott,

On Sunday 07 February 2016 04:34:59 Luke Scott wrote:
> Not sure if this is where I should comment on this issue but I just
> upgraded to Django 1.8 and am encountering this error:
>
> RedirectCycleError: Redirect loop detected.
>
>
> I have a view that looks for a certain condition. If that condition is not
> met then I redirect to a second view. That second view does something then
> most of the time will redirect back to the first view. This works well in
> reality, but in testing results in 'RedirectCycleError: Redirect loop
> detected' when the second view redirects back to the first.
>

Your use case seems to be an exact replica of the ticket referenced in this
thread [1]; it was closed as "wontfix". You can look at the explanation for
closing, and see if it satisfies you; if it doesn't, feel free to bring up new
arguments for implementing it on this list (and even this thread).

HTH,
Shai.

[1] https://code.djangoproject.com/ticket/24713