Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#8455 closed (fixed)

Lack of filesystem permissions to save uploaded file results in server hang

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

Description

If the web server user does not have filesystem permissions to write to the "upload_to" directory of a FileField or ImageField, attempting to upload a file results in the server process hanging and eating 100% CPU.

Instead, there should be a 500 error or debug page with a sensible error message.

This issue seems to exist ever since file-storage-refactor. I can replicate it on both the Django development server and Apache 2.2 on Debian Linux.

I believe the problem is that django.core.file.storage.FileSystemStorage._save() makes unwarranted assumptions about the reason for an OSError, causing an infinite loop, as outlined by rajeshdhawan in this comment on #8203.

Attachments (3)

issue-8455.diff (953 bytes) - added by carljm 7 years ago.
patch to check OSError error number
issue-8455.errno.diff (914 bytes) - added by carljm 7 years ago.
better patch, uses os.errno.EEXISTS instead of raw error number
issue-8455.errno2.diff (1005 bytes) - added by carljm 7 years ago.
os.errno is Python 2.5 only, use top-level errno instead

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by carljm

patch to check OSError error number

comment:1 Changed 7 years ago by carljm

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Don't know if this is the best way to go about it, but this patch resolves the issue for me. It should also fix the problem for any other potential error condition (out of disk space, whatever) that could result in an OSError.

comment:2 Changed 7 years ago by carljm

In particular, I should note that I have no idea whether the error number checked by my patch is cross-platform or not (I've only tested on Linux). If there aren't reliable cross-platform error numbers, a good patch might need to entirely rework the logic (do an explicit test for the existence of the file and not catch OSError at all?).

Changed 7 years ago by carljm

better patch, uses os.errno.EEXISTS instead of raw error number

Changed 7 years ago by carljm

os.errno is Python 2.5 only, use top-level errno instead

comment:3 Changed 7 years ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 7 years ago by jacob

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

(In [8639]) Fixed #8455: a lack of permissions in MEDIA_ROOT no longer causes an infinite loop when saving files. Thanks, carljm.

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