Opened 4 months ago

Closed 4 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, 4 months ago

Component: UncategorizedFile uploads/storage
Has patch: set

I opened a PR.

comment:2 by Natalia Bidart, 4 months ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/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, 4 months ago

Needs tests: set
Patch needs improvement: set

comment:4 by bcail, 4 months ago

Needs tests: unset

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

comment:5 by bcail, 4 months ago

Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 4 months ago

Owner: changed from nobody to bcail
Status: newassigned
Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

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