diff --git a/readthedocs/proxito/tests/test_full.py b/readthedocs/proxito/tests/test_full.py index 715bc79c92a..9425dbcd16a 100644 --- a/readthedocs/proxito/tests/test_full.py +++ b/readthedocs/proxito/tests/test_full.py @@ -1007,6 +1007,88 @@ def test_redirects_to_correct_index_ending_without_slash(self): self.assertEqual(response['location'], '/en/fancy-version/not-found/README.html') @mock.patch.object(BuildMediaFileSystemStorageTest, 'open') + def test_404_index_redirect_skips_not_built_versions(self, storage_open): + self.version.built = False + self.version.save() + get( + HTMLFile, + project=self.project, + version=self.version, + path="foo/index.html", + name="index.html", + ) + + response = self.client.get( + reverse( + "proxito_404_handler", + kwargs={"proxito_path": "/en/latest/foo"}, + ), + headers={"host": "project.readthedocs.io"}, + ) + self.assertEqual(response.status_code, 404) + storage_open.assert_not_called() + + @mock.patch.object(BuildMediaFileSystemStorageTest, "open") + def test_custom_404_skips_not_built_versions(self, storage_open): + self.version.built = False + self.version.save() + + fancy_version = fixture.get( + Version, + slug="fancy-version", + privacy_level=constants.PUBLIC, + active=True, + built=False, + project=self.project, + ) + + get( + HTMLFile, + project=self.project, + version=self.version, + path="404.html", + name="404.html", + ) + + get( + HTMLFile, + project=self.project, + version=fancy_version, + path="404.html", + name="404.html", + ) + + response = self.client.get( + reverse( + "proxito_404_handler", + kwargs={"proxito_path": "/en/fancy-version/not-found"}, + ), + headers={"host": "project.readthedocs.io"}, + ) + self.assertEqual(response.status_code, 404) + storage_open.assert_not_called() + + @mock.patch.object(BuildMediaFileSystemStorageTest, "open") + def test_custom_404_doesnt_exist_in_storage(self, storage_open): + storage_open.side_effect = FileNotFoundError + get( + HTMLFile, + project=self.project, + version=self.version, + path="404.html", + name="404.html", + ) + response = self.client.get( + reverse( + "proxito_404_handler", + kwargs={"proxito_path": "/en/latest/not-found"}, + ), + headers={"host": "project.readthedocs.io"}, + ) + self.assertEqual(response.status_code, 404) + storage_open.assert_called_once_with("html/project/latest/404.html") + + @mock.patch.object(BuildMediaFileSystemStorageTest, "open") def test_404_storage_serves_custom_404_sphinx_single_html(self, storage_open): self.project.versions.update(active=True, built=True) fancy_version = fixture.get( diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index 35040c025fa..313740a5aa0 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -444,8 +444,10 @@ def get(self, request, proxito_path): # If we were able to resolve to a valid version, it means that the # current file doesn't exist. So we check if we can redirect to its # index file if it exists before doing anything else. + # If the version isn't marked as built, we don't check for index files, + # since the version doesn't have any files. # This is /en/latest/foo -> /en/latest/foo/index.html. - if version: + if version and version.built: response = self._get_index_file_redirect( request=request, project=project, @@ -549,15 +551,18 @@ def _get_custom_404_page(self, request, project, version=None): We check for a 404.html or 404/index.html file. + We don't check for a custom 404 page in versions that aren't marked as built, + since they don't have any files. + If a 404 page is found, we return a response with the content of that file, `None` otherwise. """ - versions_404 = [version] if version else [] + versions_404 = [version] if version and version.built else [] if not version or version.slug != project.default_version: default_version = project.versions.filter( slug=project.default_version ).first() - if default_version: + if default_version and default_version.built: versions_404.append(default_version) if not versions_404: @@ -594,11 +599,15 @@ def _get_custom_404_page(self, request, project, version=None): version_slug_404=version_404.slug, storage_filename_path=storage_filename_path, ) - resp = HttpResponse( - build_media_storage.open(storage_filename_path).read() - ) - resp.status_code = 404 - return resp + try: + content = build_media_storage.open(storage_filename_path).read() + return HttpResponse(content, status=404) + except FileNotFoundError: + log.warning( + "File not found in storage. File out of sync with DB.", + file=storage_filename_path, + ) + return None return None def _get_index_file_redirect(self, request, project, version, filename, full_path):