Fixed #31240 -- Properly closed FileResponse when wsgi.file_wrapper is used.

Thanks to Oskar Persson for the report.

django/core/handlers/base.py

@@ -73,7 +73,7 @@ class BaseHandler:
         # Setup default url resolver for this thread
         set_urlconf(settings.ROOT_URLCONF)
         response = self._middleware_chain(request)
-        response._closable_objects.append(request)
+        response._resource_closers.append(request.close)
         if response.status_code >= 400:
             log_response(
                 '%s: %s', response.reason_phrase, request.path,

django/core/handlers/wsgi.py

@@ -141,6 +141,10 @@ class WSGIHandler(base.BaseHandler):
         ]
         start_response(status, response_headers)
         if getattr(response, 'file_to_stream', None) is not None and environ.get('wsgi.file_wrapper'):
+            # If `wsgi.file_wrapper` is used the WSGI server does not call
+            # .close on the response, but on the file wrapper. Patch it to use
+            # response.close instead which takes care of closing all files.
+            response.file_to_stream.close = response.close
             response = environ['wsgi.file_wrapper'](response.file_to_stream, response.block_size)
         return response
 

django/http/response.py

@@ -41,7 +41,7 @@ class HttpResponseBase:
         # the header (required for working with legacy systems) and the header
         # value. Both the name of the header and its value are ASCII strings.
         self._headers = {}
-        self._closable_objects = []
+        self._resource_closers = []
         # This parameter is set by the handler. It's necessary to preserve the
         # historical behavior of request_finished.
         self._handler_class = None
@@ -243,11 +243,13 @@ class HttpResponseBase:
     # The WSGI server must call this method upon completion of the request.
     # See http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html
     def close(self):
-        for closable in self._closable_objects:
+        for closer in self._resource_closers:
             try:
-                closable.close()
+                closer()
             except Exception:
                 pass
+        # Free resources that were still referenced.
+        self._resource_closers.clear()
         self.closed = True
         signals.request_finished.send(sender=self._handler_class)
 
@@ -378,7 +380,7 @@ class StreamingHttpResponse(HttpResponseBase):
         # Ensure we can never iterate on "value" more than once.
         self._iterator = iter(value)
         if hasattr(value, 'close'):
-            self._closable_objects.append(value)
+            self._resource_closers.append(value.close)
 
     def __iter__(self):
         return self.streaming_content
@@ -405,7 +407,7 @@ class FileResponse(StreamingHttpResponse):
 
         self.file_to_stream = filelike = value
         if hasattr(filelike, 'close'):
-            self._closable_objects.append(filelike)
+            self._resource_closers.append(filelike.close)
         value = iter(lambda: filelike.read(self.block_size), b'')
         self.set_headers(filelike)
         super()._set_streaming_content(value)

docs/releases/3.0.4.txt

@@ -11,3 +11,6 @@ Bugfixes
 
 * Fixed a data loss possibility when using caching from async code
   (:ticket:`31253`).
+
+* Fixed a regression in Django 3.0 that caused a file response using a
+  temporary file to be closed incorrectly (:ticket:`31240`).

tests/builtin_server/tests.py

@@ -4,6 +4,11 @@ from io import BytesIO
 from unittest import TestCase
 from wsgiref import simple_server
 
+from django.core.servers.basehttp import get_internal_wsgi_application
+from django.test import RequestFactory, override_settings
+
+from .views import FILE_RESPONSE_HOLDER
+
 # If data is too large, socket will choke, so write chunks no larger than 32MB
 # at a time. The rationale behind the 32MB can be found in #5596#comment:4.
 MAX_SOCKET_CHUNK_SIZE = 32 * 1024 * 1024  # 32 MB
@@ -89,6 +94,27 @@ class WSGIFileWrapperTests(TestCase):
         self.assertEqual(handler.stdout.getvalue().splitlines()[-1], b'Hello World!')
         self.assertEqual(handler.stderr.getvalue(), b'')
 
+    @override_settings(ROOT_URLCONF='builtin_server.urls')
+    def test_file_response_closing(self):
+        """
+        View returning a FileResponse properly closes the file and http
+        response when file_wrapper is used.
+        """
+        env = RequestFactory().get('/fileresponse/').environ
+        handler = FileWrapperHandler(None, BytesIO(), BytesIO(), env)
+        handler.run(get_internal_wsgi_application())
+        # Sendfile is used only when file_wrapper has been used.
+        self.assertTrue(handler._used_sendfile)
+        # Fetch the original response object.
+        self.assertIn('response', FILE_RESPONSE_HOLDER)
+        response = FILE_RESPONSE_HOLDER['response']
+        # The response and file buffers are closed.
+        self.assertIs(response.closed, True)
+        buf1, buf2 = FILE_RESPONSE_HOLDER['buffers']
+        self.assertIs(buf1.closed, True)
+        self.assertIs(buf2.closed, True)
+        FILE_RESPONSE_HOLDER.clear()
+
 
 class WriteChunkCounterHandler(ServerHandler):
     """

tests/builtin_server/urls.py

@@ -0,0 +1,7 @@
+from django.urls import path
+
+from . import views
+
+urlpatterns = [
+    path('fileresponse/', views.file_response),
+]

tests/builtin_server/views.py

@@ -0,0 +1,15 @@
+from io import BytesIO
+
+from django.http import FileResponse
+
+FILE_RESPONSE_HOLDER = {}
+
+
+def file_response(request):
+    f1 = BytesIO(b"test1")
+    f2 = BytesIO(b"test2")
+    response = FileResponse(f1)
+    response._resource_closers.append(f2.close)
+    FILE_RESPONSE_HOLDER['response'] = response
+    FILE_RESPONSE_HOLDER['buffers'] = (f1, f2)
+    return response