Avoid infinite redirection #4833
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4833 +/- ##
=========================================
+ Coverage 76.2% 76.2% +<.01%
=========================================
Files 158 158
Lines 10022 10024 +2
Branches 1265 1266 +1
=========================================
+ Hits 7637 7639 +2
Misses 2041 2041
Partials 344 344
|
readthedocs/core/views/__init__.py
Outdated
| @@ -116,7 +116,9 @@ def server_error_404(request, exception=None, template_name='404.html'): # pyli | |||
| Marking exception as optional to make /404/ testing page to work. | |||
| """ | |||
| response = get_redirect_response(request, path=request.get_full_path()) | |||
| if response: | |||
|
|
|||
| if response and response.url != request.build_absolute_uri(): | |||
There was a problem hiding this comment.
I wonder if we should add some logging here if there is a loop. It would be best to actually be able to alert the user, because presumably they have misconfigured a redirect and will be confused. Not sure the best course of action tho, because doing a debug log will never be shown in prod :/
Perhaps we should show a different error code in this case, perhaps a 500 level, to show that something is broken vs. just missing?
There was a problem hiding this comment.
I wonder if we should add some logging here if there is a loop
👍 on adding a log.warning if this happens.
Perhaps we should show a different error code in this case, perhaps a 500 level, to show that something is broken vs. just missing?
Mmm... Well, for the use case were we detected this, what we want here is a 404 not a 500 actually (you can read the issue that this PR closes for more context).
I suppose that returning a 404 is the first step here to avoid this problem.
There was a problem hiding this comment.
Yea, better than actually returning an infinite redirect. So I'm +1 on shipping and then improving later.
Closes #4673