Proxito: don't check for index.html if the path already ends with /. (#10153)

Author: stsewd

readthedocs/proxito/tests/test_full.py

@@ -796,10 +796,42 @@ def test_404_storage_serves_custom_404_sphinx(self, storage_exists, storage_open
         self.assertEqual(response.status_code, 404)
 
     @mock.patch.object(BuildMediaFileSystemStorageTest, 'exists')
-    def test_redirects_to_correct_index(self, storage_exists):
-        """This case is when the project uses a README.html as index."""
+    def test_redirects_to_correct_index_ending_with_slash(self, storage_exists):
+        """When the path ends with a slash, we try README.html as index."""
         self.project.versions.update(active=True, built=True)
-        fancy_version = fixture.get(
+        fixture.get(
+            Version,
+            slug="fancy-version",
+            privacy_level=constants.PUBLIC,
+            active=True,
+            built=True,
+            project=self.project,
+            documentation_type=SPHINX,
+        )
+
+        storage_exists.side_effect = [True]
+        response = self.client.get(
+            reverse(
+                "proxito_404_handler",
+                kwargs={"proxito_path": "/en/fancy-version/not-found/"},
+            ),
+            HTTP_HOST="project.readthedocs.io",
+        )
+        storage_exists.assert_has_calls(
+            [
+                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.object(BuildMediaFileSystemStorageTest, "exists")
+    def test_redirects_to_correct_index_ending_without_slash(self, storage_exists):
+        """When the path doesn't end with a slash, we try both, index.html and README.html."""
+        self.project.versions.update(active=True, built=True)
+        fixture.get(
             Version,
             slug='fancy-version',
             privacy_level=constants.PUBLIC,
@@ -811,8 +843,11 @@ def test_redirects_to_correct_index(self, storage_exists):
 
         storage_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',
+            reverse(
+                "proxito_404_handler",
+                kwargs={"proxito_path": "/en/fancy-version/not-found"},
+            ),
+            HTTP_HOST="project.readthedocs.io",
         )
         storage_exists.assert_has_calls(
             [
@@ -1143,7 +1178,7 @@ def test_track_broken_link_custom_404(self, storage_exists, storage_open):
                 HTTP_HOST="project.readthedocs.io",
             )
             self.assertEqual(resp.status_code, 404)
-            storage_open.assert_called_once_with("html/project/latest/404.html")
+            storage_open.assert_called_once()
 
         self.assertEqual(PageView.objects.all().count(), 2)
         version = self.project.versions.get(slug="latest")

readthedocs/proxito/views/serve.py

@@ -691,8 +691,15 @@ def _get_index_file_redirect(self, request, project, version, filename, full_pat
             version_type=self.version_type,
         )
 
+        tryfiles = ["index.html", "README.html"]
+        # If the path ends with `/`, we already tried to serve
+        # the `/index.html` file, so we only need to test for
+        # the `/README.html` file.
+        if full_path.endswith("/"):
+            tryfiles = ["README.html"]
+
         # First, check for dirhtml with slash
-        for tryfile in ("index.html", "README.html"):
+        for tryfile in tryfiles:
             storage_filename_path = build_media_storage.join(
                 storage_root_path,
                 f"{filename}/{tryfile}".lstrip("/"),