Opened 9 months ago

Closed 9 months ago

#35323 closed Cleanup/optimization (fixed)

Fix FileExistsError in django/core/files/move.py.

Reported by: bcail Owned by: bcail
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: bcail 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

In file_move_safe(), if the destination exists and allow_overwrite is False, ​FileExistsError is raised - but it's immediately swallowed because it's inside a try block. The code goes on to try the manual copy process, and a new FileExistsError is raised in that code.

Change History (7)

comment:1 by bcail, 9 months ago

Component: Uncategorized → File uploads/storage
Has patch: set

I opened ​a PR.

comment:2 by Natalia Bidart, 9 months ago

Triage Stage: Unreviewed → Accepted
Type: Bug → Cleanup/optimization

Makes sense, good catch!

I wonder if we could add a test that ensures that the first FileExistsError is not swallowed. Do you want to give that a try?

comment:3 by Mariusz Felisiak, 9 months ago

Needs tests: set
Patch needs improvement: set

comment:4 by bcail, 9 months ago

Needs tests: unset

Thanks, Natalia. I updated the PR with a test for that specific FileExistsError.

comment:5 by bcail, 9 months ago

Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 9 months ago

Owner: changed from nobody to bcail
Status: new → assigned
Triage Stage: Accepted → Ready for checkin

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 9 months ago

Resolution: → fixed
Status: assigned → closed

In 07c8d979:

Fixed #35323 -- Prevented file_move_safe() from trying to overwrite existing file when allow_overwrite is False.

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