Merge pull request #6387 from readthedocs/proxito-redirect

Redirect index files in proxito instead of serving

Author: ericholscher

readthedocs/proxito/tests/test_full.py

@@ -189,17 +189,49 @@ def test_custom_robots_txt(self, storage_mock):
     def test_directory_indexes(self, storage_mock):
         self.project.versions.update(active=True, built=True)
         storage_mock()().exists.return_value = True
-        storage_mock()().open().read.return_value = 'foo'
         # Confirm we've serving from storage for the `index-exists/index.html` file
         response = self.client.get(
             reverse('serve_error_404', kwargs={'proxito_path': '/en/latest/index-exists'}),
             HTTP_HOST='project.readthedocs.io',
         )
         self.assertEqual(
-            response.content, b'foo'
+            response.status_code, 302
         )
         self.assertEqual(
-            response.status_code, 200
+            response['location'], '/en/latest/index-exists/',
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_directory_indexes_readme_serving(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+
+        storage_mock()().exists.side_effect = [False, True]
+        # Confirm we've serving from storage for the `index-exists/index.html` file
+        response = self.client.get(
+            reverse('serve_error_404', kwargs={'proxito_path': '/en/latest/readme-exists'}),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(
+            response.status_code, 302
+        )
+        self.assertEqual(
+            response['location'], '/en/latest/readme-exists/README.html',
+        )
+
+    @mock.patch('readthedocs.proxito.views.get_storage_class')
+    def test_directory_indexes_get_args(self, storage_mock):
+        self.project.versions.update(active=True, built=True)
+        storage_mock()().exists.return_value = True
+        # Confirm we've serving from storage for the `index-exists/index.html` file
+        response = self.client.get(
+            reverse('serve_error_404', kwargs={'proxito_path': '/en/latest/index-exists?foo=bar'}),
+            HTTP_HOST='project.readthedocs.io',
+        )
+        self.assertEqual(
+            response.status_code, 302
+        )
+        self.assertEqual(
+            response['location'], '/en/latest/index-exists/?foo=bar',
         )
 
     @mock.patch('readthedocs.proxito.views.get_storage_class')

readthedocs/proxito/views.py

@@ -385,12 +385,19 @@ def serve_error_404(request, proxito_path, template_name='404.html'):
         )
         if storage.exists(storage_filename_path):
             log.info(
-                'Serving index file: project=%s version=%s, url=%s',
+                'Redirecting to index file: project=%s version=%s, url=%s',
                 final_project.slug,
                 version_slug,
                 storage_filename_path,
             )
-            resp = HttpResponse(storage.open(storage_filename_path).read())
+            # Use urlparse so that we maintain GET args in our redirect
+            parts = urlparse(proxito_path)
+            if tryfile == 'README.html':
+                new_path = os.path.join(parts.path, tryfile)
+            else:
+                new_path = parts.path + '/'
+            new_parts = parts._replace(path=new_path)
+            resp = HttpResponseRedirect(new_parts.geturl())
             return resp
 
     # If that doesn't work, attempt to serve the 404 of the current version (version_slug)