Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#8479 closed (duplicate)

Race condition in core.files.storage.py, leads to crash

Reported by: aubonbeurre Owned by: nobody
Component: File uploads/storage Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

This seems related to #4948, but not quite, so I opened a new ticket.

Line 148:

# This file has a file path that we can move.
if hasattr(content, 'temporary_file_path'):

file_move_safe(content.temporary_file_path(), full_path)
content.close()

This should be inverted:

# This file has a file path that we can move.
if hasattr(content, 'temporary_file_path'):

content.close()
file_move_safe(content.temporary_file_path(), full_path)

The code throws an exception, because it ends-up to copy-then-delete, since in my particular case src and dest are not on the same file system.

It will fail on Windows, and I suspect this is particular to NTFS (although I did not test on other platforms): it is not possible to remove an opened file.

Change History (5)

comment:1 Changed 7 years ago by aubonbeurre

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

better formatting...

                # This file has a file path that we can move.
                if hasattr(content, 'temporary_file_path'):
                    file_move_safe(content.temporary_file_path(), full_path)
                    content.close()

Should change to:

                # This file has a file path that we can move.
                if hasattr(content, 'temporary_file_path'):
                    content.close()
                    file_move_safe(content.temporary_file_path(), full_path)

comment:2 Changed 7 years ago by aubonbeurre

And I forgot to mention that the effect is that, because os.unlink(src) fails deep in the code, it will keep appending '_' till the name is too long. Then it will fail for good.

comment:3 Changed 7 years ago by ubernostrum

  • Component changed from Core framework to File uploads/storage

comment:4 Changed 7 years ago by aubonbeurre

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #8203. Verified fixed by [8493]. Should close the bug. Thanks!!!

comment:5 Changed 4 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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