Use `FileResponse` when serving files

- Ensure text documents are binary
- Django internally will deadlock if the file sent isn't bytes
- Remove unnecessary open call

CHANGELOG.txt

@@ -135,6 +135,7 @@ Changelog
  * Maintenance: Migrate page listing menu re-ordering (drag & drop) from jQuery inline scripts to `OrderableController` (Aman Pandey, LB (Ben) Johnston)
  * Maintenance: Clean up scss variable usage, remove unused variables and mixins, adopt more core token variables (Jai Vignesh J, Nandini Arora, LB (Ben) Johnston)
  * Maintenance: Migrate Image URL generator views to class based views (Rohit Sharma)
+ * Maintenance: Use Django's `FileResponse` when serving files such as Images or Documents (Jake Howard)
 
 
 5.2.3 (xx.xx.xxxx) - IN DEVELOPMENT

docs/releases/6.0.md

@@ -179,6 +179,7 @@ Thank you to Thibaud Colas, Badr Fourane, and Sage Abdullah for their work on th
  * Migrate page listing menu re-ordering (drag & drop) from jQuery inline scripts to `OrderableController` (Aman Pandey, LB (Ben) Johnston)
  * Clean up scss variable usage, remove unused variables and mixins, adopt more core token variables (Jai Vignesh J, Nandini Arora, LB (Ben) Johnston)
  * Migrate Image URL generator views to class based views (Rohit Sharma)
+ * Use Django's `FileResponse` when serving files such as Images or Documents (Jake Howard)
 
 ## Upgrade considerations - removal of deprecated features from Wagtail 4.2 - 5.1
 

wagtail/documents/tests/test_views.py

@@ -1,7 +1,6 @@
 import os.path
 import unittest
 import urllib
-from io import StringIO
 from unittest import mock
 
 from django.conf import settings
@@ -18,11 +17,11 @@ class TestServeView(TestCase):
     def setUp(self):
         self.document = models.Document(title="Test document", file_hash="123456")
         self.document.file.save(
-            "serve_view.doc", ContentFile("A boring example document")
+            "serve_view.doc", ContentFile(b"A boring example document")
         )
         self.pdf_document = models.Document(title="Test document", file_hash="123456")
         self.pdf_document.file.save(
-            "serve_view.pdf", ContentFile("A boring example document")
+            "serve_view.pdf", ContentFile(b"A boring example document")
         )
 
     def tearDown(self):
@@ -74,10 +73,9 @@ class TestServeView(TestCase):
         mock_doc.filename = self.document.filename
         mock_doc.content_type = self.document.content_type
         mock_doc.content_disposition = self.document.content_disposition
-        mock_doc.file = StringIO("file-like object" * 10)
+        mock_doc.file = ContentFile(b"file-like object" * 10)
         mock_doc.file.path = None
         mock_doc.file.url = None
-        mock_doc.file.size = 30
         mock_get_object_or_404.return_value = mock_doc
 
         # Bypass 'before_serve_document' hooks
@@ -108,10 +106,9 @@ class TestServeView(TestCase):
         mock_doc.filename = self.pdf_document.filename
         mock_doc.content_type = self.pdf_document.content_type
         mock_doc.content_disposition = self.pdf_document.content_disposition
-        mock_doc.file = StringIO("file-like object" * 10)
+        mock_doc.file = ContentFile(b"file-like object" * 10)
         mock_doc.file.path = None
         mock_doc.file.url = None
-        mock_doc.file.size = 30
         mock_get_object_or_404.return_value = mock_doc
 
         # Bypass 'before_serve_document' hooks
@@ -384,10 +381,9 @@ class TestServeWithUnicodeFilename(TestCase):
         # Create a mock document to hit the correct code path.
         mock_doc = mock.Mock()
         mock_doc.filename = "TÈST.doc"
-        mock_doc.file = StringIO("file-like object" * 10)
+        mock_doc.file = ContentFile(b"file-like object" * 10)
         mock_doc.file.path = None
         mock_doc.file.url = None
-        mock_doc.file.size = 30
         mock_get_object_or_404.return_value = mock_doc
 
         # Bypass 'before_serve_document' hooks

wagtail/documents/views/serve.py

@@ -1,7 +1,5 @@
-from wsgiref.util import FileWrapper
-
 from django.conf import settings
-from django.http import Http404, HttpResponse, StreamingHttpResponse
+from django.http import FileResponse, Http404, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -96,10 +94,8 @@ def serve(request, document_id, document_filename):
         # (e.g. storages.backends.s3boto.S3BotoStorage) AND the developer has not allowed
         # redirecting to the file url directly.
         # Fall back on pre-sendfile behaviour of reading the file content and serving it
-        # as a StreamingHttpResponse
-
-        wrapper = FileWrapper(doc.file)
-        response = StreamingHttpResponse(wrapper, doc.content_type)
+        # as a FileResponse
+        response = FileResponse(doc.file, doc.content_type)
 
         # set filename and filename* to handle non-ascii characters in filename
         # see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

wagtail/images/views/serve.py

@@ -1,7 +1,5 @@
-from wsgiref.util import FileWrapper
-
 from django.core.exceptions import ImproperlyConfigured, PermissionDenied
-from django.http import HttpResponse, StreamingHttpResponse
+from django.http import FileResponse, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse
 from django.utils.decorators import classonlymethod, method_decorator
@@ -66,11 +64,8 @@ class ServeView(View):
         with rendition.get_willow_image() as willow_image:
             mime_type = willow_image.mime_type
 
-        # Open and serve the file
-        rendition.file.open("rb")
-        return StreamingHttpResponse(
-            FileWrapper(rendition.file), content_type=mime_type
-        )
+        # Serve the file
+        return FileResponse(rendition.file, content_type=mime_type)
 
     def redirect(self, rendition):
         # Redirect to the file's public location

wagtail/utils/sendfile_streaming_backend.py

@@ -4,9 +4,8 @@
 import os
 import stat
 from email.utils import mktime_tz, parsedate_tz
-from wsgiref.util import FileWrapper
 
-from django.http import HttpResponseNotModified, StreamingHttpResponse
+from django.http import FileResponse, HttpResponseNotModified
 from django.utils.http import http_date
 
 
@@ -20,7 +19,7 @@ def sendfile(request, filename, **kwargs):
     ):
         return HttpResponseNotModified()
 
-    response = StreamingHttpResponse(FileWrapper(open(filename, "rb")))
+    response = FileResponse(open(filename, "rb"))
 
     response["Last-Modified"] = http_date(statobj[stat.ST_MTIME])
     return response