summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--django/core/files/storage.py2
-rw-r--r--django/core/files/uploadedfile.py3
-rw-r--r--django/core/handlers/base.py2
-rw-r--r--django/http/multipartparser.py10
-rw-r--r--django/http/request.py6
-rw-r--r--docs/releases/1.7.txt4
-rw-r--r--tests/file_storage/tests.py20
-rw-r--r--tests/file_uploads/tests.py31
-rw-r--r--tests/file_uploads/urls.py1
-rw-r--r--tests/file_uploads/views.py6
10 files changed, 79 insertions, 6 deletions
diff --git a/django/core/files/storage.py b/django/core/files/storage.py
index 98864f7f22..58c623b883 100644
--- a/django/core/files/storage.py
+++ b/django/core/files/storage.py
@@ -209,7 +209,6 @@ class FileSystemStorage(Storage):
# This file has a file path that we can move.
if hasattr(content, 'temporary_file_path'):
file_move_safe(content.temporary_file_path(), full_path)
- content.close()
# This is a normal uploadedfile that we can stream.
else:
@@ -228,7 +227,6 @@ class FileSystemStorage(Storage):
_file = os.fdopen(fd, mode)
_file.write(chunk)
finally:
- content.close()
locks.unlock(fd)
if _file is not None:
_file.close()
diff --git a/django/core/files/uploadedfile.py b/django/core/files/uploadedfile.py
index 75f25405b9..9a94894424 100644
--- a/django/core/files/uploadedfile.py
+++ b/django/core/files/uploadedfile.py
@@ -96,9 +96,6 @@ class InMemoryUploadedFile(UploadedFile):
def open(self, mode=None):
self.file.seek(0)
- def close(self):
- pass
-
def chunks(self, chunk_size=None):
self.file.seek(0)
yield self.read()
diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py
index ffc48a076a..887898a33c 100644
--- a/django/core/handlers/base.py
+++ b/django/core/handlers/base.py
@@ -207,6 +207,8 @@ class BaseHandler(object):
signals.got_request_exception.send(sender=self.__class__, request=request)
response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
+ response._closable_objects.append(request)
+
return response
def handle_uncaught_exception(self, request, resolver, exc_info):
diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py
index 20232cb7c6..1bcace94cd 100644
--- a/django/http/multipartparser.py
+++ b/django/http/multipartparser.py
@@ -228,6 +228,7 @@ class MultiPartParser(object):
break
except SkipFile:
+ self._close_files()
# Just use up the rest of this file...
exhaust(field_stream)
else:
@@ -237,6 +238,7 @@ class MultiPartParser(object):
# If this is neither a FIELD or a FILE, just exhaust the stream.
exhaust(stream)
except StopUpload as e:
+ self._close_files()
if not e.connection_reset:
exhaust(self._input_data)
else:
@@ -268,6 +270,14 @@ class MultiPartParser(object):
"""Cleanup filename from Internet Explorer full paths."""
return filename and filename[filename.rfind("\\") + 1:].strip()
+ def _close_files(self):
+ # Free up all file handles.
+ # FIXME: this currently assumes that upload handlers store the file as 'file'
+ # We should document that... (Maybe add handler.free_file to complement new_file)
+ for handler in self._upload_handlers:
+ if hasattr(handler, 'file'):
+ handler.file.close()
+
class LazyStream(six.Iterator):
"""
diff --git a/django/http/request.py b/django/http/request.py
index ded4744a87..ecad639c82 100644
--- a/django/http/request.py
+++ b/django/http/request.py
@@ -5,6 +5,7 @@ import os
import re
import sys
from io import BytesIO
+from itertools import chain
from pprint import pformat
from django.conf import settings
@@ -245,6 +246,11 @@ class HttpRequest(object):
else:
self._post, self._files = QueryDict('', encoding=self._encoding), MultiValueDict()
+ def close(self):
+ if hasattr(self, '_files'):
+ for f in chain.from_iterable(l[1] for l in self._files.lists()):
+ f.close()
+
# File-like and iterator interface.
#
# Expects self._stream to be set to an appropriate source of bytes by
diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt
index d70c40a057..e8f25a45e1 100644
--- a/docs/releases/1.7.txt
+++ b/docs/releases/1.7.txt
@@ -504,6 +504,10 @@ File Uploads
attribute is now optional. If it is omitted or given ``None`` or an empty
string, a subdirectory won't be used for storing the uploaded files.
+* Uploaded files are now explicitly closed before the response is delivered to
+ the client. Partially uploaded files are also closed as long as they are
+ named ``file`` in the upload handler.
+
Forms
^^^^^
diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index a0e2e2a6cc..b7e4bcdf6d 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -19,7 +19,8 @@ from django.core.cache import cache
from django.core.exceptions import SuspiciousOperation
from django.core.files.base import File, ContentFile
from django.core.files.storage import FileSystemStorage, get_storage_class
-from django.core.files.uploadedfile import SimpleUploadedFile
+from django.core.files.uploadedfile import (InMemoryUploadedFile, SimpleUploadedFile,
+ TemporaryUploadedFile)
from django.test import LiveServerTestCase, SimpleTestCase
from django.test import override_settings
from django.utils import six
@@ -206,6 +207,23 @@ class FileStorageTests(unittest.TestCase):
self.storage.delete('path/to/test.file')
+ def test_save_doesnt_close(self):
+ with TemporaryUploadedFile('test', 'text/plain', 1, 'utf8') as file:
+ file.write(b'1')
+ file.seek(0)
+ self.assertFalse(file.closed)
+ self.storage.save('path/to/test.file', file)
+ self.assertFalse(file.closed)
+ self.assertFalse(file.file.closed)
+
+ file = InMemoryUploadedFile(six.StringIO('1'), '', 'test',
+ 'text/plain', 1, 'utf8')
+ with file:
+ self.assertFalse(file.closed)
+ self.storage.save('path/to/test.file', file)
+ self.assertFalse(file.closed)
+ self.assertFalse(file.file.closed)
+
def test_file_path(self):
"""
File storage returns the full path of a file
diff --git a/tests/file_uploads/tests.py b/tests/file_uploads/tests.py
index cb2a6dd92f..c933d38236 100644
--- a/tests/file_uploads/tests.py
+++ b/tests/file_uploads/tests.py
@@ -327,6 +327,37 @@ class FileUploadTests(TestCase):
self.assertEqual(got.get('file1'), 1)
self.assertEqual(got.get('file2'), 2)
+ def test_fileuploads_closed_at_request_end(self):
+ file = tempfile.NamedTemporaryFile
+ with file() as f1, file() as f2a, file() as f2b:
+ response = self.client.post('/fd_closing/t/', {
+ 'file': f1,
+ 'file2': (f2a, f2b),
+ })
+
+ request = response.wsgi_request
+ # Check that the files got actually parsed.
+ self.assertTrue(hasattr(request, '_files'))
+
+ file = request._files['file']
+ self.assertTrue(file.closed)
+
+ files = request._files.getlist('file2')
+ self.assertTrue(files[0].closed)
+ self.assertTrue(files[1].closed)
+
+ def test_no_parsing_triggered_by_fd_closing(self):
+ file = tempfile.NamedTemporaryFile
+ with file() as f1, file() as f2a, file() as f2b:
+ response = self.client.post('/fd_closing/f/', {
+ 'file': f1,
+ 'file2': (f2a, f2b),
+ })
+
+ request = response.wsgi_request
+ # Check that the fd closing logic doesn't trigger parsing of the stream
+ self.assertFalse(hasattr(request, '_files'))
+
def test_file_error_blocking(self):
"""
The server should not block when there are upload errors (bug #8622).
diff --git a/tests/file_uploads/urls.py b/tests/file_uploads/urls.py
index a6f446bf55..b5f90c88c8 100644
--- a/tests/file_uploads/urls.py
+++ b/tests/file_uploads/urls.py
@@ -15,4 +15,5 @@ urlpatterns = patterns('',
(r'^getlist_count/$', views.file_upload_getlist_count),
(r'^upload_errors/$', views.file_upload_errors),
(r'^filename_case/$', views.file_upload_filename_case_view),
+ (r'^fd_closing/(?P<access>t|f)/$', views.file_upload_fd_closing),
)
diff --git a/tests/file_uploads/views.py b/tests/file_uploads/views.py
index a29391af6e..de195f4d39 100644
--- a/tests/file_uploads/views.py
+++ b/tests/file_uploads/views.py
@@ -157,3 +157,9 @@ def file_upload_content_type_extra(request):
(k, smart_str(v)) for k, v in uploadedfile.content_type_extra.items()
])
return HttpResponse(json.dumps(params))
+
+
+def file_upload_fd_closing(request, access):
+ if access == 't':
+ request.FILES # Trigger file parsing.
+ return HttpResponse('')
generated by cgit v1.3 (git 2.53.0) at 2026-07-06 01:18:43 +0000