[Django] #35323: Fix FileExistsError in django/core/files/move.py.

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.
-----------------------------------------+------------------------
Reporter: bcail | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
In file_move_safe(), if the destination exists and allow_overwrite is
False,
[https://github.com/django/django/blob/main/django/core/files/move.py#L47
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35323>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.

-------------------------------------+-------------------------------------

Reporter: bcail | Owner: nobody
Type: Bug | Status: new

Component: File | Version: dev
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by bcail):

* component: Uncategorized => File uploads/storage
* has_patch: 0 => 1

Comment:

I opened [https://github.com/django/django/pull/18001 a PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:1>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.

--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization

Comment:

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?
--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:2>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 1 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:3>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by bcail):

* needs_tests: 1 => 0

Comment:

Thanks, Natalia. I updated the PR with a test for that specific
`FileExistsError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:4>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.
--------------------------------------+------------------------------------
Reporter: bcail | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by bcail):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:5>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.

-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail
Type: | Status: assigned
Cleanup/optimization |

Component: File | Version: dev
uploads/storage |

Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* owner: nobody => bcail
* stage: Accepted => Ready for checkin
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:6>

[Django]

#35323: Fix FileExistsError in django/core/files/move.py.
-------------------------------------+-------------------------------------
Reporter: bcail | Owner: bcail

Type: | Status: closed

Cleanup/optimization |
Component: File | Version: dev
uploads/storage |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"07c8d979aecf5f1f71ce4f174369df39f24d24b3" 07c8d979]:
{{{#!CommitTicketReference repository=""
revision="07c8d979aecf5f1f71ce4f174369df39f24d24b3"
Fixed #35323 -- Prevented file_move_safe() from trying to overwrite
existing file when allow_overwrite is False.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35323#comment:7>