Merge pull request #9048 from readthedocs/humitos/dont-serve-inactive-non-existent-versions

Author: humitos

readthedocs/proxito/tests/test_full.py

@@ -198,6 +198,31 @@ def test_external_version_serving_old_slugs(self):
 
     # Invalid tests
 
+    def test_non_existent_version(self):
+        url = "/en/non-existent-version/"
+        host = "project.dev.readthedocs.io"
+        resp = self.client.get(url, HTTP_HOST=host)
+        self.assertEqual(resp.status_code, 404)
+
+    def test_non_existent_version_with_filename(self):
+        url = "/en/non-existent-version/doesnt-exist.html"
+        host = "project.dev.readthedocs.io"
+        resp = self.client.get(url, HTTP_HOST=host)
+        self.assertEqual(resp.status_code, 404)
+
+    def test_inactive_version(self):
+        url = "/en/inactive/"
+        host = "project.dev.readthedocs.io"
+        fixture.get(
+            Version,
+            verbose_name="inactive",
+            slug="inactive",
+            active=False,
+            project=self.project,
+        )
+        resp = self.client.get(url, HTTP_HOST=host)
+        self.assertEqual(resp.status_code, 404)
+
     @override_settings(
         RTD_EXTERNAL_VERSION_DOMAIN='dev.readthedocs.build',
     )

readthedocs/proxito/views/serve.py

@@ -84,23 +84,35 @@ def get(self,
             version_slug=version_slug,
             filename=filename,
         )
-
-        # All public versions can be cached.
         version = final_project.versions.filter(slug=version_slug).first()
-        if (
-            self._is_cache_enabled(final_project)
-            and version and not version.is_private
-        ):
-            self.cache_request = True
 
         log.bind(
             project_slug=final_project.slug,
             subproject_slug=subproject_slug,
             lang_slug=lang_slug,
             version_slug=version_slug,
             filename=filename,
-            cache_request=self.cache_request,
         )
+
+        # Skip serving versions that are not active (return 404). This is to
+        # avoid serving files that we have in the storage, but its associated
+        # version does not exist anymore or it was de-activated.
+        #
+        # Note that we want to serve the page when `version is None` because it
+        # could be a valid URL, like `/` or `` (empty) that does not have a
+        # version associated to it.
+        #
+        # However, if there is a `version_slug` in the URL but there is no
+        # version on the database we want to return 404.
+        if (version and not version.active) or (version_slug and not version):
+            log.warning("Version does not exist or is not active.")
+            raise Http404("Version does not exist or is not active.")
+
+        if self._is_cache_enabled(final_project) and version and not version.is_private:
+            # All public versions can be cached.
+            self.cache_request = True
+
+        log.bind(cache_request=self.cache_request)
         log.debug('Serving docs.')
 
         # Verify if the project is marked as spam and return a 401 in that case