From bc1cfb346e78f520139201112d1109dad69b3011 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 31 Oct 2018 13:29:55 +0100 Subject: [PATCH 1/2] Avoid infinite redirection --- readthedocs/core/views/__init__.py | 4 ++- readthedocs/rtd_tests/tests/test_redirects.py | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index e97a7247ba1..04838bbb328 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -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(): + # check that we do have a response and avoid infinite redirect return response r = render(request, template_name) r.status_code = 404 diff --git a/readthedocs/rtd_tests/tests/test_redirects.py b/readthedocs/rtd_tests/tests/test_redirects.py index 7595d147568..335c8feba86 100644 --- a/readthedocs/rtd_tests/tests/test_redirects.py +++ b/readthedocs/rtd_tests/tests/test_redirects.py @@ -127,6 +127,34 @@ def setUp(self): self.pip = Project.objects.get(slug='pip') self.pip.versions.create_latest() + @override_settings(USE_SUBDOMAIN=True) + def test_redirect_prefix_infinite(self): + """ + Avoid infinite redirects. + + If the URL hit is the same that the URL returned for redirection, we + return a 404. + + These examples comes from this issue: + * https://github.com/rtfd/readthedocs.org/issues/4673 + """ + Redirect.objects.create( + project=self.pip, redirect_type='prefix', + from_url='/', + ) + r = self.client.get('/redirect', HTTP_HOST='pip.readthedocs.org') + self.assertEqual(r.status_code, 302) + self.assertEqual( + r['Location'], 'http://pip.readthedocs.org/en/latest/redirect.html') + + r = self.client.get('/redirect/', HTTP_HOST='pip.readthedocs.org') + self.assertEqual(r.status_code, 302) + self.assertEqual( + r['Location'], 'http://pip.readthedocs.org/en/latest/redirect/') + + r = self.client.get('/en/latest/redirect/', HTTP_HOST='pip.readthedocs.org') + self.assertEqual(r.status_code, 404) + @override_settings(USE_SUBDOMAIN=True) def test_redirect_root(self): Redirect.objects.create( From 00fe39a5b13ffbd0d6f5da383ea52743f009f99b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 31 Oct 2018 14:42:55 +0100 Subject: [PATCH 2/2] Log a WARNING when the infinite redirect happens --- readthedocs/core/views/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index 04838bbb328..342eb530498 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -117,9 +117,15 @@ def server_error_404(request, exception=None, template_name='404.html'): # pyli """ response = get_redirect_response(request, path=request.get_full_path()) - if response and response.url != request.build_absolute_uri(): - # check that we do have a response and avoid infinite redirect - return response + if response: + if response.url == request.build_absolute_uri(): + # check that we do have a response and avoid infinite redirect + log.warning( + 'Infinite Redirect: FROM URL is the same than TO URL. url=%s', + response.url, + ) + else: + return response r = render(request, template_name) r.status_code = 404 return r