From 11c4a4412b74bb1dfe52d706a58f230066821c33 Mon Sep 17 00:00:00 2001 From: aryan Date: Thu, 20 Feb 2020 00:23:48 +0530 Subject: [PATCH] Fixed #30422 -- Made TemporaryFileUploadHandler handle interrupted uploads. This patch allows upload handlers to handle interrupted uploads. Co-Authored-By: Mariusz Felisiak --- django/core/files/uploadhandler.py | 18 +++++++++++++++++- django/http/multipartparser.py | 7 +++++++ docs/ref/files/uploads.txt | 7 +++++++ docs/releases/3.2.txt | 5 +++-- tests/file_uploads/tests.py | 27 ++++++++++++++++++++++++++- tests/file_uploads/urls.py | 1 + tests/file_uploads/views.py | 10 ++++++++++ 7 files changed, 71 insertions(+), 4 deletions(-) diff --git a/django/core/files/uploadhandler.py b/django/core/files/uploadhandler.py index 613983581c..ee6bb31fce 100644 --- a/django/core/files/uploadhandler.py +++ b/django/core/files/uploadhandler.py @@ -1,7 +1,7 @@ """ Base file upload handler classes, and the built-in concrete subclasses """ - +import os from io import BytesIO from django.conf import settings @@ -127,6 +127,13 @@ class FileUploadHandler: """ pass + def upload_interrupted(self): + """ + Signal that the upload was interrupted. Subclasses should perform + cleanup that is necessary for this handler. + """ + pass + class TemporaryFileUploadHandler(FileUploadHandler): """ @@ -147,6 +154,15 @@ class TemporaryFileUploadHandler(FileUploadHandler): self.file.size = file_size return self.file + def upload_interrupted(self): + if hasattr(self, 'file'): + temp_location = self.file.temporary_file_path() + try: + self.file.close() + os.remove(temp_location) + except FileNotFoundError: + pass + class MemoryFileUploadHandler(FileUploadHandler): """ diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py index b3472f7be2..8078393a66 100644 --- a/django/http/multipartparser.py +++ b/django/http/multipartparser.py @@ -150,6 +150,8 @@ class MultiPartParser: num_post_keys = 0 # To limit the amount of data read from the request. read_size = None + # Whether a file upload is finished. + uploaded_file = True try: for item_type, meta_data, field_stream in Parser(stream, self._boundary): @@ -159,6 +161,7 @@ class MultiPartParser: # we hit the next boundary/part of the multipart content. self.handle_file_complete(old_field_name, counters) old_field_name = None + uploaded_file = True try: disposition = meta_data['content-disposition'][1] @@ -225,6 +228,7 @@ class MultiPartParser: content_length = None counters = [0] * len(handlers) + uploaded_file = False try: for handler in handlers: try: @@ -279,6 +283,9 @@ class MultiPartParser: if not e.connection_reset: exhaust(self._input_data) else: + if not uploaded_file: + for handler in handlers: + handler.upload_interrupted() # Make sure that the request data is all fed exhaust(self._input_data) diff --git a/docs/ref/files/uploads.txt b/docs/ref/files/uploads.txt index 4ce4eb991a..eba7392ddc 100644 --- a/docs/ref/files/uploads.txt +++ b/docs/ref/files/uploads.txt @@ -212,6 +212,13 @@ attributes: Callback signaling that the entire upload (all files) has completed. +.. method:: FileUploadHandler.upload_interrupted() + + .. versionadded:: 3.2 + + Callback signaling that the upload was interrupted, e.g. when the user + closed their browser during file upload. + .. method:: FileUploadHandler.handle_raw_input(input_data, META, content_length, boundary, encoding) Allows the handler to completely override the parsing of the raw diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 2c59c488c0..ec78dd863a 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -195,8 +195,9 @@ File Storage File Uploads ~~~~~~~~~~~~ -* ... - +* The new :meth:`FileUploadHandler.upload_interrupted() + ` + callback allows handling interrupted uploads. Forms ~~~~~ diff --git a/tests/file_uploads/tests.py b/tests/file_uploads/tests.py index d8cd8425a4..2a1b2998e5 100644 --- a/tests/file_uploads/tests.py +++ b/tests/file_uploads/tests.py @@ -6,12 +6,13 @@ import sys import tempfile as sys_tempfile import unittest from io import BytesIO, StringIO +from unittest import mock from urllib.parse import quote from django.core.files import temp as tempfile from django.core.files.uploadedfile import SimpleUploadedFile from django.http.multipartparser import ( - MultiPartParser, MultiPartParserError, parse_header, + FILE, MultiPartParser, MultiPartParserError, Parser, parse_header, ) from django.test import SimpleTestCase, TestCase, client, override_settings @@ -443,6 +444,30 @@ class FileUploadTests(TestCase): temp_path = response.json()['temp_path'] self.assertIs(os.path.exists(temp_path), False) + def test_upload_interrupted_temporary_file_handler(self): + # Simulate an interrupted upload by omitting the closing boundary. + class MockedParser(Parser): + def __iter__(self): + for item in super().__iter__(): + item_type, meta_data, field_stream = item + yield item_type, meta_data, field_stream + if item_type == FILE: + return + + with tempfile.NamedTemporaryFile() as temp_file: + temp_file.write(b'a') + temp_file.seek(0) + with mock.patch( + 'django.http.multipartparser.Parser', + MockedParser, + ): + response = self.client.post( + '/temp_file/upload_interrupted/', + {'file': temp_file}, + ) + temp_path = response.json()['temp_path'] + self.assertIs(os.path.exists(temp_path), False) + def test_fileupload_getlist(self): file = tempfile.NamedTemporaryFile with file() as file1, file() as file2, file() as file2a: diff --git a/tests/file_uploads/urls.py b/tests/file_uploads/urls.py index 84bb452fc4..d171be2c76 100644 --- a/tests/file_uploads/urls.py +++ b/tests/file_uploads/urls.py @@ -14,6 +14,7 @@ urlpatterns = [ path('getlist_count/', views.file_upload_getlist_count), path('upload_errors/', views.file_upload_errors), path('temp_file/stop_upload/', views.file_stop_upload_temporary_file), + path('temp_file/upload_interrupted/', views.file_upload_interrupted_temporary_file), path('filename_case/', views.file_upload_filename_case_view), re_path(r'^fd_closing/(?Pt|f)/$', views.file_upload_fd_closing), ] diff --git a/tests/file_uploads/views.py b/tests/file_uploads/views.py index 04bb25e012..d521f001fe 100644 --- a/tests/file_uploads/views.py +++ b/tests/file_uploads/views.py @@ -2,6 +2,7 @@ import hashlib import os from django.core.files.uploadedfile import UploadedFile +from django.core.files.uploadhandler import TemporaryFileUploadHandler from django.http import HttpResponse, HttpResponseServerError, JsonResponse from .models import FileModel @@ -112,6 +113,15 @@ def file_stop_upload_temporary_file(request): ) +def file_upload_interrupted_temporary_file(request): + request.upload_handlers.insert(0, TemporaryFileUploadHandler()) + request.upload_handlers.pop(2) + request.FILES # Trigger file parsing. + return JsonResponse( + {'temp_path': request.upload_handlers[0].file.temporary_file_path()}, + ) + + def file_upload_getlist_count(request): """ Check the .getlist() function to ensure we receive the correct number of files.