Merge pull request #6906 from readthedocs/fix-trailing-slash

Handle paths with trailing `/`
readthedocs · Apr 15, 2020 · 9dd29b1 · 9dd29b1
2 parents 5270b76 + 0e946d9
commit 9dd29b1
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 2 deletions.
diff --git 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
@@ -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,
@@ -227,7 +227,7 @@ 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)
                 if tryfile == 'README.html':
-                    new_path = f'{parts.path}/{tryfile}'
+                    new_path = parts.path.rstrip('/') + f'/{tryfile}'
                 else:
                     new_path = parts.path.rstrip('/') + '/'
                 new_parts = parts._replace(path=new_path)