Opened 10 years ago
Closed 10 years ago
#24713 closed Bug (wontfix)
Redirect loop detection in test client is incorrect
Reported by: | agmathew | Owned by: | |
---|---|---|---|
Component: | Testing framework | Version: | 1.8 |
Severity: | Release blocker | Keywords: | redirect loop detection test client |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
We recently upgraded our Django installation from 1.6 and 1.8 and noticed our unit tests were failing in multiple places. We tracked down the problem to changes stemming from the fix for #23682, which introduces stronger looper detection for redirects. The problem is that we have logic in our codebase that goes like this:
- Go to /view/
- If a dirty bit is set, redirect to /update/ and remove the dirty bit
- Redirect back to /view/
The loop detection erroneously considers this a redirect loop because /view/ is visited twice. Browsers don't have a problem with our redirects. I propose that the redirect loop detection should check that a previous site has been visited N times (for some value of N) before reporting a redirect loop. If this sounds reasonable, I can create a patch.
Change History (7)
comment:1 by , 10 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 10 years ago
Please review my PullRequest and check if we raise the RedirectCycleError when redirecting to the same page more than 2 time is okay or we need to make it greater.
comment:4 by , 10 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:5 by , 10 years ago
Has patch: | set |
---|---|
Needs tests: | set |
comment:6 by , 10 years ago
From Mounir on the pull request:
I don't think making multiple redirects for a client is the right way. Let take an example: A user is trying to access a home page "/home" and the server need to update an information, let's say a last_visit datetime field or something else -- do we need to redirect him to "/update" make this update and redirect him again to "/home"? I think it's better to run a signal, an asynchronous task, make the update by calling a model method or something else or even do the logic from the "/home" view. If we are not sure that this is a common issue for many (now it look like only one person have this problem) I think it's better to keep things like they are.
I'd agree with this. Is there a reason you can't restructure your code to avoid the redirect? See also the discussion on the django-developers mailing list.
comment:7 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Yes, it does seem reasonable.