Opened 6 years ago

Closed 4 years ago

#30422 closed Bug (fixed)

Temporary files do not get deleted on canceled upload request.

Reported by: drutalj Owned by: Sanskar Jaiswal
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mariusz Felisiak)

Temporary files do not get deleted when upload (request) is canceled by client (e.g. browser is closed before upload is finished).

Change History (17)

comment:1 by Mariusz Felisiak, 6 years ago

Description: modified (diff)
Summary: Temp upload file does not get deleted on canceled upload request → Temporary files do not get deleted on canceled upload request.
Triage Stage: Unreviewed → Accepted
Version: 2.2 → master

Reproduced at ed880d92b50c641c3e7f6e8ce5741085ffe1f8fb with django.core.files.uploadhandler.TemporaryFileUploadHandler.

comment:2 by Mattia Procopio, 5 years ago

Owner: changed from nobody to Mattia Procopio
Status: new → assigned

comment:3 by Mattia Procopio, 5 years ago

I am analysing a bit the code to deal with uploads. I have noticed that file_complete method is never called if upload is interrupted.
Now, since content lenght seems to be unreliable (in my tests is always 0) I am thinking how to solve this issue in a clean way.
One of the option I am thinking about is to add a new flag to TemporaryFileUploadHandler that will be set to True within file_complete and having some cleanup code within upload_complete that will check if the flag is True or False and will delete the incomplete file.
Does this sound right?

comment:4 by Sanskar Jaiswal, 5 years ago

Does this have a PR merged? If not can I look into this? I want to get started with contributing to Django for a while now, but I haven't been able to do so. Also, could someone please tell me if this ticket is easy enough for a beginner to resolve?

comment:5 by Carlton Gibson, 5 years ago

Hi Sanskar. There's no patch. You can look into Mattia's suggestion to see if it makes sense.

comment:6 by Mattia Procopio, 5 years ago

Owner: Mattia Procopio removed
Status: assigned → new

comment:7 by Sanskar Jaiswal, 5 years ago

Owner: set to Sanskar Jaiswal
Status: new → assigned

comment:8 by Carlton Gibson, 5 years ago

Has patch: set
Patch needs improvement: set

​PR

comment:9 by Sanskar Jaiswal, 4 years ago

All tests run on my machine (macOS) but the CI test on Windows fails because of a PermissionError, due to the temporary file being used by some other process. Is there any possible fix for this on Windows? Or should I just alter my patch to handle such os errors and label this patch as POSIX only?

comment:10 by Sanskar Jaiswal, 4 years ago

Patch needs improvement: unset

comment:11 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:12 by Sanskar Jaiswal, 4 years ago

Patch needs improvement: unset

comment:13 by Mariusz Felisiak, 4 years ago

Needs tests: set
Patch needs improvement: set

comment:14 by Sanskar Jaiswal, 4 years ago

Needs tests: unset
Patch needs improvement: unset

comment:15 by Mariusz Felisiak, 4 years ago

Triage Stage: Accepted → Ready for checkin

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 21b127bf:

Refs #30422 -- Added test for removing temporary files in MultiPartParser when StopUpload is raised.

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: → fixed
Status: assigned → closed

In 11c4a44:

Fixed #30422 -- Made TemporaryFileUploadHandler handle interrupted uploads.

This patch allows upload handlers to handle interrupted uploads.

Co-Authored-By: Mariusz Felisiak <felisiak.mariusz@…>

Note: See TracTickets for help on using tickets.
Back to Top