From dd66c224052d90cb08b3b7828ab0de205e069c0b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 14 Apr 2020 18:39:38 -0500 Subject: [PATCH 1/2] Handle paths with trailing `/` --- readthedocs/proxito/tests/test_full.py | 28 ++++++++++++++++++++++++++ readthedocs/proxito/views/serve.py | 5 +++-- readthedocs/proxito/views/utils.py | 3 ++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 95419fdb6bc..c0306ea7e5c 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -370,6 +370,34 @@ def test_404_storage_serves_custom_404_sphinx(self, storage_mock): ) self.assertEqual(response.status_code, 404) + @mock.patch('readthedocs.proxito.views.serve.get_storage_class') + def test_redirects_to_correct_index(self, storage_mock): + """This case is when the project uses a README.html as index.""" + self.project.versions.update(active=True, built=True) + fancy_version = fixture.get( + Version, + slug='fancy-version', + privacy_level=constants.PUBLIC, + active=True, + built=True, + project=self.project, + documentation_type=SPHINX, + ) + + storage_mock()().exists.side_effect = [False, True] + response = self.client.get( + reverse('proxito_404_handler', kwargs={'proxito_path': '/en/fancy-version/not-found/'}), + HTTP_HOST='project.readthedocs.io', + ) + storage_mock()().exists.assert_has_calls( + [ + mock.call('html/project/fancy-version/not-found/index.html'), + mock.call('html/project/fancy-version/not-found/README.html'), + ] + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response['location'], '/en/fancy-version/not-found/README.html') + @mock.patch('readthedocs.proxito.views.serve.get_storage_class') def test_404_storage_serves_custom_404_sphinx_single_html(self, storage_mock): self.project.versions.update(active=True, built=True) diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index 64ef7a61410..fd6aa48cad1 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -226,10 +226,11 @@ def get(self, request, proxito_path, template_name='404.html'): ) # Use urlparse so that we maintain GET args in our redirect parts = urlparse(proxito_path) + path = parts.path.rstrip('/') if tryfile == 'README.html': - new_path = f'{parts.path}/{tryfile}' + new_path = f'{path}/{tryfile}' else: - new_path = parts.path.rstrip('/') + '/' + new_path = path + '/' new_parts = parts._replace(path=new_path) redirect_url = new_parts.geturl() diff --git a/readthedocs/proxito/views/utils.py b/readthedocs/proxito/views/utils.py index b4c1d10a63b..e0bdf289114 100644 --- a/readthedocs/proxito/views/utils.py +++ b/readthedocs/proxito/views/utils.py @@ -58,7 +58,7 @@ def _get_project_data_from_request( # Handle single-version projects that have URLs like a real project if current_project.single_version: if lang_slug and version_slug: - filename = os.path.join(lang_slug, version_slug, filename) + filename = f'{lang_slug}/{version_slug}/{filename}' log.warning( 'URL looks like versioned on a single version project.' 'Changing filename to match. filename=%s', @@ -85,4 +85,5 @@ def _get_project_data_from_request( # * Subproject # * Translations + filename = filename.rstrip('/') return final_project, lang_slug, version_slug, filename From 0e946d9ea5e84170ba13816d6456be2eebdcf9dd Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 14 Apr 2020 19:16:37 -0500 Subject: [PATCH 2/2] Only change where path is used --- readthedocs/proxito/views/serve.py | 7 +++---- readthedocs/proxito/views/utils.py | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index fd6aa48cad1..9146e3f574d 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -210,7 +210,7 @@ def get(self, request, proxito_path, template_name='404.html'): # First, check for dirhtml with slash for tryfile in ('index.html', 'README.html'): - storage_filename_path = f'{storage_root_path}/{filename}/{tryfile}' + storage_filename_path = f'{storage_root_path}/' + filename.strip('/') + f'/{tryfile}' log.debug( 'Trying index filename: project=%s version=%s, file=%s', final_project.slug, @@ -226,11 +226,10 @@ def get(self, request, proxito_path, template_name='404.html'): ) # Use urlparse so that we maintain GET args in our redirect parts = urlparse(proxito_path) - path = parts.path.rstrip('/') if tryfile == 'README.html': - new_path = f'{path}/{tryfile}' + new_path = parts.path.rstrip('/') + f'/{tryfile}' else: - new_path = path + '/' + new_path = parts.path.rstrip('/') + '/' new_parts = parts._replace(path=new_path) redirect_url = new_parts.geturl() diff --git a/readthedocs/proxito/views/utils.py b/readthedocs/proxito/views/utils.py index e0bdf289114..b4c1d10a63b 100644 --- a/readthedocs/proxito/views/utils.py +++ b/readthedocs/proxito/views/utils.py @@ -58,7 +58,7 @@ def _get_project_data_from_request( # Handle single-version projects that have URLs like a real project if current_project.single_version: if lang_slug and version_slug: - filename = f'{lang_slug}/{version_slug}/{filename}' + filename = os.path.join(lang_slug, version_slug, filename) log.warning( 'URL looks like versioned on a single version project.' 'Changing filename to match. filename=%s', @@ -85,5 +85,4 @@ def _get_project_data_from_request( # * Subproject # * Translations - filename = filename.rstrip('/') return final_project, lang_slug, version_slug, filename